[JIT] Fix - Do not remove `CAST` nodes on store if the `LCL_VAR` is a parameter or...

* Do not remove CAST nodes on assignment if the LCL_VAR is a parameter.

* Added NormalizeOnLoad rules from SingleAccretion. Added description of why we cannot remove CAST nodes from parameters.

* Remove morph optimization for NormalizeOnLoad in fgMorphLocalVar. Update test.

* Do not OptimizeCastOnStore for params and struct fields

* Update src/coreclr/jit/morph.cpp

Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
* Formatting

---------

Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 37fe012..01e5446 100644 (file)
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -1096,6 +1096,14 @@ public:
         assert(lvIsStructField);
         return ((lvFldOffset % TARGET_POINTER_SIZE) == 0);
     }
+
+    // NormalizeOnLoad Rules:
+    //     1. All small locals are actually TYP_INT locals.
+    //     2. NOL locals are such that not all definitions can be controlled by the compiler and so the upper bits can
+    //        be undefined.For parameters this is the case because of ABI.For struct fields - because of padding.For
+    //        address - exposed locals - because not all stores are direct.
+    //     3. Hence, all NOL uses(unless proven otherwise) are assumed in morph to have undefined upper bits and
+    //        explicit casts have be inserted to "normalize" them back to conform to IL semantics.
     bool lvNormalizeOnLoad() const
     {
         return varTypeIsSmall(TypeGet()) &&

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index c420051..1522e4d 100644 (file)
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -10078,8 +10078,20 @@ GenTree* Compiler::fgOptimizeCastOnStore(GenTree* store)
     if (!src->OperIs(GT_CAST))
         return store;
 
-    if (store->OperIs(GT_STORE_LCL_VAR) && !lvaGetDesc(store->AsLclVarCommon())->lvNormalizeOnLoad())
-        return store;
+    if (store->OperIs(GT_STORE_LCL_VAR))
+    {
+        LclVarDsc* varDsc = lvaGetDesc(store->AsLclVarCommon()->GetLclNum());
+
+        // We can make this transformation only under the assumption that NOL locals are always normalized before they
+        // are used,
+        // however this is not always the case: the JIT will utilize subrange assertions for NOL locals to make
+        // normalization
+        // assumptions -- see fgMorphLeafLocal. Thus we can only do this for cases where we know for sure that
+        // subsequent uses
+        // will normalize, which we can only guarantee when the local is address exposed.
+        if (!varDsc->lvNormalizeOnLoad() || !varDsc->IsAddressExposed())
+            return store;
+    }
 
     if (src->gtOverflow())
         return store;

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs b/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs
index 19e39ef..65e6202 100644 (file)
--- a/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs
@@ -28,7 +28,7 @@ public class Test
                }
        }
 
-       [ActiveIssue("https://github.com/dotnet/runtime/issues/85081")]
+       [Fact]
        public static int TestEntryPoint() {
                var result = Test.Program.M8(1);
                if (result != 255)

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_85382/Runtime_85382.cs b/src/tests/JIT/Regression/JitBlue/Runtime_85382/Runtime_85382.cs
new file mode 100644 (file)
index 0000000..0b39426
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_85382/Runtime_85382.cs
@@ -0,0 +1,98 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class Test
+{
+    // This is trying to verify that we properly zero-extend on all platforms.
+       public class Program
+       {
+               public static long s_15;
+               public static sbyte s_17;
+               public static ushort s_21 = 36659;
+               public static int Test()
+               {
+                       s_15 = ~1;
+                       return M40(0);
+               }
+
+               [MethodImpl(MethodImplOptions.NoInlining)]
+               public static void Consume(ushort x) { }
+
+               [MethodImpl(MethodImplOptions.NoInlining)]
+               public static int M40(ushort arg0)
+               {
+                       for (int var0 = 0; var0 < 2; var0++)
+                       {
+                               arg0 = 65535;
+                               arg0 &= (ushort)(s_15++ >> s_17);
+                               arg0 %= s_21;
+                       }
+
+                       Consume(arg0);
+
+                       if (arg0 != 28876)
+                       {
+                               return 0;
+                       }
+                       return 100;
+               }
+       }
+
+       public class Program2
+       {
+               public static long s_15;
+               public static sbyte s_17;
+               public static ushort s_21 = 36659;
+               public static int Test()
+               {
+                       s_15 = ~1;
+                       return M40();
+               }
+
+               [MethodImpl(MethodImplOptions.NoInlining)]
+               public static void Consume(ushort x) { }
+
+               [MethodImpl(MethodImplOptions.NoInlining)]
+               public static int M40()
+               {
+                       S s = default;
+                       for (int var0 = 0; var0 < 2; var0++)
+                       {
+                               s.U = 65535;
+                               s.U &= (ushort)(s_15++ >> s_17);
+                               s.U %= s_21;
+                       }
+
+                       Consume(s.U);
+
+                       if (s.U != 28876)
+                       {
+                               return 0;
+                       }
+                       return 100;
+               }
+
+               public struct S { public ushort U; }
+       }
+
+       [Fact]
+       public static int TestEntryPoint() {
+               if (Test.Program.Test() != 100)
+               {
+                       return 0;
+               }
+
+               if (Test.Program2.Test() != 100)
+               {
+                       return 0;
+               }
+
+               return 100;
+       }
+}

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_85382/Runtime_85382.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_85382/Runtime_85382.csproj
new file mode 100644 (file)
index 0000000..15edd99
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_85382/Runtime_85382.csproj
@@ -0,0 +1,8 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file