From 9a61573cd58c315fe45c1e81442608e412414496 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 12 Nov 2021 19:39:42 +0300 Subject: [PATCH 1/2] Remove the "anything + null => null" optimization 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 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 83946626f25d92..c2b28db5a014ec 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12332,27 +12332,9 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) 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. From e60e4523ff5d0373207194a18b4119acdc634d23 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 12 Nov 2021 20:42:28 +0300 Subject: [PATCH 2/2] Add the test --- .../JitBlue/Runtime_61510/Runtime_61510.cs | 23 +++++++++++++++++++ .../Runtime_61510/Runtime_61510.csproj | 10 ++++++++ 2 files changed, 33 insertions(+) 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/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 00000000000000..fc40b9b1d775fd --- /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 00000000000000..cf94135633b19a --- /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