Remove the "anything + null => null" optimization (#61518)
authorSingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Mon, 15 Nov 2021 16:35:28 +0000 (19:35 +0300)
committerGitHub <noreply@github.com>
Mon, 15 Nov 2021 16:35:28 +0000 (08:35 -0800)
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
src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.cs [new file with mode: 0644]
src/tests/JIT/Regression/JitBlue/Runtime_61510/Runtime_61510.csproj [new file with mode: 0644]

index 3862f49..d6adf0d 100644 (file)
@@ -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 (file)
index 0000000..fc40b9b
--- /dev/null
@@ -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<byte>(), 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 (file)
index 0000000..cf94135
--- /dev/null
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file