Do not fold relocatable constants into displacements (#68851)

The code in "genCreateAddrMode" was performing the equivalent of constant
folding ADDs, but failing to take into account the legality of doing that.

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index 293350f..fe6437b 100644 (file)
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -1215,15 +1215,18 @@ AGAIN:
                 break;
             }
 
-            if (op1->AsOp()->gtOp2->IsIntCnsFitsInI32() &&
-                FitsIn<INT32>(cns + op1->AsOp()->gtOp2->AsIntCon()->gtIconVal))
+            if (op1->AsOp()->gtOp2->IsIntCnsFitsInI32())
             {
-                cns += op1->AsOp()->gtOp2->AsIntCon()->gtIconVal;
-                op1 = op1->AsOp()->gtOp1;
+                GenTreeIntCon* addConst = op1->AsOp()->gtOp2->AsIntCon();
 
-                goto AGAIN;
-            }
+                if (addConst->ImmedValCanBeFolded(compiler, GT_ADD) && FitsIn<INT32>(cns + addConst->IconValue()))
+                {
+                    cns += addConst->IconValue();
+                    op1 = op1->AsOp()->gtOp1;
 
+                    goto AGAIN;
+                }
+            }
             break;
 
         case GT_MUL:
@@ -1295,15 +1298,18 @@ AGAIN:
                 break;
             }
 
-            if (op2->AsOp()->gtOp2->IsIntCnsFitsInI32() &&
-                FitsIn<INT32>(cns + op2->AsOp()->gtOp2->AsIntCon()->gtIconVal))
+            if (op2->AsOp()->gtOp2->IsIntCnsFitsInI32())
             {
-                cns += op2->AsOp()->gtOp2->AsIntCon()->gtIconVal;
-                op2 = op2->AsOp()->gtOp1;
+                GenTreeIntCon* addConst = op2->AsOp()->gtOp2->AsIntCon();
+
+                if (addConst->ImmedValCanBeFolded(compiler, GT_ADD) && FitsIn<INT32>(cns + addConst->IconValue()))
+                {
+                    cns += addConst->IconValue();
+                    op2 = op2->AsOp()->gtOp1;
+                }
 
                 goto AGAIN;
             }
-
             break;
 
         case GT_MUL:

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 587ebc1..1458c21 100644 (file)
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -3611,29 +3611,34 @@ void Compiler::gtWalkOp(GenTree** op1WB, GenTree** op2WB, GenTree* base, bool co
     op1 = op1->gtEffectiveVal();
 
     // Now we look for op1's with non-overflow GT_ADDs [of constants]
-    while ((op1->gtOper == GT_ADD) && (!op1->gtOverflow()) && (!constOnly || (op1->AsOp()->gtOp2->IsCnsIntOrI())))
+    while (op1->OperIs(GT_ADD) && !op1->gtOverflow())
     {
+        GenTreeOp* add    = op1->AsOp();
+        GenTree*   addOp1 = add->gtGetOp1();
+        GenTree*   addOp2 = add->gtGetOp2();
+
+        if (constOnly && (!addOp2->IsCnsIntOrI() || !addOp2->AsIntCon()->ImmedValCanBeFolded(this, GT_ADD)))
+        {
+            break;
+        }
+
         // mark it with GTF_ADDRMODE_NO_CSE
-        op1->gtFlags |= GTF_ADDRMODE_NO_CSE;
+        add->gtFlags |= GTF_ADDRMODE_NO_CSE;
 
         if (!constOnly)
         {
-            op2 = op1->AsOp()->gtOp2;
+            op2 = addOp2;
         }
-        op1 = op1->AsOp()->gtOp1;
+        op1 = addOp1;
 
         // If op1 is a GT_NOP then swap op1 and op2.
         // (Why? Also, presumably op2 is not a GT_NOP in this case?)
-        if (op1->gtOper == GT_NOP)
+        if (op1->OperIs(GT_NOP))
         {
-            GenTree* tmp;
-
-            tmp = op1;
-            op1 = op2;
-            op2 = tmp;
+            std::swap(op1, op2);
         }
 
-        if (!constOnly && ((op2 == base) || (!op2->IsCnsIntOrI())))
+        if (!constOnly && ((op2 == base) || !op2->IsCnsIntOrI() || !op2->AsIntCon()->ImmedValCanBeFolded(this, GT_ADD)))
         {
             break;
         }

diff --git a/src/tests/JIT/Directed/ConstantFolding/HandlesInAddrModes.il b/src/tests/JIT/Directed/ConstantFolding/HandlesInAddrModes.il
new file mode 100644 (file)
index 0000000..17c1d4c
--- /dev/null
+++ b/src/tests/JIT/Directed/ConstantFolding/HandlesInAddrModes.il
@@ -0,0 +1,35 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+.assembly HandlesInAddrModes { }
+.assembly extern System.Runtime { }
+
+.class HandlesInAddrModes
+{
+  .method public static int32 Main()
+  {
+    .entrypoint
+
+    ldc.i4 0
+    conv.i
+    call char .this::Problem(native int)
+    ldc.i4 120 // 'x'
+    ceq
+    ldc.i4 100
+    mul
+    ret
+  }
+
+  .method private static char Problem(native int idx) noinlining
+  {
+    ldstr "xyz"
+    call instance char& modreq([System.Runtime]System.Runtime.InteropServices.InAttribute) string::GetPinnableReference()
+    ldarg idx
+    ldc.i4 2
+    conv.i
+    mul
+    add
+    ldind.u2
+    ret
+  }
+}

diff --git a/src/tests/JIT/Directed/ConstantFolding/HandlesInAddrModes.ilproj b/src/tests/JIT/Directed/ConstantFolding/HandlesInAddrModes.ilproj
new file mode 100644 (file)
index 0000000..bcaa593
--- /dev/null
+++ b/src/tests/JIT/Directed/ConstantFolding/HandlesInAddrModes.ilproj
@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk.IL">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+</Project>

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
index 08d7e7a..87b1c54 100644 (file)
--- 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
@@ -1,3 +1,6 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
 public class FoldingExtendsInt32On64BitHostsTest
 {
     // On 64 bit hosts, 32 bit constants are stored as 64 bit signed values.