Fix bad folding (#54722)

* Always sign-extend from int32 in gtFoldExprConst

GT_CNS_INT of TYP_INT on 64 hosts has an implicit contract:
the value returned by IconValue() must "fit" into 32 bit
signed integer, i. e. "static_cast<int>(IconValue()) == IconValue()".
gtFoldExprConst was failing to uphold this contract when the target
was 32 bit and the host was 64 bit.

Fix this by always truncating before calling SetIconValue().

* Add a simple test that reproduces bad codegen

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 277cd53e1c10bcae07d5c818b0d980393c6cd021..7f16e6cd400484ad9a0960dfb607d1bc77df743f 100644 (file)
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -14856,19 +14856,15 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree)
             JITDUMP("\nFolding operator with constant nodes into a constant:\n");
             DISPTREE(tree);
 
-#ifdef TARGET_64BIT
-            // Some operations are performed as 64 bit instead of 32 bit so the upper 32 bits
-            // need to be discarded. Since constant values are stored as ssize_t and the node
-            // has TYP_INT the result needs to be sign extended rather than zero extended.
-            i1 = INT32(i1);
-#endif // TARGET_64BIT
-
             // Also all conditional folding jumps here since the node hanging from
             // GT_JTRUE has to be a GT_CNS_INT - value 0 or 1.
 
             tree->ChangeOperConst(GT_CNS_INT);
             tree->ChangeType(TYP_INT);
-            tree->AsIntCon()->SetIconValue(i1);
+            // Some operations are performed as 64 bit instead of 32 bit so the upper 32 bits
+            // need to be discarded. Since constant values are stored as ssize_t and the node
+            // has TYP_INT the result needs to be sign extended rather than zero extended.
+            tree->AsIntCon()->SetIconValue(static_cast<int>(i1));
             tree->AsIntCon()->gtFieldSeq = fieldSeq;
             if (vnStore != nullptr)
             {

diff --git a/src/tests/JIT/Directed/ConstantFolding/folding_extends_int32_on_64_bit_hosts.cs b/src/tests/JIT/Directed/ConstantFolding/folding_extends_int32_on_64_bit_hosts.cs
new file mode 100644 (file)
index 0000000..08d7e7a
--- /dev/null
+++ b/src/tests/JIT/Directed/ConstantFolding/folding_extends_int32_on_64_bit_hosts.cs
@@ -0,0 +1,27 @@
+public class FoldingExtendsInt32On64BitHostsTest
+{
+    // On 64 bit hosts, 32 bit constants are stored as 64 bit signed values.
+    // gtFoldExpr failed to properly truncate the folded value to 32 bits when
+    // the host was 64 bit and the target - 32 bit. Thus local assertion prop
+    // got the "poisoned" value, which lead to silent bad codegen.
+
+    public static int Main()
+    {
+        var r1 = 31;
+        // "Poisoned" value.
+        var s1 = 0b11 << r1;
+
+        if (s1 == 0b11 << 31)
+        {
+            return 100;
+        }
+
+        // Just so that Roslyn actually uses locals.
+        Use(s1);
+        Use(r1);
+
+        return -1;
+    }
+
+    private static void Use(int a) { }
+}

diff --git a/src/tests/JIT/Directed/ConstantFolding/folding_extends_int32_on_64_bit_hosts.csproj b/src/tests/JIT/Directed/ConstantFolding/folding_extends_int32_on_64_bit_hosts.csproj
new file mode 100644 (file)
index 0000000..edc51be
--- /dev/null
+++ b/src/tests/JIT/Directed/ConstantFolding/folding_extends_int32_on_64_bit_hosts.csproj
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+    <DebugType>None</DebugType>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>