From 7ebb61eb79ac95b98b14f37b0cc7d1f52ee9deb4 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 11 Oct 2022 14:54:33 -0700 Subject: [PATCH 1/6] Check if interval interfers with the call --- src/coreclr/jit/clrjit.natvis | 2 +- src/coreclr/jit/lsra.cpp | 4 ++ src/coreclr/jit/lsra.h | 9 ++++ src/coreclr/jit/lsrabuild.cpp | 90 +++++++++++++++++++++++++++++++++-- 4 files changed, 99 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/clrjit.natvis b/src/coreclr/jit/clrjit.natvis index 5873fca508786b..140c7e4dec277d 100644 --- a/src/coreclr/jit/clrjit.natvis +++ b/src/coreclr/jit/clrjit.natvis @@ -153,7 +153,7 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u - [#{rpNum,d} - {refType,en}] + [{nodeLocation,d}. #{rpNum,d} - {refType,en}] (RegRecord*)this->referent (Interval*)this->referent diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 841b5bf0e796b9..871108042dd185 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -624,6 +624,8 @@ LinearScan::LinearScan(Compiler* theCompiler) , intervals(theCompiler->getAllocator(CMK_LSRA_Interval)) , allocationPassComplete(false) , refPositions(theCompiler->getAllocator(CMK_LSRA_RefPosition)) + , callRefPositionLocations(theCompiler->getAllocator(CMK_LSRA)) + , callKillMasks(theCompiler->getAllocator(CMK_LSRA)) , listNodePool(theCompiler) { regSelector = new (theCompiler, CMK_LSRA) RegisterSelection(this); @@ -703,6 +705,8 @@ LinearScan::LinearScan(Compiler* theCompiler) blockSequenceWorkList = nullptr; curBBSeqNum = 0; bbSeqCount = 0; + callRefPositionCount = 0; + recentRefPosition = nullptr; // Information about each block, including predecessor blocks used for variable locations at block entry. blockInfo = nullptr; diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index abb900c348b64d..d27a2c300ba5b9 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1582,6 +1582,15 @@ class LinearScan : public LinearScanInterface // Ordered list of RefPositions RefPositionList refPositions; + // Most recent refpostion assigned + RefPosition* recentRefPosition; + + // Ordered list of locations where CALL happens + jitstd::list callRefPositionLocations; + // Equivalent list of kill masks for those calls + jitstd::list callKillMasks; + int callRefPositionCount; + // Per-block variable location mappings: an array indexed by block number that yields a // pointer to an array of regNumber, one per variable. VarToRegMap* inVarToRegMaps; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 32cad825fe9e4d..73e173fc96d1a5 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -186,6 +186,7 @@ RefPosition* LinearScan::newRefPositionRaw(LsraLocation nodeLocation, GenTree* t currBuildNode = nullptr; newRP->rpNum = static_cast(refPositions.size() - 1); #endif // DEBUG + recentRefPosition = newRP; return newRP; } @@ -1141,8 +1142,9 @@ regMaskTP LinearScan::getKillSetForNode(GenTree* tree) // // Notes: // The return value is needed because if we have any kills, we need to make sure that -// all defs are located AFTER the kills. On the other hand, if there aren't kills, -// the multiple defs for a regPair are in different locations. +// all defs are located AFTER the kills. TODO: This is always the case. +// On the other hand, if there aren't kills, the multiple defs for a regPair are in +// different locations. // If we generate any kills, we will mark all currentLiveVars as being preferenced // to avoid the killed registers. This is somewhat conservative. // @@ -1167,6 +1169,18 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo // if (!blockSequence[curBBSeqNum]->isRunRarely()) if (enregisterLocalVars) { + const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH)); + if (isCallKill) + { + assert(recentRefPosition); + if (recentRefPosition->refType != RefTypeBB) + { + callRefPositionLocations.push_back(recentRefPosition->nodeLocation); + callKillMasks.push_back(killMask); + callRefPositionCount++; + } + } + VarSetOps::Iter iter(compiler, currentLiveVars); unsigned varIndex = 0; while (iter.NextElem(&varIndex)) @@ -1187,9 +1201,7 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo { continue; } - Interval* interval = getIntervalForLocalVar(varIndex); - const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH)); - + Interval* interval = getIntervalForLocalVar(varIndex); if (isCallKill) { interval->preferCalleeSave = true; @@ -2613,6 +2625,74 @@ void LinearScan::buildIntervals() } } + // Adjust preferCalleeSave + if (callRefPositionCount > 0) + { + LsraLocation firstCallLocation = callRefPositionLocations.front(); + LsraLocation lastCallLocation = callRefPositionLocations.back(); + + JITDUMP("\n\nCHECKING if Intervals can use callee-save registers"); + JITDUMP("\n==============================\n"); + for (Interval& interval : intervals) + { + if (interval.isLocalVar) + { + // We already handle them in BuildKillPositionsForNode. + continue; + } + + LsraLocation firstRef = interval.firstRefPosition->nodeLocation; + LsraLocation lastRef = interval.lastRefPosition->nodeLocation; + + if ((firstRef > lastCallLocation) || (lastRef < firstCallLocation)) + { + // Current interval doesn't interfer with any calls. Skip it. + continue; + } + + jitstd::list callKillMasksLocal = callKillMasks; + for (LsraLocation& callLocation : callRefPositionLocations) + { + regMaskTP killMask = callKillMasksLocal.front(); + callKillMasksLocal.pop_front(); + + if ((firstRef <= callLocation) && (callLocation < lastRef)) + { + interval.preferCalleeSave = true; + if (!interval.isWriteThru) + { + // We are more conservative about allocating callee-saves registers to write-thru vars, + // since a call only requires reloading after (not spilling before). So we record (above) + // the fact that we'd prefer a callee-save register, but we don't update the preferences at + // this point. See the "heuristics for writeThru intervals" in 'buildIntervals()'. + regMaskTP newPreferences = allRegs(interval.registerType) & ~killMask; + + if (newPreferences != RBM_NONE) + { + if (VERBOSE) + { + printf("Interval %2u: Update preferences (callee-save) from ", + interval.intervalIndex); + dumpRegMask(interval.registerPreferences); + printf(" to "); + dumpRegMask(newPreferences); + printf("\n"); + } + interval.updateRegisterPreferences(newPreferences); + } + } + break; + } + + if (lastRef < callLocation) + { + // We passed lastRef, no point in checking further callRefPositions. + break; + } + } + } + } + #ifdef DEBUG if (getLsraExtendLifeTimes()) { From 55e2c5d3cdd8e42324165591de639e8ab78761b9 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 11 Oct 2022 16:04:58 -0700 Subject: [PATCH 2/6] Wrap #ifdef --- src/coreclr/jit/lsrabuild.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 73e173fc96d1a5..d47fb620c00ebb 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2669,6 +2669,7 @@ void LinearScan::buildIntervals() if (newPreferences != RBM_NONE) { +#ifdef DEBUG if (VERBOSE) { printf("Interval %2u: Update preferences (callee-save) from ", @@ -2678,6 +2679,7 @@ void LinearScan::buildIntervals() dumpRegMask(newPreferences); printf("\n"); } +#endif interval.updateRegisterPreferences(newPreferences); } } From 245039dc27d9ab66639fe27dc3572fa29754d50a Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 13 Oct 2022 22:42:45 -0700 Subject: [PATCH 3/6] Faster way to find out if interval interferes with the call --- src/coreclr/jit/lsra.cpp | 15 ++++--- src/coreclr/jit/lsra.h | 1 + src/coreclr/jit/lsrabuild.cpp | 81 +++++++++++++++++++++++++++++++---- 3 files changed, 82 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 871108042dd185..fc1118434d00a4 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -700,13 +700,14 @@ LinearScan::LinearScan(Compiler* theCompiler) // Note that we don't initialize the bbVisitedSet until we do the first traversal // This is so that any blocks that are added during the first traversal // are accounted for (and we don't have BasicBlockEpoch issues). - blockSequencingDone = false; - blockSequence = nullptr; - blockSequenceWorkList = nullptr; - curBBSeqNum = 0; - bbSeqCount = 0; - callRefPositionCount = 0; - recentRefPosition = nullptr; + blockSequencingDone = false; + blockSequence = nullptr; + blockSequenceWorkList = nullptr; + curBBSeqNum = 0; + bbSeqCount = 0; + callRefPositionCount = 0; + recentRefPosition = nullptr; + recentCallRefPositionLocation = 0; // Information about each block, including predecessor blocks used for variable locations at block entry. blockInfo = nullptr; diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index d27a2c300ba5b9..fd132c65fb6dc1 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1586,6 +1586,7 @@ class LinearScan : public LinearScanInterface RefPosition* recentRefPosition; // Ordered list of locations where CALL happens + LsraLocation recentCallRefPositionLocation; jitstd::list callRefPositionLocations; // Equivalent list of kill masks for those calls jitstd::list callKillMasks; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index d47fb620c00ebb..96a9f04359d3fa 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -357,14 +357,13 @@ void LinearScan::resolveConflictingDefAndUse(Interval* interval, RefPosition* de } //------------------------------------------------------------------------ -// applyCalleeSaveHeuristics: Set register preferences for an interval based on the given RefPosition +// applyCalleeSaveHeuristics: Check if an interval should be set to prefer callee-save. +// This is done by checking if this interval interferes with any call (`recentCallRefPositionLocation`) +// and if yes, sets `preferCalleeSave = true`. // // Arguments: // rp - The RefPosition of interest // -// Notes: -// This is slightly more general than its name applies, and updates preferences not just -// for callee-save registers. // void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp) { @@ -376,15 +375,70 @@ void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp) } #endif // TARGET_AMD64 - Interval* theInterval = rp->getInterval(); + Interval* theInterval = rp->getInterval(); + regMaskTP calleeSavePreference = rp->registerAssignment; + + if (!theInterval->preferCalleeSave && !theInterval->isLocalVar && RefTypeIsUse(rp->refType)) + { + RefPosition* firstPosition = theInterval->firstRefPosition; + if ((firstPosition != nullptr) && (firstPosition->nodeLocation > 0)) + { + if (firstPosition->nodeLocation < recentCallRefPositionLocation) + { + JITDUMP("Interval %2u: Prefer callee-save because of presence of kills at location %d. First " + "RefPosition is at #%d @%d and current RefPosition is #%d @%d\n", + theInterval->intervalIndex, recentCallRefPositionLocation, firstPosition->rpNum, + firstPosition->nodeLocation, rp->rpNum, rp->nodeLocation); + + theInterval->preferCalleeSave = true; + if (!theInterval->isWriteThru) + { + regMaskTP killMask = RBM_CALLEE_TRASH; + // if there is no FP used, we can ignore the FP kills + if (!compiler->compFloatingPointUsed) + { + killMask &= ~RBM_FLT_CALLEE_TRASH; + } + + // We are more conservative about allocating callee-saves registers to write-thru vars, + // since a call only requires reloading after (not spilling before). So we record (above) + // the fact that we'd prefer a callee-save register, but we don't update the preferences at + // this point. See the "heuristics for writeThru intervals" in 'buildIntervals()'. + regMaskTP newPreferences = allRegs(theInterval->registerType) & (~killMask); +#ifdef DEBUG + if (VERBOSE) + { + JITDUMP("Interval %2u: Current preference= ", theInterval->intervalIndex); + dumpRegMask(theInterval->registerPreferences); + JITDUMP(", New preference= "); + dumpRegMask(newPreferences); + } +#endif + if (newPreferences != RBM_NONE) + { + calleeSavePreference = newPreferences; + } + } + } + } + } #ifdef DEBUG if (!doReverseCallerCallee()) #endif // DEBUG { // Set preferences so that this register set will be preferred for earlier refs - theInterval->mergeRegisterPreferences(rp->registerAssignment); + theInterval->mergeRegisterPreferences(calleeSavePreference); + } + +#ifdef DEBUG + if (VERBOSE) + { + JITDUMP(", Merged preference= "); + dumpRegMask(theInterval->registerPreferences); + printf("\n"); } +#endif } //------------------------------------------------------------------------ @@ -1175,6 +1229,8 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo assert(recentRefPosition); if (recentRefPosition->refType != RefTypeBB) { + JITDUMP("Recording killMask at %d\n", recentRefPosition->nodeLocation); + recentCallRefPositionLocation = recentRefPosition->nodeLocation; callRefPositionLocations.push_back(recentRefPosition->nodeLocation); callKillMasks.push_back(killMask); callRefPositionCount++; @@ -2626,6 +2682,7 @@ void LinearScan::buildIntervals() } // Adjust preferCalleeSave + callRefPositionCount = 0; if (callRefPositionCount > 0) { LsraLocation firstCallLocation = callRefPositionLocations.front(); @@ -2641,6 +2698,11 @@ void LinearScan::buildIntervals() continue; } + if ((interval.firstRefPosition == nullptr) || (interval.lastRefPosition == nullptr)) + { + continue; + } + LsraLocation firstRef = interval.firstRefPosition->nodeLocation; LsraLocation lastRef = interval.lastRefPosition->nodeLocation; @@ -2675,12 +2737,15 @@ void LinearScan::buildIntervals() printf("Interval %2u: Update preferences (callee-save) from ", interval.intervalIndex); dumpRegMask(interval.registerPreferences); - printf(" to "); + JITDUMP(", New preference= "); dumpRegMask(newPreferences); - printf("\n"); } #endif interval.updateRegisterPreferences(newPreferences); + + printf(" to "); + dumpRegMask(interval.registerPreferences); + printf("\n"); } } break; From a0fc8b23f7663b232c2732365254a148aa3e7956 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 13 Oct 2022 22:44:06 -0700 Subject: [PATCH 4/6] Revert "Wrap #ifdef" This reverts commit 55e2c5d3cdd8e42324165591de639e8ab78761b9. --- src/coreclr/jit/lsrabuild.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 96a9f04359d3fa..1299db4f181e59 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2731,7 +2731,6 @@ void LinearScan::buildIntervals() if (newPreferences != RBM_NONE) { -#ifdef DEBUG if (VERBOSE) { printf("Interval %2u: Update preferences (callee-save) from ", @@ -2740,7 +2739,6 @@ void LinearScan::buildIntervals() JITDUMP(", New preference= "); dumpRegMask(newPreferences); } -#endif interval.updateRegisterPreferences(newPreferences); printf(" to "); From 218d02d049cc60aa095f0ffbcf0df01ce625bca1 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 13 Oct 2022 22:45:23 -0700 Subject: [PATCH 5/6] Revert "Check if interval interfers with the call" This reverts commit 7ebb61eb79ac95b98b14f37b0cc7d1f52ee9deb4. --- src/coreclr/jit/clrjit.natvis | 2 +- src/coreclr/jit/lsra.cpp | 4 -- src/coreclr/jit/lsra.h | 9 ---- src/coreclr/jit/lsrabuild.cpp | 89 ++--------------------------------- 4 files changed, 6 insertions(+), 98 deletions(-) diff --git a/src/coreclr/jit/clrjit.natvis b/src/coreclr/jit/clrjit.natvis index 140c7e4dec277d..5873fca508786b 100644 --- a/src/coreclr/jit/clrjit.natvis +++ b/src/coreclr/jit/clrjit.natvis @@ -153,7 +153,7 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u - [{nodeLocation,d}. #{rpNum,d} - {refType,en}] + [#{rpNum,d} - {refType,en}] (RegRecord*)this->referent (Interval*)this->referent diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index fc1118434d00a4..3b8605692e5edf 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -624,8 +624,6 @@ LinearScan::LinearScan(Compiler* theCompiler) , intervals(theCompiler->getAllocator(CMK_LSRA_Interval)) , allocationPassComplete(false) , refPositions(theCompiler->getAllocator(CMK_LSRA_RefPosition)) - , callRefPositionLocations(theCompiler->getAllocator(CMK_LSRA)) - , callKillMasks(theCompiler->getAllocator(CMK_LSRA)) , listNodePool(theCompiler) { regSelector = new (theCompiler, CMK_LSRA) RegisterSelection(this); @@ -705,8 +703,6 @@ LinearScan::LinearScan(Compiler* theCompiler) blockSequenceWorkList = nullptr; curBBSeqNum = 0; bbSeqCount = 0; - callRefPositionCount = 0; - recentRefPosition = nullptr; recentCallRefPositionLocation = 0; // Information about each block, including predecessor blocks used for variable locations at block entry. diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index fd132c65fb6dc1..97cfe83d97eed3 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1582,16 +1582,7 @@ class LinearScan : public LinearScanInterface // Ordered list of RefPositions RefPositionList refPositions; - // Most recent refpostion assigned - RefPosition* recentRefPosition; - - // Ordered list of locations where CALL happens LsraLocation recentCallRefPositionLocation; - jitstd::list callRefPositionLocations; - // Equivalent list of kill masks for those calls - jitstd::list callKillMasks; - int callRefPositionCount; - // Per-block variable location mappings: an array indexed by block number that yields a // pointer to an array of regNumber, one per variable. VarToRegMap* inVarToRegMaps; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 1299db4f181e59..2136251163eb89 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -186,7 +186,6 @@ RefPosition* LinearScan::newRefPositionRaw(LsraLocation nodeLocation, GenTree* t currBuildNode = nullptr; newRP->rpNum = static_cast(refPositions.size() - 1); #endif // DEBUG - recentRefPosition = newRP; return newRP; } @@ -1196,9 +1195,8 @@ regMaskTP LinearScan::getKillSetForNode(GenTree* tree) // // Notes: // The return value is needed because if we have any kills, we need to make sure that -// all defs are located AFTER the kills. TODO: This is always the case. -// On the other hand, if there aren't kills, the multiple defs for a regPair are in -// different locations. +// all defs are located AFTER the kills. On the other hand, if there aren't kills, +// the multiple defs for a regPair are in different locations. // If we generate any kills, we will mark all currentLiveVars as being preferenced // to avoid the killed registers. This is somewhat conservative. // @@ -1223,20 +1221,8 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo // if (!blockSequence[curBBSeqNum]->isRunRarely()) if (enregisterLocalVars) { - const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH)); - if (isCallKill) - { - assert(recentRefPosition); - if (recentRefPosition->refType != RefTypeBB) - { JITDUMP("Recording killMask at %d\n", recentRefPosition->nodeLocation); recentCallRefPositionLocation = recentRefPosition->nodeLocation; - callRefPositionLocations.push_back(recentRefPosition->nodeLocation); - callKillMasks.push_back(killMask); - callRefPositionCount++; - } - } - VarSetOps::Iter iter(compiler, currentLiveVars); unsigned varIndex = 0; while (iter.NextElem(&varIndex)) @@ -1257,7 +1243,9 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo { continue; } - Interval* interval = getIntervalForLocalVar(varIndex); + Interval* interval = getIntervalForLocalVar(varIndex); + const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH)); + if (isCallKill) { interval->preferCalleeSave = true; @@ -2681,83 +2669,16 @@ void LinearScan::buildIntervals() } } - // Adjust preferCalleeSave callRefPositionCount = 0; - if (callRefPositionCount > 0) - { - LsraLocation firstCallLocation = callRefPositionLocations.front(); - LsraLocation lastCallLocation = callRefPositionLocations.back(); - - JITDUMP("\n\nCHECKING if Intervals can use callee-save registers"); - JITDUMP("\n==============================\n"); - for (Interval& interval : intervals) - { - if (interval.isLocalVar) - { - // We already handle them in BuildKillPositionsForNode. - continue; - } - if ((interval.firstRefPosition == nullptr) || (interval.lastRefPosition == nullptr)) { continue; } - LsraLocation firstRef = interval.firstRefPosition->nodeLocation; - LsraLocation lastRef = interval.lastRefPosition->nodeLocation; - - if ((firstRef > lastCallLocation) || (lastRef < firstCallLocation)) - { - // Current interval doesn't interfer with any calls. Skip it. - continue; - } - - jitstd::list callKillMasksLocal = callKillMasks; - for (LsraLocation& callLocation : callRefPositionLocations) - { - regMaskTP killMask = callKillMasksLocal.front(); - callKillMasksLocal.pop_front(); - - if ((firstRef <= callLocation) && (callLocation < lastRef)) - { - interval.preferCalleeSave = true; - if (!interval.isWriteThru) - { - // We are more conservative about allocating callee-saves registers to write-thru vars, - // since a call only requires reloading after (not spilling before). So we record (above) - // the fact that we'd prefer a callee-save register, but we don't update the preferences at - // this point. See the "heuristics for writeThru intervals" in 'buildIntervals()'. - regMaskTP newPreferences = allRegs(interval.registerType) & ~killMask; - - if (newPreferences != RBM_NONE) - { - if (VERBOSE) - { - printf("Interval %2u: Update preferences (callee-save) from ", - interval.intervalIndex); - dumpRegMask(interval.registerPreferences); - JITDUMP(", New preference= "); - dumpRegMask(newPreferences); - } - interval.updateRegisterPreferences(newPreferences); printf(" to "); dumpRegMask(interval.registerPreferences); printf("\n"); - } - } - break; - } - - if (lastRef < callLocation) - { - // We passed lastRef, no point in checking further callRefPositions. - break; - } - } - } - } - #ifdef DEBUG if (getLsraExtendLifeTimes()) { From 70426b8c627959c9e6f05daabf03c80f63d79fa8 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 13 Oct 2022 22:53:16 -0700 Subject: [PATCH 6/6] Fix revert conflicts --- src/coreclr/jit/lsra.cpp | 13 +++++++------ src/coreclr/jit/lsra.h | 3 ++- src/coreclr/jit/lsrabuild.cpp | 35 +++++++++++++++-------------------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 3b8605692e5edf..870049034d931e 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -698,12 +698,13 @@ LinearScan::LinearScan(Compiler* theCompiler) // Note that we don't initialize the bbVisitedSet until we do the first traversal // This is so that any blocks that are added during the first traversal // are accounted for (and we don't have BasicBlockEpoch issues). - blockSequencingDone = false; - blockSequence = nullptr; - blockSequenceWorkList = nullptr; - curBBSeqNum = 0; - bbSeqCount = 0; - recentCallRefPositionLocation = 0; + blockSequencingDone = false; + blockSequence = nullptr; + blockSequenceWorkList = nullptr; + curBBSeqNum = 0; + bbSeqCount = 0; + recentRefPosition = nullptr; + recentKillLocation = 0; // Information about each block, including predecessor blocks used for variable locations at block entry. blockInfo = nullptr; diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 97cfe83d97eed3..70415326ca26b2 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1582,7 +1582,8 @@ class LinearScan : public LinearScanInterface // Ordered list of RefPositions RefPositionList refPositions; - LsraLocation recentCallRefPositionLocation; + RefPosition* recentRefPosition; + LsraLocation recentKillLocation; // Per-block variable location mappings: an array indexed by block number that yields a // pointer to an array of regNumber, one per variable. VarToRegMap* inVarToRegMaps; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 2136251163eb89..a4af98b87cac1b 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -186,6 +186,7 @@ RefPosition* LinearScan::newRefPositionRaw(LsraLocation nodeLocation, GenTree* t currBuildNode = nullptr; newRP->rpNum = static_cast(refPositions.size() - 1); #endif // DEBUG + recentRefPosition = newRP; return newRP; } @@ -382,11 +383,11 @@ void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp) RefPosition* firstPosition = theInterval->firstRefPosition; if ((firstPosition != nullptr) && (firstPosition->nodeLocation > 0)) { - if (firstPosition->nodeLocation < recentCallRefPositionLocation) + if (firstPosition->nodeLocation < recentKillLocation) { JITDUMP("Interval %2u: Prefer callee-save because of presence of kills at location %d. First " "RefPosition is at #%d @%d and current RefPosition is #%d @%d\n", - theInterval->intervalIndex, recentCallRefPositionLocation, firstPosition->rpNum, + theInterval->intervalIndex, recentKillLocation, firstPosition->rpNum, firstPosition->nodeLocation, rp->rpNum, rp->nodeLocation); theInterval->preferCalleeSave = true; @@ -407,10 +408,11 @@ void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp) #ifdef DEBUG if (VERBOSE) { - JITDUMP("Interval %2u: Current preference= ", theInterval->intervalIndex); + printf("Interval %2u: Current preference= ", theInterval->intervalIndex); dumpRegMask(theInterval->registerPreferences); - JITDUMP(", New preference= "); + printf(", New preference= "); dumpRegMask(newPreferences); + printf(", "); } #endif if (newPreferences != RBM_NONE) @@ -433,7 +435,7 @@ void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp) #ifdef DEBUG if (VERBOSE) { - JITDUMP(", Merged preference= "); + printf("Merged preference= "); dumpRegMask(theInterval->registerPreferences); printf("\n"); } @@ -1221,8 +1223,13 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo // if (!blockSequence[curBBSeqNum]->isRunRarely()) if (enregisterLocalVars) { - JITDUMP("Recording killMask at %d\n", recentRefPosition->nodeLocation); - recentCallRefPositionLocation = recentRefPosition->nodeLocation; + const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH)); + if (isCallKill) + { + JITDUMP("Recording location of kill at %d\n", recentRefPosition->nodeLocation); + recentKillLocation = recentRefPosition->nodeLocation; + } + VarSetOps::Iter iter(compiler, currentLiveVars); unsigned varIndex = 0; while (iter.NextElem(&varIndex)) @@ -1243,9 +1250,7 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo { continue; } - Interval* interval = getIntervalForLocalVar(varIndex); - const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH)); - + Interval* interval = getIntervalForLocalVar(varIndex); if (isCallKill) { interval->preferCalleeSave = true; @@ -2669,16 +2674,6 @@ void LinearScan::buildIntervals() } } - callRefPositionCount = 0; - if ((interval.firstRefPosition == nullptr) || (interval.lastRefPosition == nullptr)) - { - continue; - } - - - printf(" to "); - dumpRegMask(interval.registerPreferences); - printf("\n"); #ifdef DEBUG if (getLsraExtendLifeTimes()) {