[InstrRef] Fix mismatch between LiveDebugValues and salvageCopySSA #124233
Conversation
The LiveDebugValues pass and the instruction selector (which calls) salvageCopySSA need to be consistent on what they consider a copy instruction. With llvm#75184, the definition of what a copy instruction is was narrowed for AArch64 to exclude a w->x ORR and treat it as a zero-extend rather than a copy However, to make sure LiveDebugValues still treats a w->x ORR as a copy, the new function, isCopyLikeInstr was created. We need to make sure that salvageCopySSA also calls that function. This patch addresses this mismatch.
rastogishubham
requested review from
jmorse,
adrian-prantl,
felipepiovezan and
dianqk
|
Since nothing breaks currently and I am able to bootstrap build clang with instruction referencing turned on, on AArch64, I was unable to come up with a testcase for this, I am not the best at writing MIR by hand, and we would need an ORR which translates to a W register being ORRed with the zero register and being written to an X register. Not sure how to do that. Any advice would be really appreciated, thank you! |
There was a problem hiding this comment.
LGTM; I think we can skip writing a test as the failure isn't really an illegal output from the compiler, it's an inconsistency between the way these two passes are implemented. We might test certain things are considered copies, but it'd be exhausting to test that everything considered a copy by salvageCopySSA is considered a copy by LiveDebugValues.
A bigger question is "how do we prevent this happening in the future", and I'm not sure we need to as (IIRC) these are the only two parts of instr-ref that care about copies.
…lvm#124233) The LiveDebugValues pass and the instruction selector (which calls salvageCopySSA) need to be consistent on what they consider a copy instruction. With llvm#75184, the definition of what a copy instruction is was narrowed for AArch64 to exclude a w->x ORR and treat it as a zero-extend rather than a copy However, to make sure LiveDebugValues still treats a w->x ORR as a copy, the new function, isCopyLikeInstr was created. We need to make sure that salvageCopySSA also calls that function. This patch addresses this mismatch. (cherry picked from commit 44c9e46)
The LiveDebugValues pass and the instruction selector (which calls salvageCopySSA) need to be consistent on what they consider a copy instruction. With #75184, the definition of what a copy instruction is was narrowed for AArch64 to exclude a w->x ORR and treat it as a zero-extend rather than a copy
However, to make sure LiveDebugValues still treats a w->x ORR as a copy, the new function, isCopyLikeInstr was created. We need to make sure that salvageCopySSA also calls that function.
This patch addresses this mismatch.