Fix consume-order checking in codegen. #8601
Conversation
|
@CarolEidt @dotnet/jit-contrib PTAL |
| { | ||
| assert(operand->gtUseNum == -1); | ||
| operand->gtUseNum = useNum; | ||
| useNum++; |
There was a problem hiding this comment.
Although this works, we may want to set these use numbers during LSRA's traversal of a node's operands to help ensure that the correspondence between a node's use number and the relative order in which it is visited by LSRA is correct. Thoughts?
There was a problem hiding this comment.
This seems like a reasonable approach. If one were to get fancy, one could maintain the LsraLocation at which it is consumed, along with its operand index - but that would be more info than is needed here (though possibly handy for debugging purposes!)
pgavlin
force-pushed
the
gh7963
branch
2 times, most recently
from
b3329a4
to
d589d61
Compare
| } | ||
|
|
||
| assert((node->OperGet() == GT_CATCH_ARG) || ((node->gtDebugFlags & GTF_DEBUG_NODE_CG_CONSUMED) == 0)); |
There was a problem hiding this comment.
Re-enabling the checks revealed that we (always?) consume CATCH_ARG nodes twice on xarch: once in the codegen for the CATCH_ARG node itself and then once for the node that consumes the CATCH_ARG (if any). @BruceForstall @CarolEidt any idea why the processing for the CATCH_ARG node contains a call to genConsumeReg?
There was a problem hiding this comment.
I don't know - I'm not very familiar with the code for GT_CATCH_ARG but in general we shouldn't be consuming the current node in genCodeForTreeNode
There was a problem hiding this comment.
The switch to LIR invalidated the correspondence between a node's sequence number and the order in which it must be consumed with respect to other nodes. This in turn made the consume-order checks during code generation incorrect. This change introduces a new field, `gtUseNum`, that is used during code generation to check the order in which nodes are consumed. Re-enabling these checks revealed a bug in code generation for locked instructions on x86, which were consuming their operands out-of-order; this change also contains a fix for that bug. Fixes #7963.
Fix consume-order checking in codegen. Commit migrated from dotnet/coreclr@b038f90
The switch to LIR invalidated the correspondence between a node's
sequence number and the order in which it must be consumed with respect
to other nodes. This in turn made the consume-order checks during code
generation incorrect.
This change introduces a new field,
gtUseNum, that is used during codegeneration to check the order in which nodes are consumed. Re-enabling
these checks revealed a bug in code generation for locked instructions
on x86, which were consuming their operands out-of-order; this change
also contains a fix for that bug.
Fixes #7963.