Skip to content

Commit

Permalink
Fix min-opts spill of tree temp large vectors (dotnet/coreclr#22530)
Browse files Browse the repository at this point in the history 
* Fix min-opts spill of tree temp large vectors

Even if we're not enregistering local vars, we may have large vectors live across a call that need to be spilled.

Fix dotnet/coreclr#22200


Commit migrated from dotnet/coreclr@fc70d03
  • Loading branch information
@CarolEidt
CarolEidt committed Feb 13, 2019
1 parent 9d64b5d commit ab6240a
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 51 deletions.
4 changes: 1 addition & 3 deletions src/coreclr/src/jit/lsra.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -989,9 +989,7 @@ class LinearScan : public LinearScanInterface
void buildRefPositionsForNode(GenTree* tree, BasicBlock* block, LsraLocation loc);

#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
VARSET_VALRET_TP buildUpperVectorSaveRefPositions(GenTree* tree,
LsraLocation currentLoc,
regMaskTP fpCalleeKillSet);
void buildUpperVectorSaveRefPositions(GenTree* tree, LsraLocation currentLoc, VARSET_VALARG_TP liveLargeVectors);
void buildUpperVectorRestoreRefPositions(GenTree* tree, LsraLocation currentLoc, VARSET_VALARG_TP liveLargeVectors);
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE

Expand Down
103 changes: 55 additions & 48 deletions src/coreclr/src/jit/lsrabuild.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1315,34 +1315,26 @@ void LinearScan::buildInternalRegisterUses()
#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
//------------------------------------------------------------------------
// buildUpperVectorSaveRefPositions - Create special RefPositions for saving
// the upper half of a set of large vector.
// the upper half of a set of large vectors.
//
// Arguments:
// tree - The current node being handled
// currentLoc - The location of the current node
// fpCalleeKillSet - The set of registers killed by this node.
//
// Return Value: Returns the set of lclVars that are killed by this node, and therefore
// required RefTypeUpperVectorSaveDef RefPositions.
//
// Notes: The returned set contains any lclVars that were partially spilled, and is
// used by buildUpperVectorRestoreRefPositions.
// This is called by BuildDefsWithKills for any node that kills registers in the
// RBM_FLT_CALLEE_TRASH set. We actually need to find any calls that kill the upper-half
// of the callee-save vector registers.
// But we will use as a proxy any node that kills floating point registers.
// (Note that some calls are masquerading as other nodes at this point so we can't just check for calls.)
//
VARSET_VALRET_TP
LinearScan::buildUpperVectorSaveRefPositions(GenTree* tree, LsraLocation currentLoc, regMaskTP fpCalleeKillSet)
// tree - The current node being handled.
// currentLoc - The location of the current node.
// liveLargeVectors - The set of large vector lclVars live across 'tree'.
//
// Notes:
// At this time we create these RefPositions for any large vectors that are live across this node,
// though we don't yet know which, if any, will be in a callee-save register at this point.
// (If they're in a caller-save register, they will simply be fully spilled.)
// This method is called after all the use and kill RefPositions are created for 'tree' but before
// any definitions.
//
void LinearScan::buildUpperVectorSaveRefPositions(GenTree* tree,
LsraLocation currentLoc,
VARSET_VALARG_TP liveLargeVectors)
{
assert(enregisterLocalVars);
VARSET_TP liveLargeVectors(VarSetOps::MakeEmpty(compiler));
if (!VarSetOps::IsEmpty(compiler, largeVectorVars))
if (enregisterLocalVars && !VarSetOps::IsEmpty(compiler, largeVectorVars))
{
assert((fpCalleeKillSet & RBM_FLT_CALLEE_TRASH) != RBM_NONE);
VarSetOps::AssignNoCopy(compiler, liveLargeVectors,
VarSetOps::Intersection(compiler, currentLiveVars, largeVectorVars));
VarSetOps::Iter iter(compiler, liveLargeVectors);
unsigned varIndex = 0;
while (iter.NextElem(&varIndex))
Expand All @@ -1359,31 +1351,34 @@ LinearScan::buildUpperVectorSaveRefPositions(GenTree* tree, LsraLocation current
tempInterval->relatedInterval = varInterval->relatedInterval;
varInterval->relatedInterval = tempInterval;
}
// For any non-lclVar intervals that are live at this point (i.e. in the DefList), we will also create
// a RefTypeUpperVectorSaveDef. For now these will all be spilled at this point, as we don't currently
// have a mechanism to communicate any non-lclVar intervals that need to be restored.
// TODO-CQ: Consider reworking this as part of addressing GitHub Issues #18144 and #17481.
for (RefInfoListNode *listNode = defList.Begin(), *end = defList.End(); listNode != end;
listNode = listNode->Next())
}
// For any non-lclVar large vector intervals that are live at this point (i.e. tree temps in the DefList),
// we will also create a RefTypeUpperVectorSaveDef. For now these will all be spilled at this point,
// as we don't currently have a mechanism to communicate any non-lclVar intervals that need to be restored.
// TODO-CQ: Consider reworking this as part of addressing GitHub Issues #18144 and #17481.
for (RefInfoListNode *listNode = defList.Begin(), *end = defList.End(); listNode != end;
listNode = listNode->Next())
{
var_types treeType = listNode->treeNode->TypeGet();
if (varTypeIsSIMD(treeType) && varTypeNeedsPartialCalleeSave(treeType))
{
var_types treeType = listNode->treeNode->TypeGet();
if (varTypeIsSIMD(treeType) && varTypeNeedsPartialCalleeSave(treeType))
{
RefPosition* pos = newRefPosition(listNode->ref->getInterval(), currentLoc, RefTypeUpperVectorSaveDef,
tree, RBM_FLT_CALLEE_SAVED);
}
RefPosition* pos = newRefPosition(listNode->ref->getInterval(), currentLoc, RefTypeUpperVectorSaveDef, tree,
RBM_FLT_CALLEE_SAVED);
}
}
return liveLargeVectors;
}

// buildUpperVectorRestoreRefPositions - Create special RefPositions for restoring
// the upper half of a set of large vectors.
//
// Arguments:
// tree - The current node being handled
// currentLoc - The location of the current node
// liveLargeVectors - The set of lclVars needing restores (returned by buildUpperVectorSaveRefPositions)
// tree - The current node being handled.
// currentLoc - The location of the current node.
// liveLargeVectors - The set of large vector lclVars live across 'tree'.
//
// Notes:
// This is called after all the RefPositions for 'tree' have been created, since the restore,
// if needed, will be inserted after the call.
//
void LinearScan::buildUpperVectorRestoreRefPositions(GenTree* tree,
LsraLocation currentLoc,
Expand Down Expand Up @@ -2562,7 +2557,7 @@ void LinearScan::BuildDefsWithKills(GenTree* tree, int dstCount, regMaskTP dstCa
// Generate Kill RefPositions
assert(killMask == getKillSetForNode(tree));
#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
VARSET_TP liveLargeVectors(VarSetOps::UninitVal());
VARSET_TP liveLargeVectorVars(VarSetOps::UninitVal());
bool doLargeVectorRestore = false;
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE

Expand All @@ -2572,13 +2567,25 @@ void LinearScan::BuildDefsWithKills(GenTree* tree, int dstCount, regMaskTP dstCa
if (killMask != RBM_NONE)
{
#if FEATURE_PARTIAL_SIMD_CALLEE_SAVE
if (enregisterLocalVars && ((killMask & RBM_FLT_CALLEE_TRASH) != RBM_NONE))
// Build RefPositions to account for the fact that, even in a callee-save register, the upper half of any large
// vector will be killed by a call.
// We actually need to find any calls that kill the upper-half of the callee-save vector registers.
// But we will use as a proxy any node that kills floating point registers.
// (Note that some calls are masquerading as other nodes at this point so we can't just check for calls.)
// We call this unconditionally for such nodes, as we will create RefPositions for any large vector tree temps
// even if 'enregisterLocalVars' is false, or 'liveLargeVectors' is empty, though currently the allocation
// phase will fully (rather than partially) spill those, so we don't need to build the UpperVectorRestore
// RefPositions in that case.
//
if ((killMask & RBM_FLT_CALLEE_TRASH) != RBM_NONE)
{
// Build RefPositions for saving any live large vectors.
// This must be done after the kills, so that we know which large vectors are still live.
VarSetOps::AssignNoCopy(compiler, liveLargeVectors,
buildUpperVectorSaveRefPositions(tree, currentLoc + 1, killMask));
doLargeVectorRestore = true;
if (enregisterLocalVars)
{
VarSetOps::AssignNoCopy(compiler, liveLargeVectorVars,
VarSetOps::Intersection(compiler, currentLiveVars, largeVectorVars));
}
buildUpperVectorSaveRefPositions(tree, currentLoc, liveLargeVectorVars);
doLargeVectorRestore = (enregisterLocalVars && !VarSetOps::IsEmpty(compiler, liveLargeVectorVars));
}
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE
}
Expand All @@ -2590,7 +2597,7 @@ void LinearScan::BuildDefsWithKills(GenTree* tree, int dstCount, regMaskTP dstCa
// Finally, generate the UpperVectorRestores
if (doLargeVectorRestore)
{
buildUpperVectorRestoreRefPositions(tree, currentLoc, liveLargeVectors);
buildUpperVectorRestoreRefPositions(tree, currentLoc, liveLargeVectorVars);
}
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE
}
Expand Down

0 comments on commit ab6240a

Please sign in to comment.