[release/7.0] JIT: fix incorrect scale in genCreateAddrMode + no-opt (#…

…75560)

* Fix invalid addressing mode scale in debug mode

* Add a test

* Update Runtime_75312.il

* Update Runtime_75312.ilproj

* Drop unneeded initialization

* Slightly better fix

* Test an even better fix?

* Refactoring

* address feedback

Co-authored-by: EgorBo <egorbo@gmail.com>

src/coreclr/jit/codegencommon.cpp

@@ -1370,65 +1370,46 @@ bool CodeGen::genCreateAddrMode(
 
     if (rv2)
     {
-        /* Make sure a GC address doesn't end up in 'rv2' */
-
+        // Make sure a GC address doesn't end up in 'rv2'
         if (varTypeIsGC(rv2->TypeGet()))
         {
             noway_assert(rv1 && !varTypeIsGC(rv1->TypeGet()));
-
-            tmp = rv1;
-            rv1 = rv2;
-            rv2 = tmp;
-
+            std::swap(rv1, rv2);
             rev = !rev;
         }
 
-        /* Special case: constant array index (that is range-checked) */
-
+        // Special case: constant array index (that is range-checked)
         if (fold)
         {
-            ssize_t  tmpMul;
-            GenTree* index;
+            // By default, assume index is rv2 and indexScale is mul (or 1 if mul is zero)
+            GenTree* index      = rv2;
+            ssize_t  indexScale = mul == 0 ? 1 : mul;
 
-            if ((rv2->gtOper == GT_MUL || rv2->gtOper == GT_LSH) && (rv2->AsOp()->gtOp2->IsCnsIntOrI()))
+            if (rv2->OperIs(GT_MUL, GT_LSH) && (rv2->gtGetOp2()->IsCnsIntOrI()))
             {
-                /* For valuetype arrays where we can't use the scaled address
-                   mode, rv2 will point to the scaled index. So we have to do
-                   more work */
-
-                tmpMul = compiler->optGetArrayRefScaleAndIndex(rv2, &index DEBUGARG(false));
-                if (mul)
-                {
-                    tmpMul *= mul;
-                }
+                indexScale *= compiler->optGetArrayRefScaleAndIndex(rv2, &index DEBUGARG(false));
             }
-            else
-            {
-                /* May be a simple array. rv2 will points to the actual index */
 
-                index  = rv2;
-                tmpMul = mul;
+            // "index * 0" means index is zero
+            if (indexScale == 0)
+            {
+                mul = 0;
+                rv2 = nullptr;
             }
-
-            /* Get hold of the array index and see if it's a constant */
-            if (index->IsIntCnsFitsInI32())
+            else if (index->IsIntCnsFitsInI32())
             {
-                /* Get hold of the index value */
-                ssize_t ixv = index->AsIntConCommon()->IconValue();
-
-                /* Scale the index if necessary */
-                if (tmpMul)
+                ssize_t constantIndex = index->AsIntConCommon()->IconValue() * indexScale;
+                if (constantIndex == 0)
                 {
-                    ixv *= tmpMul;
+                    // while scale is a non-zero constant, the actual index is zero so drop it
+                    mul = 0;
+                    rv2 = nullptr;
                 }
-
-                if (FitsIn<INT32>(cns + ixv))
+                else if (FitsIn<INT32>(cns + constantIndex))
                 {
-                    /* Add the scaled index to the offset value */
-
-                    cns += ixv;
-
-                    /* There is no scaled operand any more */
+                    // Add the constant index to the accumulated offset value
+                    cns += constantIndex;
+                    // and get rid of index
                     mul = 0;
                     rv2 = nullptr;
                 }

src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.il

@@ -0,0 +1,52 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+.assembly extern System.Runtime {}
+.assembly extern System.Runtime.Extensions {}
+.assembly extern System.Console {}
+.assembly Runtime_75312 {}
+
+.class public abstract auto ansi sealed beforefieldinit Runtime_75312
+       extends [System.Runtime]System.Object
+{
+    .method private hidebysig static 
+        int32 Main () cil managed 
+    {
+        .entrypoint
+        .maxstack 2
+        .locals init (
+            [0] int64 a
+        )
+        ldc.i8 1234605616436508552
+        stloc.0
+        ldc.i4 1146447579
+        ldloca.s 0
+        conv.u
+        newobj instance void [System.Runtime]System.IntPtr::.ctor(void*)
+        call int32 [System.Runtime]System.IntPtr::op_Explicit(native int)
+        call int32 Runtime_75312::Test(int32)
+        sub
+        ret
+    }
+
+    .method private hidebysig static int32 Test (int32 lcl) cil managed noinlining nooptimization
+    {
+        .maxstack 8
+
+        // return *(int*)(arg0 + ((3 * 0) << 2) + 1);
+        // to avoid constant folding in Roslyn (even for Debug) it's written in IL
+
+        ldarg.0
+        ldc.i4.3
+        ldc.i4.0
+        mul
+        ldc.i4.2
+        shl
+        add
+        ldc.i4.1
+        add
+        conv.i
+        ldind.i4
+        ret
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_75312/Runtime_75312.ilproj

@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk.IL">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestTargetUnsupported Condition="'$(TargetBits)' != '32'">true</CLRTestTargetUnsupported>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+</Project>