Kill a non-gc-type lclVar if it has a gc-type value (#679)
authorCarol Eidt <carol.eidt@microsoft.com>
Wed, 11 Dec 2019 22:33:59 +0000 (14:33 -0800)
committerGitHub <noreply@github.com>
Wed, 11 Dec 2019 22:33:59 +0000 (14:33 -0800)
* Kill a non-gc-type lclVar if it has a gc-type value

src/coreclr/src/jit/lsra.cpp
src/coreclr/src/jit/lsra.h

index e0cb77e..7475529 100644 (file)
@@ -4410,7 +4410,7 @@ void LinearScan::unassignPhysReg(RegRecord* regRec, RefPosition* spillRefPositio
 }
 
 //------------------------------------------------------------------------
-// spillGCRefs: Spill any GC-type intervals that are currently in registers.a
+// spillGCRefs: Spill any GC-type intervals that are currently in registers.
 //
 // Arguments:
 //    killRefPosition - The RefPosition for the kill
@@ -4418,6 +4418,10 @@ void LinearScan::unassignPhysReg(RegRecord* regRec, RefPosition* spillRefPositio
 // Return Value:
 //    None.
 //
+// Notes:
+//    This is used to ensure that we have no live GC refs in registers at an
+//    unmanaged call.
+//
 void LinearScan::spillGCRefs(RefPosition* killRefPosition)
 {
     // For each physical register that can hold a GC type,
@@ -4430,12 +4434,29 @@ void LinearScan::spillGCRefs(RefPosition* killRefPosition)
         regNumber  nextReg          = genRegNumFromMask(nextRegBit);
         RegRecord* regRecord        = getRegisterRecord(nextReg);
         Interval*  assignedInterval = regRecord->assignedInterval;
-        if (assignedInterval == nullptr || (assignedInterval->isActive == false) ||
-            !varTypeIsGC(assignedInterval->registerType))
+        if (assignedInterval == nullptr || (assignedInterval->isActive == false))
         {
             continue;
         }
-        unassignPhysReg(regRecord, assignedInterval->recentRefPosition);
+        bool needsKill = varTypeIsGC(assignedInterval->registerType);
+        if (!needsKill)
+        {
+            // The importer will assign a GC type to the rhs of an assignment if the lhs type is a GC type,
+            // even if the rhs is not. See the CEE_STLOC* case in impImportBlockCode(). As a result,
+            // we can have a 'GT_LCL_VAR' node with a GC type, when the lclVar itself is an integer type.
+            // The emitter will mark this register as holding a GC type. Therfore we must spill this value.
+            // This was exposed on Arm32 with EH write-thru.
+            if ((assignedInterval->recentRefPosition != nullptr) &&
+                (assignedInterval->recentRefPosition->treeNode != nullptr))
+            {
+                needsKill = varTypeIsGC(assignedInterval->recentRefPosition->treeNode);
+            }
+        }
+        if (needsKill)
+        {
+            INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_KILL_GC_REF, nullptr, nextReg, nullptr));
+            unassignPhysReg(regRecord, assignedInterval->recentRefPosition);
+        }
     }
     INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DONE_KILL_GC_REFS, nullptr, REG_NA, nullptr));
 }
@@ -5190,7 +5211,8 @@ void LinearScan::allocateRegisters()
         printf("\n\nAllocating Registers\n"
                "--------------------\n");
         // Start with a small set of commonly used registers, so that we don't keep having to print a new title.
-        registersToDump = LsraLimitSmallIntSet | LsraLimitSmallFPSet;
+        // Include all the arg regs, as they may already have values assigned to them.
+        registersToDump = LsraLimitSmallIntSet | LsraLimitSmallFPSet | RBM_ARG_REGS;
         dumpRegRecordHeader();
         // Now print an empty "RefPosition", since we complete the dump of the regs at the beginning of the loop.
         printf(indentFormat, "");
@@ -9822,7 +9844,7 @@ void LinearScan::dumpRegRecordHeader()
            "action taken during allocation (e.g. Alloc a new register, or Keep an existing one).\n"
            "The subsequent columns show the Interval occupying each register, if any, followed by 'a' if it is\n"
            "active, a 'p' if it is a large vector that has been partially spilled, and 'i'if it is inactive.\n"
-           "Columns are only printed up to the last modifed register, which may increase during allocation,"
+           "Columns are only printed up to the last modifed register, which may increase during allocation,\n"
            "in which case additional columns will appear.  \n"
            "Registers which are not marked modified have ---- in their column.\n\n");
 
index 775d85e..3ea2fd4 100644 (file)
@@ -1290,7 +1290,7 @@ private:
 
         // Spilling
         LSRA_EVENT_SPILL, LSRA_EVENT_SPILL_EXTENDED_LIFETIME, LSRA_EVENT_RESTORE_PREVIOUS_INTERVAL,
-        LSRA_EVENT_RESTORE_PREVIOUS_INTERVAL_AFTER_SPILL, LSRA_EVENT_DONE_KILL_GC_REFS,
+        LSRA_EVENT_RESTORE_PREVIOUS_INTERVAL_AFTER_SPILL, LSRA_EVENT_KILL_GC_REF, LSRA_EVENT_DONE_KILL_GC_REFS,
 
         // Block boundaries
         LSRA_EVENT_START_BB, LSRA_EVENT_END_BB,