From 3d7e89c1c929b1365423730693d9732391c608ec Mon Sep 17 00:00:00 2001 From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Mon, 15 Nov 2021 19:35:28 +0300 Subject: [PATCH] Remove the "anything + null => null" optimization (#61518) This optimization is only legal if: 1) "Anything" is a sufficiently small constant itself. 2) We are in a context where we know the address will in fact be used for an indirection. It is the second point that is problematic - one would like to use MorphAddrContext, but it is not suitable for this purpose, as an unknown context is counted as an indirecting one. Additionally, the value of this optimization is rather low. I am guessing it was meant to support the legacy nullchecks, before GT_NULLCHECK was introduced, and had higher impact then. So, just remove the optimization and leave the 5 small regressions across all of SPMI be. --- src/coreclr/jit/morph.cpp | 18 ----------------- .../JitBlue/Runtime_61510/Runtime_61510.cs | 23 ++++++++++++++++++++++ .../JitBlue/Runtime_61510/Runtime_61510.csproj | 10 ++++++++++ 3 files changed, 33 insertions(+), 18 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.csproj diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 3862f49..d6adf0d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12401,27 +12401,9 @@ DONE_MORPHING_CHILDREN: if (op2->IsCnsIntOrI() && varTypeIsIntegralOrI(typ)) { - CLANG_FORMAT_COMMENT_ANCHOR; - // Fold (x + 0). - if ((op2->AsIntConCommon()->IconValue() == 0) && !gtIsActiveCSE_Candidate(tree)) { - - // If this addition is adding an offset to a null pointer, - // avoid the work and yield the null pointer immediately. - // Dereferencing the pointer in either case will have the - // same effect. - - if (!optValnumCSE_phase && varTypeIsGC(op2->TypeGet()) && - ((op1->gtFlags & GTF_ALL_EFFECT) == 0)) - { - op2->gtType = tree->gtType; - DEBUG_DESTROY_NODE(op1); - DEBUG_DESTROY_NODE(tree); - return op2; - } - // Remove the addition iff it won't change the tree type // to TYP_REF. diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.cs b/src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.cs new file mode 100644 index 0000000..fc40b9b --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; + +unsafe class Runtime_61510 +{ + [FixedAddressValueType] + private static byte s_field; + + public static int Main() + { + ref byte result = ref AddZeroByrefToNativeInt((nint)Unsafe.AsPointer(ref s_field)); + + return Unsafe.AreSame(ref s_field, ref result) ? 100 : 101; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static ref byte AddZeroByrefToNativeInt(nint addr) + { + return ref Unsafe.Add(ref Unsafe.NullRef(), addr); + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.csproj new file mode 100644 index 0000000..cf94135 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.csproj @@ -0,0 +1,10 @@ + + + Exe + True + true + + + + + \ No newline at end of file -- 2.7.4