Skip to content

Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kunalspathak merged 12 commits into from
Apr 11, 2023
Merged

Update handling of limited register during consecutive registers allocation #84588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d452147
Fix issue to handle candidates that don't have assignedInterval in sp…
kunalspathak Apr 9, 2023
1708964
Do not let limitRegs reduce the number of candidates
kunalspathak Apr 10, 2023
c31cfbd
getConsecutiveCandidates update if freeCandidate=RBM_NONE
kunalspathak Apr 10, 2023
e684275
Relax limitStressRegs for refpositions live at consecutive register p…
kunalspathak Apr 10, 2023
13e85ac
Update minRegCount for registerAssignment
kunalspathak Apr 10, 2023
f2cda1e
Remove LsraLimitFPSetForConsecutive
kunalspathak Apr 10, 2023
c0c3f4a
Added an assert
kunalspathak Apr 10, 2023
01ec812
misc changes
kunalspathak Apr 10, 2023
e5698cc
jit format
kunalspathak Apr 11, 2023
fe4f675
Merge remote-tracking branch 'origin/main' into consecutive_jitstress…
kunalspathak Apr 11, 2023
2a3d0f2
Use BitOperations::BitScanForward()
kunalspathak Apr 11, 2023
3f5dfd7
review feedback
kunalspathak Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 133 additions & 55 deletions src/coreclr/jit/lsra.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
{
mask |= refPosition->registerAssignment;
}

#ifdef TARGET_ARM64
if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
{
mask |= LsraLimitFPSetForConsecutive;
}
#endif
}

return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
firstColdLoc = MaxLocation;

#ifdef DEBUG
maxNodeLocation = 0;
maxNodeLocation = 0;
consecutiveRegistersLocation = 0;

activeRefPosition = nullptr;
currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
}
}
prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
if (hasConsecutiveRegister)
{
if (currentRefPosition.needsConsecutive)
{
// track all the refpositions around the location that is also
// allocating consecutive registers.
consecutiveRegistersLocation = currentLocation;
}
else if (consecutiveRegistersLocation < currentLocation)
{
consecutiveRegistersLocation = MinLocation;
}
}
#endif // DEBUG
#endif // TARGET_ARM64

// get previous refposition, then current refpos is the new previous
if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
if (assignedInterval == nullptr)
{
// Ideally we should not be seeing this candidate because it is not assigned to
// any interval. But based on that, we cannot determine if it is a good spill
// candidate or not. Skip processing it.
continue;
}

if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
(recentRefPosition->needsConsecutive))
{
continue;
}
#endif // TARGET_ARM64

if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
!assignedInterval->getNextRefPosition()->RegOptional())
{
continue;
}
if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
else if (assignedInterval != nullptr)
#endif
{
continue;
}
if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
!assignedInterval->getNextRefPosition()->RegOptional())
{
continue;
}
if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
{
continue;
}

weight_t currentSpillWeight = 0;
if ((recentRefPosition != nullptr) &&
(recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
{
// We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
// is not an actual ref. In those cases, we will reload in future (next) refPosition.
// For such cases, consider the spill cost of next refposition.
// See notes in "spillInterval()".
RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
if (reloadRefPosition != nullptr)
if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
!(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
{
currentSpillWeight = linearScan->getWeight(reloadRefPosition);
// We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
// is not an actual ref. In those cases, we will reload in future (next) refPosition.
// For such cases, consider the spill cost of next refposition.
// See notes in "spillInterval()".
RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
if (reloadRefPosition != nullptr)
{
currentSpillWeight = linearScan->getWeight(reloadRefPosition);
}
}
}
#ifdef TARGET_ARM64
else
{
// Ideally we should not be seeing this candidate because it is not assigned to
// any interval. But it is possible for certain scenarios. One of them is that
// `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
// registers. This candidate is part of that set and is free and hence is not assigned
// to any interval.
}
#endif // TARGET_ARM64

// Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
// The assigned should be non-null, and should have a recentRefPosition, however since
// this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
if (!hasAssignedInterval)
if (!hasAssignedInterval
#ifdef TARGET_ARM64
// We could see a candidate that doesn't have assignedInterval because allocation is
// happening for `refPosition` that needs consecutive registers and we decided to pick
// a mix of free+busy registers. This candidate is part of that set and is free and hence
// is not assigned to any interval.

&& !refPosition->needsConsecutive
#endif
)
{
assert(!"Spill candidate has no assignedInterval recentRefPosition");
}
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
*registerScore = NONE;
#endif

#ifdef TARGET_ARM64
assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

reset(currentInterval, refPosition);

// process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
}

#ifdef DEBUG
candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
{
// If a method has consecutive registers and we are assigning to refPositions that are not part
// of consecutive registers, but are live at same location, skip the limit stress for them, because
// there are high chances that many registers are busy for consecutive requirements and we don't
// have enough remaining for other refpositions (like operands).
}
else
#endif
{
candidates = linearScan->stressLimitRegs(refPosition, candidates);
}
#endif
assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
}
}

#ifdef DEBUG
regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

// Eliminate candidates that are in-use or busy.
if (!found)
{
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
candidates &= ~busyRegs;

#ifdef DEBUG
inUseOrBusyRegsMask |= busyRegs;
#endif

// Also eliminate as busy any register with a conflicting fixed reference at this or
// the next location.
// Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
(refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
{
candidates &= ~checkConflictBit;
#ifdef DEBUG
inUseOrBusyRegsMask |= checkConflictBit;
#endif
}
}
candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
prevRegBit = genRegMask(prevRegRec->regNum);
if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
{
#ifdef TARGET_ARM64
// If this is allocating for consecutive register, we need to make sure that
// we allocate register, whose consecutive registers are also free.
if (!needsConsecutiveRegisters)
#endif
{
// If this is allocating for consecutive register, we need to make sure that
// we allocate register, whose consecutive registers are also free.
candidates = prevRegBit;
found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
prevRegBit = RBM_NONE;
}

if (!found && (candidates == RBM_NONE))
{
assert(refPosition->RegOptional());
currentInterval->assignedReg = nullptr;
return RBM_NONE;
}

// TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
// It needs to be re-engineered with this refactoring.
// In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
if (needsConsecutiveRegisters)
{
#ifdef TARGET_ARM64
regMaskTP busyConsecutiveCandidates = RBM_NONE;
if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
{
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
{
noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
// Need to make sure that candidates has N consecutive registers to assign
if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
{
// If the refPosition needs consecutive registers, then we want to make sure that
// the candidates have atleast one range of N registers that are consecutive, where N
// is the number of consecutive registers needed.
// Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
// such range that is consecutive. Next, append that range to the `candidates`.
//
regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
regMaskTP overallLimitCandidates;
regMaskTP limitConsecutiveResult =
linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
&overallLimitCandidates);
assert(limitConsecutiveResult != RBM_NONE);

unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
candidates |= (registersNeededMask << startRegister);
}

if (candidates == RBM_NONE)
#endif // DEBUG
{
noway_assert(!"Not sufficient consecutive registers available.");
}
}
#endif // TARGET_ARM64
}
else
#endif // TARGET_ARM64
{
if (!found && (candidates == RBM_NONE))
{
assert(refPosition->RegOptional());
currentInterval->assignedReg = nullptr;
return RBM_NONE;
}

freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
}

Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/jit/lsra.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
// LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
// the requirements for allocating consecutive registers.
static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Update handling of limited register during consecutive registers allocation by kunalspathak · Pull Request #84588 · dotnet/runtime · GitHub
Skip to content
Update handling of limited register during consecutive registers allocation#84588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

kunalspathak
merged 12 commits into

from

Apr 11, 2023
 Merged


Update handling of limited register during consecutive registers allocation
#84588
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


12 commits


Select commit
Hold shift + click to select a range
d452147

Fix issue to handle candidates that don't have assignedInterval in sp…


kunalspathak Apr 9, 2023
1708964

Do not let limitRegs reduce the number of candidates


kunalspathak Apr 10, 2023
c31cfbd

getConsecutiveCandidates update if freeCandidate=RBM_NONE


kunalspathak Apr 10, 2023
e684275

Relax limitStressRegs for refpositions live at consecutive register p…


kunalspathak Apr 10, 2023
13e85ac

Update minRegCount for registerAssignment


kunalspathak Apr 10, 2023
f2cda1e

Remove LsraLimitFPSetForConsecutive


kunalspathak Apr 10, 2023
c0c3f4a

Added an assert


kunalspathak Apr 10, 2023
01ec812

misc changes


kunalspathak Apr 10, 2023
e5698cc

jit format


kunalspathak Apr 11, 2023
fe4f675

Merge remote-tracking branch 'origin/main' into consecutive_jitstress…


kunalspathak Apr 11, 2023
2a3d0f2

Use BitOperations::BitScanForward()


kunalspathak Apr 11, 2023
3f5dfd7

review feedback


kunalspathak Apr 11, 2023

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


188 changes: 133 additions & 55 deletions
src/coreclr/jit/lsra.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -496,13 +496,6 @@ regMaskTP LinearScan::stressLimitRegs(RefPosition* refPosition, regMaskTP mask)
 {
 mask |= refPosition->registerAssignment;
 }

#ifdef TARGET_ARM64
 if ((refPosition != nullptr) && refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
 mask |= LsraLimitFPSetForConsecutive;
 }
#endif
 }

 return mask;
Expand DownExpand Up@@ -662,7 +655,9 @@ LinearScan::LinearScan(Compiler* theCompiler)
 firstColdLoc = MaxLocation;

#ifdef DEBUG
 maxNodeLocation = 0;
 maxNodeLocation = 0;
 consecutiveRegistersLocation = 0;

 activeRefPosition = nullptr;
 currBuildNode = nullptr;

Expand DownExpand Up@@ -4901,6 +4896,24 @@ void LinearScan::allocateRegisters()
 }
 }
 prevLocation = currentLocation;
#ifdef TARGET_ARM64

#ifdef DEBUG
 if (hasConsecutiveRegister)
 {
 if (currentRefPosition.needsConsecutive)
 {
 // track all the refpositions around the location that is also
 // allocating consecutive registers.
 consecutiveRegistersLocation = currentLocation;
 }
 else if (consecutiveRegistersLocation < currentLocation)
 {
 consecutiveRegistersLocation = MinLocation;
 }
 }
#endif // DEBUG
#endif // TARGET_ARM64

 // get previous refposition, then current refpos is the new previous
 if (currentReferent != nullptr)
Expand DownExpand Up@@ -11683,49 +11696,53 @@ void LinearScan::RegisterSelection::try_SPILL_COST()
 Interval* assignedInterval = spillCandidateRegRecord->assignedInterval;
 RefPosition* recentRefPosition = assignedInterval != nullptr ? assignedInterval->recentRefPosition : nullptr;

// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?
// Can and should the interval in this register be spilled for this one,
// if we don't find a better alternative?

 weight_t currentSpillWeight = 0;
#ifdef TARGET_ARM64
 if (assignedInterval == nullptr)
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But based on that, we cannot determine if it is a good spill
 // candidate or not. Skip processing it.
 continue;
 }

 if ((recentRefPosition != nullptr) && linearScan->isRefPositionActive(recentRefPosition, thisLocation) &&
 (recentRefPosition->needsConsecutive))
 {
 continue;
 }
#endif // TARGET_ARM64

 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 else if (assignedInterval != nullptr)
#endif
 {
 continue;
 }
 if ((linearScan->getNextIntervalRef(spillCandidateRegNum, regType) == thisLocation) &&
 !assignedInterval->getNextRefPosition()->RegOptional())
 {
 continue;
 }
 if (!linearScan->isSpillCandidate(currentInterval, refPosition, spillCandidateRegRecord))
 {
 continue;
 }

 weight_t currentSpillWeight = 0;
 if ((recentRefPosition != nullptr) &&
 (recentRefPosition->RegOptional() && !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 if ((recentRefPosition != nullptr) && (recentRefPosition->RegOptional() &&
 !(assignedInterval->isLocalVar && recentRefPosition->IsActualRef())))
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 // We do not "spillAfter" if previous (recent) refPosition was regOptional or if it
 // is not an actual ref. In those cases, we will reload in future (next) refPosition.
 // For such cases, consider the spill cost of next refposition.
 // See notes in "spillInterval()".
 RefPosition* reloadRefPosition = assignedInterval->getNextRefPosition();
 if (reloadRefPosition != nullptr)
 {
 currentSpillWeight = linearScan->getWeight(reloadRefPosition);
 }
 }
 }
#ifdef TARGET_ARM64
 else
 {
 // Ideally we should not be seeing this candidate because it is not assigned to
 // any interval. But it is possible for certain scenarios. One of them is that
 // `refPosition` needs consecutive registers and we decided to pick a mix of free+busy
 // registers. This candidate is part of that set and is free and hence is not assigned
 // to any interval.
 }
#endif // TARGET_ARM64

 // Only consider spillCost if we were not able to calculate weight of reloadRefPosition.
 if (currentSpillWeight == 0)
Expand DownExpand Up@@ -11875,7 +11892,16 @@ void LinearScan::RegisterSelection::try_PREV_REG_OPT()
#ifdef DEBUG
 // The assigned should be non-null, and should have a recentRefPosition, however since
 // this is a heuristic, we don't want a fatal error, so we just assert (not noway_assert).
 if (!hasAssignedInterval)
 if (!hasAssignedInterval
#ifdef TARGET_ARM64
 // We could see a candidate that doesn't have assignedInterval because allocation is
 // happening for `refPosition` that needs consecutive registers and we decided to pick
 // a mix of free+busy registers. This candidate is part of that set and is free and hence
 // is not assigned to any interval.

 && !refPosition->needsConsecutive
#endif
 )
 {
 assert(!"Spill candidate has no assignedInterval recentRefPosition");
 }
Expand DownExpand Up@@ -11988,6 +12014,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 *registerScore = NONE;
#endif

#ifdef TARGET_ARM64
 assert(!needsConsecutiveRegisters || refPosition->needsConsecutive);
#endif

 reset(currentInterval, refPosition);

 // process data-structures
Expand DownExpand Up@@ -12036,7 +12066,19 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }

#ifdef DEBUG
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
#ifdef TARGET_ARM64
 if (!refPosition->needsConsecutive && (linearScan->consecutiveRegistersLocation == refPosition->nodeLocation))
 {
 // If a method has consecutive registers and we are assigning to refPositions that are not part
 // of consecutive registers, but are live at same location, skip the limit stress for them, because
 // there are high chances that many registers are busy for consecutive requirements and we don't
 // have enough remaining for other refpositions (like operands).
 }
 else
#endif
 {
 candidates = linearScan->stressLimitRegs(refPosition, candidates);
 }
#endif
 assert(candidates != RBM_NONE);

Expand DownExpand Up@@ -12186,6 +12228,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 }
 }

#ifdef DEBUG
 regMaskTP inUseOrBusyRegsMask = RBM_NONE;
#endif

 // Eliminate candidates that are in-use or busy.
 if (!found)
 {
Expand All@@ -12195,6 +12241,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 regMaskTP busyRegs = linearScan->regsBusyUntilKill | linearScan->regsInUseThisLocation;
 candidates &= ~busyRegs;

#ifdef DEBUG
 inUseOrBusyRegsMask |= busyRegs;
#endif

 // Also eliminate as busy any register with a conflicting fixed reference at this or
 // the next location.
 // Note that this will eliminate the fixedReg, if any, but we'll add it back below.
Expand All@@ -12210,6 +12260,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 (refPosition->delayRegFree && (checkConflictLocation == (refPosition->nodeLocation + 1))))
 {
 candidates &= ~checkConflictBit;
#ifdef DEBUG
 inUseOrBusyRegsMask |= checkConflictBit;
#endif
 }
 }
 candidates |= fixedRegMask;
Expand All@@ -12226,12 +12279,10 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = genRegMask(prevRegRec->regNum);
 if ((prevRegRec->assignedInterval == currentInterval) && ((candidates & prevRegBit) != RBM_NONE))
 {
#ifdef TARGET_ARM64
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 if (!needsConsecutiveRegisters)
#endif
 {
 // If this is allocating for consecutive register, we need to make sure that
 // we allocate register, whose consecutive registers are also free.
 candidates = prevRegBit;
 found = true;
#ifdef DEBUG
Expand All@@ -12245,13 +12296,6 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 prevRegBit = RBM_NONE;
 }

 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 // TODO-Cleanup: Previously, the "reverseSelect" stress mode reversed the order of the heuristics.
 // It needs to be re-engineered with this refactoring.
 // In non-debug builds, this will simply get optimized away
Expand All@@ -12260,9 +12304,9 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,
 reverseSelect = linearScan->doReverseSelect();
#endif // DEBUG

#ifdef TARGET_ARM64
 if (needsConsecutiveRegisters)
 {
#ifdef TARGET_ARM64
 regMaskTP busyConsecutiveCandidates = RBM_NONE;
 if (refPosition->isFirstRefPositionOfConsecutiveRegisters())
 {
Expand All@@ -12287,12 +12331,46 @@ regMaskTP LinearScan::RegisterSelection::select(Interval* currentInterval,

 if ((freeCandidates == RBM_NONE) && (candidates == RBM_NONE))
 {
 noway_assert(!"Not sufficient consecutive registers available.");
#ifdef DEBUG
 // Need to make sure that candidates has N consecutive registers to assign
 if (linearScan->getStressLimitRegs() != LSRA_LIMIT_NONE)
 {
 // If the refPosition needs consecutive registers, then we want to make sure that
 // the candidates have atleast one range of N registers that are consecutive, where N
 // is the number of consecutive registers needed.
 // Remove the `inUseOrBusyRegsMask` from the original candidates list and find one
 // such range that is consecutive. Next, append that range to the `candidates`.
 //
 regMaskTP limitCandidatesForConsecutive = refPosition->registerAssignment & ~inUseOrBusyRegsMask;
 regMaskTP overallLimitCandidates;
 regMaskTP limitConsecutiveResult =
 linearScan->filterConsecutiveCandidates(limitCandidatesForConsecutive, refPosition->regCount,
 &overallLimitCandidates);
 assert(limitConsecutiveResult != RBM_NONE);

 unsigned startRegister = BitOperations::BitScanForward(limitConsecutiveResult);

 regMaskTP registersNeededMask = (1ULL << refPosition->regCount) - 1;
 candidates |= (registersNeededMask << startRegister);
 }

 if (candidates == RBM_NONE)
#endif // DEBUG
 {
 noway_assert(!"Not sufficient consecutive registers available.");
 }
 }
#endif // TARGET_ARM64
 }
 else
#endif // TARGET_ARM64
 {
 if (!found && (candidates == RBM_NONE))
 {
 assert(refPosition->RegOptional());
 currentInterval->assignedReg = nullptr;
 return RBM_NONE;
 }

 freeCandidates = linearScan->getFreeCandidates(candidates ARM_ARG(regType));
 }

Expand Down

9 changes: 5 additions & 4 deletions
src/coreclr/jit/lsra.h

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,9 +792,6 @@ class LinearScan : public LinearScanInterface
#elif defined(TARGET_ARM64)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_R0 | RBM_R1 | RBM_R2 | RBM_R19 | RBM_R20);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_V0 | RBM_V1 | RBM_V2 | RBM_V8 | RBM_V9);
 // LsraLimitFPSetForConsecutive is used for stress mode and gives few extra registers to satisfy
 // the requirements for allocating consecutive registers.
 static const regMaskTP LsraLimitFPSetForConsecutive = (RBM_V3 | RBM_V5 | RBM_V7);
#elif defined(TARGET_X86)
 static const regMaskTP LsraLimitSmallIntSet = (RBM_EAX | RBM_ECX | RBM_EDI);
 static const regMaskTP LsraLimitSmallFPSet = (RBM_XMM0 | RBM_XMM1 | RBM_XMM2 | RBM_XMM6 | RBM_XMM7);
Expand DownExpand Up@@ -2006,9 +2003,13 @@ class LinearScan : public LinearScanInterface
 int BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCount);
#ifdef TARGET_ARM64
 int BuildConsecutiveRegistersForUse(GenTree* treeNode, GenTree* rmwNode = nullptr);
#endif
#endif // TARGET_ARM64
#endif // FEATURE_HW_INTRINSICS

#ifdef DEBUG
 LsraLocation consecutiveRegistersLocation;
#endif // DEBUG

 int BuildPutArgStk(GenTreePutArgStk* argNode);
#if FEATURE_ARG_SPLIT
 int BuildPutArgSplit(GenTreePutArgSplit* tree);
Expand Down
Loading