Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix initialization of resolution sets #8840

Merged
merged 1 commit into from
Jan 12, 2017
Merged

Fix initialization of resolution sets #8840

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 23 additions & 7 deletions src/jit/lsra.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1914,6 +1914,14 @@ bool LinearScan::isRegCandidate(LclVarDsc* varDsc)

void LinearScan::identifyCandidates()
{

// Initialize the sets of lclVars that are used to determine whether, and for which lclVars,
// we need to perform resolution across basic blocks.
// Note that we can't do this in the constructor because the number of tracked lclVars may
// change between the constructor and the actual allocation.
VarSetOps::AssignNoCopy(compiler, resolutionCandidateVars, VarSetOps::MakeEmpty(compiler));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this cannot be done in the class constructor or do you prefer having these here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this cannot be done in the class constructor or do you prefer having these here?

Took me awhile to track this down but it can't be done in the constructor because at that point the set of tracked lclVars hasn't been finalized, so the epoch on the bit sets doesn't match. I've added a comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this cannot be done in the class constructor or do you prefer having these here?

Took me awhile to track this down but it can't be done in the constructor because at that point the set of tracked lclVars hasn't been finalized, so the epoch on the bit sets doesn't match. I've added a comment.

VarSetOps::AssignNoCopy(compiler, splitOrSpilledVars, VarSetOps::MakeEmpty(compiler));

if (compiler->lvaCount == 0)
{
return;
Expand Down Expand Up @@ -1950,8 +1958,6 @@ void LinearScan::identifyCandidates()
// for vectors on Arm64, though the actual value may differ.

VarSetOps::AssignNoCopy(compiler, fpCalleeSaveCandidateVars, VarSetOps::MakeEmpty(compiler));
VarSetOps::AssignNoCopy(compiler, resolutionCandidateVars, VarSetOps::MakeEmpty(compiler));
VarSetOps::AssignNoCopy(compiler, splitOrSpilledVars, VarSetOps::MakeEmpty(compiler));
VARSET_TP VARSET_INIT_NOCOPY(fpMaybeCandidateVars, VarSetOps::MakeEmpty(compiler));
unsigned int floatVarCount = 0;
unsigned int thresholdFPRefCntWtd = 4 * BB_UNITY_WEIGHT;
Expand Down Expand Up @@ -10412,11 +10418,21 @@ void LinearScan::lsraDispNode(GenTreePtr tree, LsraTupleDumpMode mode, bool hasD
}
if (!hasDest && tree->gtHasReg())
{
// This can be true for the "localDefUse" case - defining a reg, but
// pushing it on the stack
assert(spillChar == ' ');
spillChar = '*';
hasDest = true;
// A node can define a register, but not produce a value for a parent to consume,
// i.e. in the "localDefUse" case.
// There used to be an assert here that we wouldn't spill such a node.
// However, we can have unused lclVars that wind up being the node at which
// it is spilled. This probably indicates a bug, but we don't realy want to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Typo "realy" instead of "really"

// assert during a dump.
if (spillChar == 'S')
{
spillChar = '$';
}
else
{
spillChar = '*';
}
hasDest = true;
}
}
printf("%c N%03u. ", spillChar, tree->gtSeqNum);
Expand Down