Skip to content

Commit

Permalink
JIT: Fix resolveConflictingDefAndUse to take kills into account (#1…
Browse files Browse the repository at this point in the history
…03562)

This function should have been updated as part of f95a76b to take into
account that kills are no longer present in the RefPosition list for
`RegRecord`s. We apparently did not see any issues as a result, but this
PR fixes it to account for that.
  • Loading branch information
jakobbotsch authored Jun 18, 2024
1 parent 59f7bbc commit 504b4fb
Showing 1 changed file with 18 additions and 19 deletions.
37 changes: 18 additions & 19 deletions src/coreclr/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,13 @@ RefPosition* LinearScan::newRefPositionRaw(LsraLocation nodeLocation, GenTree* t
// leaving the registerAssignment as-is on the def, so that if we find that we need to spill anyway
// we can use the fixed-reg on the def.
//

void LinearScan::resolveConflictingDefAndUse(Interval* interval, RefPosition* defRefPosition)
{
assert(!interval->isLocalVar);

RefPosition* useRefPosition = defRefPosition->nextRefPosition;
SingleTypeRegSet defRegAssignment = defRefPosition->registerAssignment;
SingleTypeRegSet useRegAssignment = useRefPosition->registerAssignment;
RegRecord* defRegRecord = nullptr;
RegRecord* useRegRecord = nullptr;
regNumber defReg = REG_NA;
regNumber useReg = REG_NA;
bool defRegConflict = ((defRegAssignment & useRegAssignment) == RBM_NONE);
Expand All @@ -272,16 +269,18 @@ void LinearScan::resolveConflictingDefAndUse(Interval* interval, RefPosition* de
}
if (defRefPosition->isFixedRegRef && !defRegConflict)
{
defReg = defRefPosition->assignedReg();
defRegRecord = getRegisterRecord(defReg);
defReg = defRefPosition->assignedReg();
if (canChangeUseAssignment)
{
#ifdef DEBUG
RegRecord* defRegRecord = getRegisterRecord(defReg);
RefPosition* currFixedRegRefPosition = defRegRecord->recentRefPosition;
assert(currFixedRegRefPosition != nullptr &&
currFixedRegRefPosition->nodeLocation == defRefPosition->nodeLocation);
assert((currFixedRegRefPosition != nullptr) &&
(currFixedRegRefPosition->nodeLocation == defRefPosition->nodeLocation));
#endif

if (currFixedRegRefPosition->nextRefPosition == nullptr ||
currFixedRegRefPosition->nextRefPosition->nodeLocation > useRefPosition->getRefEndLocation())
LsraLocation nextRegLoc = getNextFixedRef(defReg, defRefPosition->getRegisterType());
if (nextRegLoc > useRefPosition->getRefEndLocation())
{
// This is case #1. Use the defRegAssignment
INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DEFUSE_CASE1));
Expand All @@ -296,19 +295,19 @@ void LinearScan::resolveConflictingDefAndUse(Interval* interval, RefPosition* de
}
if (useRefPosition->isFixedRegRef && !useRegConflict)
{
useReg = useRefPosition->assignedReg();
useRegRecord = getRegisterRecord(useReg);
useReg = useRefPosition->assignedReg();

LsraLocation nextRegLoc = getNextFixedRef(useReg, useRefPosition->getRegisterType());

// We know that useRefPosition is a fixed use, so the nextRefPosition must not be null.
RefPosition* nextFixedRegRefPosition = useRegRecord->getNextRefPosition();
assert(nextFixedRegRefPosition != nullptr &&
nextFixedRegRefPosition->nodeLocation <= useRefPosition->nodeLocation);
// We know that useRefPosition is a fixed use, so there is a next reference.
assert(nextRegLoc <= useRefPosition->nodeLocation);

// First, check to see if there are any conflicting FixedReg references between the def and use.
if (nextFixedRegRefPosition->nodeLocation == useRefPosition->nodeLocation)
if (nextRegLoc == useRefPosition->nodeLocation)
{
// OK, no conflicting FixedReg references.
// Now, check to see whether it is currently in use.
RegRecord* useRegRecord = getRegisterRecord(useReg);
if (useRegRecord->assignedInterval != nullptr)
{
RefPosition* possiblyConflictingRef = useRegRecord->assignedInterval->recentRefPosition;
Expand All @@ -331,21 +330,21 @@ void LinearScan::resolveConflictingDefAndUse(Interval* interval, RefPosition* de
useRegConflict = true;
}
}
if (defRegRecord != nullptr && !useRegConflict)
if ((defReg != REG_NA) && !useRegConflict)
{
// This is case #3.
INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DEFUSE_CASE3, interval));
defRefPosition->registerAssignment = useRegAssignment;
return;
}
if (useRegRecord != nullptr && !defRegConflict && canChangeUseAssignment)
if ((useReg != REG_NA) && !defRegConflict && canChangeUseAssignment)
{
// This is case #4.
INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DEFUSE_CASE4, interval));
useRefPosition->registerAssignment = defRegAssignment;
return;
}
if (defRegRecord != nullptr && useRegRecord != nullptr)
if ((defReg != REG_NA) && (useReg != REG_NA))
{
// This is case #5.
INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DEFUSE_CASE5, interval));
Expand Down

0 comments on commit 504b4fb

Please sign in to comment.