Ensure that we consistently copy otherRegs#128010
Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
There was a problem hiding this comment.
Pull request overview
This PR extends the JIT’s “copy register assignment” helper logic to more consistently propagate multi-register (“otherRegs”) state across additional node kinds (locals, multireg ops, and JIT intrinsics), and adjusts some multireg handling in register-count/dump code to rely less on FEATURE_MULTIREG_RET compile-time gating.
Changes:
- Add
CopyOtherRegshelpers forGenTreeLclVar,GenTreeMultiRegOp, andGenTreeJitIntrinsic. - Update
GenTree::CopyRegto copy multireg state for calls, multireg ops (32-bit), copy/reload nodes, HW intrinsics, and multireg locals. - Refactor
GenTree::GetRegisterDstCount, and makegtDispMultiRegCount/gtDispRegValavailable underDEBUGwithoutFEATURE_MULTIREG_RETgating.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/jit/gentree.h | Adds CopyOtherRegs helpers on several node types to support multireg state propagation. |
| src/coreclr/jit/gentree.cpp | Extends CopyReg logic and refactors GetRegisterDstCount; adjusts debug dump helpers’ feature gating. |
| src/coreclr/jit/compiler.h | Removes FEATURE_MULTIREG_RET gating for gtDispMultiRegCount declaration under DEBUG. |
|
Talked with @jakobbotsch and they suggested this may not be needed for anything but calls and may not even be needed there (possibly some arm32 weirdness) so I’m marking back as draft and will update this to avoid it everywhere but calls and assert the data is reg_na instead |
| copy->CopyRawCosts(tree); | ||
| copy->CopyReg(tree); | ||
|
|
||
| assert(tree->GetRegNum() == REG_NA); | ||
| assert(tree->GetRegTag() == GenTree::GT_REGTAG_NONE); | ||
|
|
| assert(exp->GetRegNum() == REG_NA); | ||
| assert(exp->GetRegTag() == GenTree::GT_REGTAG_NONE); | ||
|
|
|
Going to close this. Arm32 is definitely assigning the register state pre-LSRA and so removing this is very non-trivial. I will extract out the other cleanup I had down to not do repeated IsMultiReg() checks as that had a significant TP improvement for Arm64 |
No description provided.