-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix initialization of resolution sets #8840
Conversation
@sivarv @dotnet/jit-contrib PTAL |
@@ -1914,6 +1914,8 @@ bool LinearScan::isRegCandidate(LclVarDsc* varDsc) | |||
|
|||
void LinearScan::identifyCandidates() | |||
{ | |||
VarSetOps::AssignNoCopy(compiler, resolutionCandidateVars, VarSetOps::MakeEmpty(compiler)); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
// 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 |
There was a problem hiding this comment.
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"
Looks good. |
Ubuntu x64 leg has a failure: GC.Scenarios.DoublinkList.doublinkgen.doublinkgen |
Already there: #6574 |
This was causing a failure during dumping. Fixing it exposed another dumping failure in crossgen of System.Private.Corelib.dll on x86, due to a dead node. I've fixed the assert, but I believe the dead node should have been eliminated. I filed issue #8839 for this.
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. |
Fix initialization of resolution sets Commit migrated from dotnet/coreclr@d69811c
This was causing a failure during dumping.
Fixing it exposed another dumping failure in crossgen of System.Private.Corelib.dll on x86, due to a dead node. I've fixed the assert, but I believe the dead node should have been eliminated. I filed issue #8839 for this.
Fix #8824