Skip to content

Commit

Permalink
Do not set pre-decided consecutive registers as busy (again) (#85566)
Browse files Browse the repository at this point in the history
  • Loading branch information
kunalspathak authored May 1, 2023
1 parent ef45c07 commit 049acec
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 3 deletions.
28 changes: 27 additions & 1 deletion src/coreclr/jit/lsra.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4818,6 +4818,12 @@ void LinearScan::allocateRegisters()
copyRegsToFree = RBM_NONE;
regsInUseThisLocation = regsInUseNextLocation;
regsInUseNextLocation = RBM_NONE;
#ifdef TARGET_ARM64
if (hasConsecutiveRegister)
{
consecutiveRegsInUseThisLocation = RBM_NONE;
}
#endif
if ((regsToFree | delayRegsToFree) != RBM_NONE)
{
freeRegisters(regsToFree);
Expand Down Expand Up @@ -5433,13 +5439,24 @@ void LinearScan::allocateRegisters()
// It doesn't satisfy, so do a copyReg for the first RefPosition to such a register, so
// it would be possible to allocate consecutive registers to the subsequent RefPositions.
regNumber copyReg = assignCopyReg<true>(&currentRefPosition);
assignConsecutiveRegisters(&currentRefPosition, copyReg);

if (copyReg != assignedRegister)
{
lastAllocatedRefPosition = &currentRefPosition;
regMaskTP copyRegMask = getRegMask(copyReg, currentInterval->registerType);
regMaskTP assignedRegMask = getRegMask(assignedRegister, currentInterval->registerType);

if ((consecutiveRegsInUseThisLocation & assignedRegMask) != RBM_NONE)
{
// If assigned register is one of the consecutive register we are about to assign
// to the subsequent RefPositions, do not mark it busy otherwise when we allocate
// for that particular subsequent RefPosition, we will not find any candidates
// available. Not marking it busy should be fine because we have already set the
// register assignments for all the consecutive refpositions.
assignedRegMask = RBM_NONE;
}

// For consecutive register, although it shouldn't matter what the assigned register was,
// because we have just assigned it `copyReg` and that's the one in-use, and not the
// one that was assigned previously. However, in situation where an upper-vector restore
Expand Down Expand Up @@ -5480,7 +5497,6 @@ void LinearScan::allocateRegisters()
currentRefPosition.registerAssignment = assignedRegBit;
}

assignConsecutiveRegisters(&currentRefPosition, copyReg);
continue;
}
}
Expand Down Expand Up @@ -5538,6 +5554,16 @@ void LinearScan::allocateRegisters()
// registers.
assignConsecutiveRegisters(&currentRefPosition, copyReg);
}

if ((consecutiveRegsInUseThisLocation & assignedRegMask) != RBM_NONE)
{
// If assigned register is one of the consecutive register we are about to assign
// to the subsequent RefPositions, do not mark it busy otherwise when we allocate
// for that particular subsequent RefPosition, we will not find any candidates
// available. Not marking it busy should be fine because we have already set the
// register assignments for all the consecutive refpositions.
assignedRegMask = RBM_NONE;
}
}
#endif
// For consecutive register, although it shouldn't matter what the assigned register was,
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/lsra.h
Original file line number Diff line number Diff line change
Expand Up @@ -1839,6 +1839,9 @@ class LinearScan : public LinearScanInterface
regMaskTP regsBusyUntilKill;
regMaskTP regsInUseThisLocation;
regMaskTP regsInUseNextLocation;
#ifdef TARGET_ARM64
regMaskTP consecutiveRegsInUseThisLocation;
#endif
bool isRegBusy(regNumber reg, var_types regType)
{
regMaskTP regMask = getRegMask(reg, regType);
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/lsraarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ void LinearScan::assignConsecutiveRegisters(RefPosition* firstRefPosition, regNu
assert(firstRefPosition->assignedReg() == firstRegAssigned);
assert(firstRefPosition->isFirstRefPositionOfConsecutiveRegisters());
assert(emitter::isVectorRegister(firstRegAssigned));
assert(consecutiveRegsInUseThisLocation == RBM_NONE);

RefPosition* consecutiveRefPosition = getNextConsecutiveRefPosition(firstRefPosition);
regNumber regToAssign = firstRegAssigned == REG_FP_LAST ? REG_FP_FIRST : REG_NEXT(firstRegAssigned);
Expand All @@ -75,7 +76,7 @@ void LinearScan::assignConsecutiveRegisters(RefPosition* firstRefPosition, regNu
assert(firstRefPosition->refType != RefTypeUpperVectorRestore);

INDEBUG(int refPosCount = 1);
regMaskTP busyConsecutiveRegMask = (((1ULL << firstRefPosition->regCount) - 1) << firstRegAssigned);
consecutiveRegsInUseThisLocation = (((1ULL << firstRefPosition->regCount) - 1) << firstRegAssigned);

while (consecutiveRefPosition != nullptr)
{
Expand All @@ -95,7 +96,7 @@ void LinearScan::assignConsecutiveRegisters(RefPosition* firstRefPosition, regNu
// RefTypeUpperVectorRestore positions of corresponding variables for which (another criteria)
// we are trying to find consecutive registers.

consecutiveRefPosition->registerAssignment &= ~busyConsecutiveRegMask;
consecutiveRefPosition->registerAssignment &= ~consecutiveRegsInUseThisLocation;
}
consecutiveRefPosition = getNextConsecutiveRefPosition(consecutiveRefPosition);
}
Expand Down

0 comments on commit 049acec

Please sign in to comment.