Use the latest base register while accessing the stack (#38834)

* logging

* the fix

* revert lclvars.cpp changes

* Revert "revert lclvars.cpp changes"

This reverts commit d39af7084687e8fe5e6d4f71674ec84d36a88340.

* wip

* revert lclvars.cpp changes

* deleted inst_RV_ST()

* removing logging, added some asserts

* jit-formatting

* add back case of INS_add and some more asserts

* reset lclvars.cpp

* delete comments and cleanup code

* revert changes inside common.il

* Revert "Disable failing Windows arm32 tests (#38844)"

This reverts commit 311fd81.

* review comments

* added standard function header

Author: kunalspathak

src/coreclr/src/jit/codegen.h

@@ -1406,8 +1406,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     void inst_SA_RV(instruction ins, unsigned ofs, regNumber reg, var_types type);
     void inst_SA_IV(instruction ins, unsigned ofs, int val, var_types type);
 
-    void inst_RV_ST(
-        instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size = EA_UNKNOWN);
     void inst_FS_ST(instruction ins, emitAttr size, TempDsc* tmp, unsigned ofs);
 
     void inst_TT(instruction ins, GenTree* tree, unsigned offs = 0, int shfv = 0, emitAttr size = EA_UNKNOWN);

src/coreclr/src/jit/emitarm.cpp

@@ -3510,11 +3510,22 @@ void emitter::emitIns_S(instruction ins, emitAttr attr, int varx, int offs)
     NYI("emitIns_S");
 }
 
-/*****************************************************************************
- *
- *  Add an instruction referencing a register and a stack-based local variable.
- */
-void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs)
+//-------------------------------------------------------------------------------------
+// emitIns_R_S: Add an instruction referencing a register and a stack-based local variable.
+//
+// Arguments:
+//    ins      - The instruction to add.
+//    attr     - Oeration size.
+//    varx     - The variable to generate offset for.
+//    offs     - The offset of variable or field in stack.
+//    pBaseReg - The base register that is used while calculating the offset. For example, if the offset
+//               with "stack pointer" can't be encoded in instruction, "frame pointer" can be used to get
+//               the offset of the field. In such case, pBaseReg will store the "fp".
+//
+// Return Value:
+//    The pBaseReg that holds the base register that was used to calculate the offset.
+//
+void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs, regNumber* pBaseReg)
 {
     if (ins == INS_mov)
     {
@@ -3547,6 +3558,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
     insFormat fmt = IF_NONE;
     insFlags  sf  = INS_FLAGS_NOT_SET;
     regNumber reg2;
+    regNumber baseRegUsed;
 
     /* Figure out the variable's frame position */
     int      base;
@@ -3555,6 +3567,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
 
     base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, &reg2, offs,
                                      CodeGen::instIsFP(ins));
+    if (pBaseReg != nullptr)
+    {
+        *pBaseReg = reg2;
+    }
 
     disp   = base + offs;
     undisp = unsigned_abs(disp);
@@ -3575,8 +3591,8 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
         else
         {
             regNumber rsvdReg = codeGen->rsGetRsvdReg();
-            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true);
-            emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2);
+            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed);
+            emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, baseRegUsed);
             emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0);
             return;
         }
@@ -3599,8 +3615,11 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
         {
             // Load disp into a register
             regNumber rsvdReg = codeGen->rsGetRsvdReg();
-            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false);
+            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed);
             fmt = IF_T2_E0;
+
+            // Ensure the baseReg calculated is correct.
+            assert(baseRegUsed == reg2);
         }
     }
     else if (ins == INS_add)
@@ -3625,7 +3644,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
         {
             // Load disp into a register
             regNumber rsvdReg = codeGen->rsGetRsvdReg();
-            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false);
+            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed);
+
+            // Ensure the baseReg calculated is correct.
+            assert(baseRegUsed == reg2);
             emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg);
             return;
         }
@@ -3662,8 +3684,24 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va
     appendToCurIG(id);
 }
 
-// generate the offset of &varx + offs into a register
-void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage)
+//-------------------------------------------------------------------------------------
+// emitIns_genStackOffset: Generate the offset of &varx + offs into a register
+//
+// Arguments:
+//    r            - Register in which offset calculation result is stored.
+//    varx         - The variable to generate offset for.
+//    offs         - The offset of variable or field in stack.
+//    isFloatUsage - True if the instruction being generated is a floating point instruction. This requires using
+//                   floating-point offset restrictions. Note that a variable can be non-float, e.g., struct, but
+//                   accessed as a float local field.
+//    pBaseReg     - The base register that is used while calculating the offset. For example, if the offset with
+//                   "stack pointer" can't be encoded in instruction, "frame pointer" can be used to get the offset
+//                   of the field. In such case, pBaseReg will store the "fp".
+//
+// Return Value:
+//    The pBaseReg that holds the base register that was used to calculate the offset.
+//
+void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg)
 {
     regNumber regBase;
     int       base;
@@ -3673,11 +3711,13 @@ void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFlo
         emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, &regBase, offs, isFloatUsage);
     disp = base + offs;
 
-    emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs);
+    emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, pBaseReg);
 
     if ((disp & 0xffff) != disp)
     {
-        emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs);
+        regNumber regBaseUsedInMovT;
+        emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs, &regBaseUsedInMovT);
+        assert(*pBaseReg == regBaseUsedInMovT);
     }
 }
 
@@ -3708,6 +3748,7 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va
     insFormat fmt = IF_NONE;
     insFlags  sf  = INS_FLAGS_NOT_SET;
     regNumber reg2;
+    regNumber baseRegUsed;
 
     /* Figure out the variable's frame position */
     int      base;
@@ -3736,7 +3777,10 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va
         else
         {
             regNumber rsvdReg = codeGen->rsGetRsvdReg();
-            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true);
+            emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed);
+
+            // Ensure the baseReg calculated is correct.
+            assert(baseRegUsed == reg2);
             emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2);
             emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0);
             return;
@@ -3758,8 +3802,11 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va
     {
         // Load disp into a register
         regNumber rsvdReg = codeGen->rsGetRsvdReg();
-        emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false);
+        emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed);
         fmt = IF_T2_E0;
+
+        // Ensure the baseReg calculated is correct.
+        assert(baseRegUsed == reg2);
     }
     assert((fmt == IF_T1_J2) || (fmt == IF_T2_E0) || (fmt == IF_T2_H0) || (fmt == IF_T2_VLDST) || (fmt == IF_T2_K1));
     assert(sf != INS_FLAGS_DONT_CARE);

src/coreclr/src/jit/emitarm.h

@@ -267,11 +267,11 @@ void emitIns_C(instruction ins, emitAttr attr, CORINFO_FIELD_HANDLE fdlHnd, int
 
 void emitIns_S(instruction ins, emitAttr attr, int varx, int offs);
 
-void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage);
+void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg);
 
 void emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs);
 
-void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs);
+void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs, regNumber* pBaseReg = nullptr);
 
 void emitIns_S_I(instruction ins, emitAttr attr, int varx, int offs, int val);
 

src/coreclr/src/jit/instr.cpp

@@ -1308,40 +1308,6 @@ void CodeGen::inst_RV_ST(instruction ins, emitAttr size, regNumber reg, GenTree*
     inst_RV_TT(ins, reg, tree, 0, size);
 }
 
-void CodeGen::inst_RV_ST(instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size)
-{
-    if (size == EA_UNKNOWN)
-    {
-        size = emitActualTypeSize(type);
-    }
-
-#ifdef TARGET_ARM
-    switch (ins)
-    {
-        case INS_mov:
-            assert(!"Please call ins_Load(type) to get the load instruction");
-            break;
-
-        case INS_add:
-        case INS_ldr:
-        case INS_ldrh:
-        case INS_ldrb:
-        case INS_ldrsh:
-        case INS_ldrsb:
-        case INS_lea:
-        case INS_vldr:
-            GetEmitter()->emitIns_R_S(ins, size, reg, tmp->tdTempNum(), ofs);
-            break;
-
-        default:
-            assert(!"Default inst_RV_ST case not supported for Arm");
-            break;
-    }
-#else  // !TARGET_ARM
-    GetEmitter()->emitIns_R_S(ins, size, reg, tmp->tdTempNum(), ofs);
-#endif // !TARGET_ARM
-}
-
 void CodeGen::inst_mov_RV_ST(regNumber reg, GenTree* tree)
 {
     /* Figure out the size of the value being loaded */

src/coreclr/tests/issues.targets

@@ -560,18 +560,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i10/*">
             <Issue>https://github.com/dotnet/runtime/issues/12979</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i74/*">
-            <Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i75/*">
-            <Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i76/*">
-            <Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/jit64/mcc/interop/mcc_i77/*">
-            <Issue>https://github.com/dotnet/runtime/issues/35501</Issue>
-        </ExcludeList>
     </ItemGroup>
 
     <!-- Windows arm64 specific excludes -->