Skip to content

LSRA: PreferCalleeSave for non-localVar intervals #92744

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

Closed
kunalspathak wants to merge 6 commits into from
Closed

LSRA: PreferCalleeSave for non-localVar intervals #92744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fef90c7
Update preferCalleeSave for call interfering intervals
kunalspathak Sep 27, 2023
9786e5f
Skip doing it for LocalVar
kunalspathak Sep 27, 2023
7663de0
Update the dump
kunalspathak Sep 27, 2023
1ed28a5
jit format
kunalspathak Sep 27, 2023
c1f2848
Merge remote-tracking branch 'origin/main' into lsra/preferCalleeSave
kunalspathak Nov 7, 2023
84181f9
Set preferCalleeSave only if the interval actually overlap with call
kunalspathak Nov 8, 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
5 changes: 5 additions & 0 deletions src/coreclr/jit/lsra.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ LinearScan::LinearScan(Compiler* theCompiler)
blockSequenceWorkList = nullptr;
curBBSeqNum = 0;
bbSeqCount = 0;
recentKillLocation = 0;

// Information about each block, including predecessor blocks used for variable locations at block entry.
blockInfo = nullptr;
Expand Down Expand Up @@ -9668,6 +9669,10 @@ void Interval::dump(Compiler* compiler)

printf(" Preferences=");
compiler->dumpRegMask(this->registerPreferences);
if (preferCalleeSave)
{
printf("(prefer-callee-save)");
}

if (relatedInterval)
{
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/lsra.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ class Referenceable
firstRefPosition = nullptr;
recentRefPosition = nullptr;
lastRefPosition = nullptr;
recentDefRefPosition = nullptr;
}

// A linked list of RefPositions. These are only traversed in the forward
Expand All @@ -458,6 +459,7 @@ class Referenceable
RefPosition* firstRefPosition;
RefPosition* recentRefPosition;
RefPosition* lastRefPosition;
RefPosition* recentDefRefPosition;

// Get the position of the next reference which is at or greater than
// the current location (relies upon recentRefPosition being updated
Expand Down Expand Up @@ -1626,6 +1628,8 @@ class LinearScan : public LinearScanInterface
unsigned int bbSeqCount;
// The Location of the start of the current block.
LsraLocation curBBStartLocation;
// Most recent kill location
LsraLocation recentKillLocation;
// True if the method contains any critical edges.
bool hasCriticalEdges;

Expand Down
30 changes: 28 additions & 2 deletions src/coreclr/jit/lsrabuild.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,15 @@ void LinearScan::applyCalleeSaveHeuristics(RefPosition* rp)

Interval* theInterval = rp->getInterval();

//if ((theInterval->recentDefRefPosition != nullptr) && RefTypeIsUse(rp->refType) && !theInterval->isWriteThru &&
// !theInterval->isLocalVar)
//{
// if (theInterval->recentDefRefPosition->nodeLocation < recentKillLocation)
// {
// theInterval->preferCalleeSave = true;
// }
//}

#ifdef DEBUG
if (!doReverseCallerCallee())
#endif // DEBUG
Expand Down Expand Up @@ -482,6 +491,16 @@ void LinearScan::associateRefPosWithInterval(RefPosition* rp)
if (prevRP != nullptr)
{
prevRP->nextRefPosition = rp;
if (rp->isIntervalRef() && RefTypeIsDef(rp->refType))
{
Interval* theInterval = rp->getInterval();
if (theInterval->recentDefRefPosition != nullptr && theInterval->recentDefRefPosition->nodeLocation <
recentKillLocation &&
recentKillLocation < prevRP->nodeLocation)
{
theInterval->preferCalleeSave = true;
}
}
}
else
{
Expand Down Expand Up @@ -639,6 +658,7 @@ RefPosition* LinearScan::newRefPosition(Interval* theInterval,
{
assert(theInterval != nullptr);
theInterval->isSingleDef = theInterval->firstRefPosition == newRP;
theInterval->recentDefRefPosition = newRP;
}

DBEXEC(VERBOSE, newRP->dump(this));
Expand Down Expand Up @@ -1159,6 +1179,12 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo
{
addRefsForPhysRegMask(killMask, currentLoc, RefTypeKill, true);

const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH));
if (isCallKill)
{
recentKillLocation = currentLoc;
}

// TODO-CQ: It appears to be valuable for both fp and int registers to avoid killing the callee
// save regs on infrequently executed paths. However, it results in a large number of asmDiffs,
// many of which appear to be regressions (because there is more spill on the infrequently path),
Expand Down Expand Up @@ -1189,8 +1215,7 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo
{
continue;
}
Interval* interval = getIntervalForLocalVar(varIndex);
const bool isCallKill = ((killMask == RBM_INT_CALLEE_TRASH) || (killMask == RBM_CALLEE_TRASH));
Interval* interval = getIntervalForLocalVar(varIndex);

if (isCallKill)
{
Expand Down Expand Up @@ -1228,6 +1253,7 @@ bool LinearScan::buildKillPositionsForNode(GenTree* tree, LsraLocation currentLo
{
RefPosition* pos =
newRefPosition((Interval*)nullptr, currentLoc, RefTypeKillGCRefs, tree, (availableIntRegs & ~RBM_ARG_REGS));

insertedKills = true;
}

Expand Down