Merge pull request #20527 from BruceForstall/FixPspSymMove

Stop attempting to move PSPSym during localloc for arm32/arm64

4 files changed, 36 insertions, 141 deletions

diff --git a/src/jit/codegenarm.cpp b/src/jit/codegenarm.cpp
index b198da5a36..1e6044fc97 100644
--- a/src/jit/codegenarm.cpp
+++ b/src/jit/codegenarm.cpp
@@ -263,6 +263,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* treeNode)
 void CodeGen::genLclHeap(GenTree* tree)
 {
     assert(tree->OperGet() == GT_LCLHEAP);
+    assert(compiler->compLocallocUsed);
 
     GenTree* size = tree->gtOp.gtOp1;
     noway_assert((genActualType(size->gtType) == TYP_INT) || (genActualType(size->gtType) == TYP_I_IMPL));
@@ -271,11 +272,9 @@ void CodeGen::genLclHeap(GenTree* tree)
     // Also it used as temporary register in code generation
     // for storing allocation size
     regNumber   regCnt          = tree->gtRegNum;
-    regNumber   pspSymReg       = REG_NA;
     var_types   type            = genActualType(size->gtType);
     emitAttr    easz            = emitTypeSize(type);
     BasicBlock* endLabel        = nullptr;
-    BasicBlock* loop            = nullptr;
     unsigned    stackAdjustment = 0;
 
 #ifdef DEBUG
@@ -298,14 +297,6 @@ void CodeGen::genLclHeap(GenTree* tree)
     noway_assert(isFramePointerUsed()); // localloc requires Frame Pointer to be established since SP changes
     noway_assert(genStackLevel == 0);   // Can't have anything on the stack
 
-    // Whether method has PSPSym.
-    bool hasPspSym;
-#if FEATURE_EH_FUNCLETS
-    hasPspSym = (compiler->lvaPSPSym != BAD_VAR_NUM);
-#else
-    hasPspSym = false;
-#endif
-
     // Check to 0 size allocations
     // size_t amount = 0;
     if (size->IsCnsIntOrI())
@@ -332,19 +323,7 @@ void CodeGen::genLclHeap(GenTree* tree)
     }
 
     stackAdjustment = 0;
-#if FEATURE_EH_FUNCLETS
-    // If we have PSPsym, then need to re-locate it after localloc.
-    if (hasPspSym)
-    {
-        stackAdjustment += STACK_ALIGN;
-
-        // Save a copy of PSPSym
-        pspSymReg = tree->ExtractTempReg();
-        getEmitter()->emitIns_R_S(ins_Load(TYP_I_IMPL), EA_PTRSIZE, pspSymReg, compiler->lvaPSPSym, 0);
-    }
-#endif
 
-#if FEATURE_FIXED_OUT_ARGS
     // If we have an outgoing arg area then we must adjust the SP by popping off the
     // outgoing arg area. We will restore it right before we return from this method.
     if (compiler->lvaOutgoingArgSpaceSize > 0)
@@ -354,7 +333,6 @@ void CodeGen::genLclHeap(GenTree* tree)
         inst_RV_IV(INS_add, REG_SPBASE, compiler->lvaOutgoingArgSpaceSize, EA_PTRSIZE);
         stackAdjustment += compiler->lvaOutgoingArgSpaceSize;
     }
-#endif
 
     // Put aligned allocation size to regCnt
     if (size->IsCnsIntOrI())
@@ -382,7 +360,7 @@ void CodeGen::genLclHeap(GenTree* tree)
         }
         else if (!compiler->info.compInitMem && (amount < compiler->eeGetPageSize())) // must be < not <=
         {
-            // Since the size is a page or less, simply adjust the SP value
+            // Since the size is less than a page, simply adjust the SP value.
             // The SP might already be in the guard page, must touch it BEFORE
             // the alloc, not after.
             getEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, regCnt, REG_SP, 0);
@@ -403,8 +381,8 @@ void CodeGen::genLclHeap(GenTree* tree)
     // Allocation
     if (compiler->info.compInitMem)
     {
-        // At this point 'regCnt' is set to the total number of bytes to locAlloc.
-        // Since we have to zero out the allocated memory AND ensure that RSP is always valid
+        // At this point 'regCnt' is set to the total number of bytes to localloc.
+        // Since we have to zero out the allocated memory AND ensure that the stack pointer is always valid
         // by tickling the pages, we will just push 0's on the stack.
 
         regNumber regTmp = tree->ExtractTempReg();
@@ -498,21 +476,13 @@ void CodeGen::genLclHeap(GenTree* tree)
     }
 
 ALLOC_DONE:
-    // Re-adjust SP to allocate PSPSym and out-going arg area
+    // Re-adjust SP to allocate out-going arg area
     if (stackAdjustment != 0)
     {
         assert((stackAdjustment % STACK_ALIGN) == 0); // This must be true for the stack to remain aligned
         assert(stackAdjustment > 0);
         getEmitter()->emitIns_R_R_I(INS_sub, EA_PTRSIZE, REG_SPBASE, REG_SPBASE, (int)stackAdjustment);
 
-#if FEATURE_EH_FUNCLETS
-        // Write PSPSym to its new location.
-        if (hasPspSym)
-        {
-            assert(genIsValidIntReg(pspSymReg));
-            getEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, pspSymReg, compiler->lvaPSPSym, 0);
-        }
-#endif
         // Return the stackalloc'ed address in result register.
         // regCnt = RSP + stackAdjustment.
         getEmitter()->emitIns_R_R_I(INS_add, EA_PTRSIZE, regCnt, REG_SPBASE, (int)stackAdjustment);
@@ -541,7 +511,7 @@ BAILOUT:
         noway_assert(compiler->lvaReturnEspCheck != 0xCCCCCCCC &&
                      compiler->lvaTable[compiler->lvaReturnEspCheck].lvDoNotEnregister &&
                      compiler->lvaTable[compiler->lvaReturnEspCheck].lvOnFrame);
-        getEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, regCnt, compiler->lvaReturnEspCheck, 0);
+        getEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, REG_SPBASE, compiler->lvaReturnEspCheck, 0);
     }
 #endif
 
diff --git a/src/jit/codegenarm64.cpp b/src/jit/codegenarm64.cpp
index 6aad5d957a..b2a793b156 100644
--- a/src/jit/codegenarm64.cpp
+++ b/src/jit/codegenarm64.cpp
@@ -1831,6 +1831,7 @@ void CodeGen::genSimpleReturn(GenTree* treeNode)
 void CodeGen::genLclHeap(GenTree* tree)
 {
     assert(tree->OperGet() == GT_LCLHEAP);
+    assert(compiler->compLocallocUsed);
 
     GenTree* size = tree->gtOp.gtOp1;
     noway_assert((genActualType(size->gtType) == TYP_INT) || (genActualType(size->gtType) == TYP_I_IMPL));
@@ -1864,14 +1865,6 @@ void CodeGen::genLclHeap(GenTree* tree)
     noway_assert(isFramePointerUsed()); // localloc requires Frame Pointer to be established since SP changes
     noway_assert(genStackLevel == 0);   // Can't have anything on the stack
 
-    // Whether method has PSPSym.
-    bool hasPspSym;
-#if FEATURE_EH_FUNCLETS
-    hasPspSym = (compiler->lvaPSPSym != BAD_VAR_NUM);
-#else
-    hasPspSym = false;
-#endif
-
     // compute the amount of memory to allocate to properly STACK_ALIGN.
     size_t amount = 0;
     if (size->IsCnsIntOrI())
@@ -1900,9 +1893,9 @@ void CodeGen::genLclHeap(GenTree* tree)
         inst_JMP(jmpEqual, endLabel);
 
         // Compute the size of the block to allocate and perform alignment.
-        // If the method has no PSPSym and compInitMem=true, we can reuse targetReg as regcnt,
+        // If compInitMem=true, we can reuse targetReg as regcnt,
         // since we don't need any internal registers.
-        if (!hasPspSym && compiler->info.compInitMem)
+        if (compiler->info.compInitMem)
         {
             assert(tree->AvailableTempRegCount() == 0);
             regCnt = targetReg;
@@ -1923,34 +1916,18 @@ void CodeGen::genLclHeap(GenTree* tree)
     }
 
     stackAdjustment = 0;
-#if FEATURE_EH_FUNCLETS
-    // If we have PSPsym, then need to re-locate it after localloc.
-    if (hasPspSym)
-    {
-        stackAdjustment += STACK_ALIGN;
-
-        // Save a copy of PSPSym
-        pspSymReg = tree->ExtractTempReg();
-        getEmitter()->emitIns_R_S(ins_Load(TYP_I_IMPL), EA_PTRSIZE, pspSymReg, compiler->lvaPSPSym, 0);
-    }
-#endif
 
-#if FEATURE_FIXED_OUT_ARGS
     // If we have an outgoing arg area then we must adjust the SP by popping off the
     // outgoing arg area. We will restore it right before we return from this method.
     //
-    // Localloc is supposed to return stack space that is STACK_ALIGN'ed.  The following
-    // are the cases that needs to be handled:
-    //   i) Method has PSPSym + out-going arg area.
-    //      It is guaranteed that size of out-going arg area is STACK_ALIGNED (see fgMorphArgs).
-    //      Therefore, we will pop-off RSP upto out-going arg area before locallocating.
-    //      We need to add padding to ensure RSP is STACK_ALIGN'ed while re-locating PSPSym + arg area.
-    //  ii) Method has no PSPSym but out-going arg area.
-    //      Almost same case as above without the requirement to pad for the final RSP to be STACK_ALIGN'ed.
-    // iii) Method has PSPSym but no out-going arg area.
-    //      Nothing to pop-off from the stack but needs to relocate PSPSym with SP padded.
-    //  iv) Method has neither PSPSym nor out-going arg area.
-    //      Nothing needs to popped off from stack nor relocated.
+    // Localloc returns stack space that aligned to STACK_ALIGN bytes. The following
+    // are the cases that need to be handled:
+    //   i) Method has out-going arg area.
+    //      It is guaranteed that size of out-going arg area is STACK_ALIGN'ed (see fgMorphArgs).
+    //      Therefore, we will pop off the out-going arg area from the stack pointer before allocating the localloc
+    //      space.
+    //  ii) Method has no out-going arg area.
+    //      Nothing to pop off from the stack.
     if (compiler->lvaOutgoingArgSpaceSize > 0)
     {
         assert((compiler->lvaOutgoingArgSpaceSize % STACK_ALIGN) == 0); // This must be true for the stack to remain
@@ -1958,7 +1935,6 @@ void CodeGen::genLclHeap(GenTree* tree)
         inst_RV_IV(INS_add, REG_SPBASE, compiler->lvaOutgoingArgSpaceSize, EA_PTRSIZE);
         stackAdjustment += compiler->lvaOutgoingArgSpaceSize;
     }
-#endif
 
     if (size->IsCnsIntOrI())
     {
@@ -1983,9 +1959,10 @@ void CodeGen::genLclHeap(GenTree* tree)
         }
         else if (!compiler->info.compInitMem && (amount < compiler->eeGetPageSize())) // must be < not <=
         {
-            // Since the size is a page or less, simply adjust the SP value
-            // The SP might already be in the guard page, must touch it BEFORE
+            // Since the size is less than a page, simply adjust the SP value.
+            // The SP might already be in the guard page, so we must touch it BEFORE
             // the alloc, not after.
+
             // ldr wz, [SP, #0]
             getEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, REG_ZR, REG_SP, 0);
 
@@ -1995,10 +1972,10 @@ void CodeGen::genLclHeap(GenTree* tree)
         }
 
         // else, "mov regCnt, amount"
-        // If the method has no PSPSym and compInitMem=true, we can reuse targetReg as regcnt.
+        // If compInitMem=true, we can reuse targetReg as regcnt.
         // Since size is a constant, regCnt is not yet initialized.
         assert(regCnt == REG_NA);
-        if (!hasPspSym && compiler->info.compInitMem)
+        if (compiler->info.compInitMem)
         {
             assert(tree->AvailableTempRegCount() == 0);
             regCnt = targetReg;
@@ -2015,7 +1992,7 @@ void CodeGen::genLclHeap(GenTree* tree)
         BasicBlock* loop = genCreateTempLabel();
 
         // At this point 'regCnt' is set to the total number of bytes to locAlloc.
-        // Since we have to zero out the allocated memory AND ensure that RSP is always valid
+        // Since we have to zero out the allocated memory AND ensure that the stack pointer is always valid
         // by tickling the pages, we will just push 0's on the stack.
         //
         // Note: regCnt is guaranteed to be even on Amd64 since STACK_ALIGN/TARGET_POINTER_SIZE = 2
@@ -2038,7 +2015,7 @@ void CodeGen::genLclHeap(GenTree* tree)
     }
     else
     {
-        // At this point 'regCnt' is set to the total number of bytes to locAlloc.
+        // At this point 'regCnt' is set to the total number of bytes to localloc.
         //
         // We don't need to zero out the allocated memory. However, we do have
         // to tickle the pages to ensure that SP is always valid and is
@@ -2109,23 +2086,15 @@ void CodeGen::genLclHeap(GenTree* tree)
     }
 
 ALLOC_DONE:
-    // Re-adjust SP to allocate PSPSym and out-going arg area
+    // Re-adjust SP to allocate out-going arg area
     if (stackAdjustment != 0)
     {
         assert((stackAdjustment % STACK_ALIGN) == 0); // This must be true for the stack to remain aligned
         assert(stackAdjustment > 0);
         getEmitter()->emitIns_R_R_I(INS_sub, EA_PTRSIZE, REG_SPBASE, REG_SPBASE, (int)stackAdjustment);
 
-#if FEATURE_EH_FUNCLETS
-        // Write PSPSym to its new location.
-        if (hasPspSym)
-        {
-            assert(genIsValidIntReg(pspSymReg));
-            getEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE, pspSymReg, compiler->lvaPSPSym, 0);
-        }
-#endif
         // Return the stackalloc'ed address in result register.
-        // TargetReg = RSP + stackAdjustment.
+        // TargetReg = SP + stackAdjustment.
         //
         getEmitter()->emitIns_R_R_I(INS_add, EA_PTRSIZE, targetReg, REG_SPBASE, (int)stackAdjustment);
     }
@@ -2248,7 +2217,7 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree)
                 {
                     GenTreeIntConCommon* intConstTree  = divisorOp->AsIntConCommon();
                     ssize_t              intConstValue = intConstTree->IconValue();
-                    assert(intConstValue != 0); // already checked above by IsIntegralConst(0))
+                    assert(intConstValue != 0); // already checked above by IsIntegralConst(0)
                     if (intConstValue != -1)
                     {
                         checkDividend = false; // We statically know that the dividend is not -1
diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp
index 980b2a3d64..f64b7fb3f0 100644
--- a/src/jit/lsraarm.cpp
+++ b/src/jit/lsraarm.cpp
@@ -36,18 +36,11 @@ int LinearScan::BuildLclHeap(GenTree* tree)
     //
     //  Size?                   Init Memory?    # temp regs
     //   0                          -               0
-    //   const and <=4 str instr    -             hasPspSym ? 1 : 0
-    //   const and <PageSize        No            hasPspSym ? 1 : 0
-    //   >4 ptr words               Yes           hasPspSym ? 2 : 1
-    //   Non-const                  Yes           hasPspSym ? 2 : 1
-    //   Non-const                  No            hasPspSym ? 2 : 1
-
-    bool hasPspSym;
-#if FEATURE_EH_FUNCLETS
-    hasPspSym = (compiler->lvaPSPSym != BAD_VAR_NUM);
-#else
-    hasPspSym = false;
-#endif
+    //   const and <=4 str instr    -               0
+    //   const and <PageSize        No              0
+    //   >4 ptr words               Yes             1
+    //   Non-const                  Yes             1
+    //   Non-const                  No              1
 
     GenTree* size = tree->gtGetOp1();
     int      internalIntCount;
@@ -87,18 +80,13 @@ int LinearScan::BuildLclHeap(GenTree* tree)
             {
                 internalIntCount = 1;
             }
-
-            if (hasPspSym)
-            {
-                internalIntCount++;
-            }
         }
     }
     else
     {
-        // target (regCnt) + tmp + [psp]
+        // target (regCnt) + tmp
         srcCount         = 1;
-        internalIntCount = hasPspSym ? 2 : 1;
+        internalIntCount = 1;
         BuildUse(size);
     }
 
diff --git a/src/jit/lsraarm64.cpp b/src/jit/lsraarm64.cpp
index 76b56b3688..d5446362fe 100644
--- a/src/jit/lsraarm64.cpp
+++ b/src/jit/lsraarm64.cpp
@@ -522,18 +522,10 @@ int LinearScan::BuildNode(GenTree* tree)
             //   0                          -               0
             //   const and <=6 ptr words    -               0
             //   const and <PageSize        No              0
-            //   >6 ptr words               Yes           hasPspSym ? 1 : 0
-            //   Non-const                  Yes           hasPspSym ? 1 : 0
+            //   >6 ptr words               Yes             0
+            //   Non-const                  Yes             0
             //   Non-const                  No              2
             //
-            // PSPSym - If the method has PSPSym increment internalIntCount by 1.
-            //
-            bool hasPspSym;
-#if FEATURE_EH_FUNCLETS
-            hasPspSym = (compiler->lvaPSPSym != BAD_VAR_NUM);
-#else
-            hasPspSym = false;
-#endif
 
             GenTree* size = tree->gtGetOp1();
             if (size->IsCnsIntOrI())
@@ -572,14 +564,6 @@ int LinearScan::BuildNode(GenTree* tree)
                             buildInternalIntRegisterDefForNode(tree);
                         }
                     }
-                    else if (hasPspSym)
-                    {
-                        // greater than 4 and need to zero initialize allocated stack space.
-                        // If the method has PSPSym, we need an internal register to hold regCnt
-                        // since targetReg allocated to GT_LCLHEAP node could be the same as one of
-                        // the the internal registers.
-                        buildInternalIntRegisterDefForNode(tree);
-                    }
                 }
             }
             else
@@ -590,24 +574,8 @@ int LinearScan::BuildNode(GenTree* tree)
                     buildInternalIntRegisterDefForNode(tree);
                     buildInternalIntRegisterDefForNode(tree);
                 }
-                else if (hasPspSym)
-                {
-                    // If the method has PSPSym, we need an internal register to hold regCnt
-                    // since targetReg allocated to GT_LCLHEAP node could be the same as one of
-                    // the the internal registers.
-                    buildInternalIntRegisterDefForNode(tree);
-                }
             }
 
-            // If the method has PSPSym, we need an additional register to relocate it on stack.
-            if (hasPspSym)
-            {
-                // Exclude const size 0
-                if (!size->IsCnsIntOrI() || (size->gtIntCon.gtIconVal > 0))
-                {
-                    buildInternalIntRegisterDefForNode(tree);
-                }
-            }
             if (!size->isContained())
             {
                 BuildUse(size);