[RISCV][MoveMerge] Don't copy kill flag when moving past an instruction that reads the register. #153644
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…on that reads the register. If we're moving the second copy before another instruction that reads the copied register, we need to clear the kill flag on the combined move. Fixes llvm#153598.
Member
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesIf we're moving the second copy before another instruction that reads the copied register, we need to clear the kill flag on the combined move. Fixes #153598. Full diff: https://github.com/llvm/llvm-project/pull/153644.diff 1 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp b/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp
index 7a2541a652b58..0d37db0138e47 100644
--- a/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp
+++ b/llvm/lib/Target/RISCV/RISCVMoveMerger.cpp
@@ -137,6 +137,11 @@ RISCVMoveMerge::mergePairedInsns(MachineBasicBlock::iterator I,
NextI = next_nodbg(NextI, E);
DebugLoc DL = I->getDebugLoc();
+ // Make a copy so we can update the kill flag in the MoveFromAToS case. The
+ // copied operand needs to be scoped outside the if since we make a pointer
+ // to it.
+ MachineOperand PairedSource = *PairedRegs.Source;
+
// The order of S-reg depends on which instruction holds A0, instead of
// the order of register pair.
// e,g.
@@ -147,8 +152,15 @@ RISCVMoveMerge::mergePairedInsns(MachineBasicBlock::iterator I,
// mv a1, s1 => cm.mva01s s2,s1
bool StartWithX10 = ARegInFirstPair == RISCV::X10;
if (isMoveFromAToS(Opcode)) {
- Sreg1 = StartWithX10 ? FirstPair.Source : PairedRegs.Source;
- Sreg2 = StartWithX10 ? PairedRegs.Source : FirstPair.Source;
+ // We are moving one of the copies earlier so its kill flag may become
+ // invalid. Clear the copied kill flag if there are any reads of the
+ // register between the new location and the old location.
+ for (auto It = std::next(I); It != Paired && PairedSource.isKill(); ++It)
+ if (It->readsRegister(PairedSource.getReg(), TRI))
+ PairedSource.setIsKill(false);
+
+ Sreg1 = StartWithX10 ? FirstPair.Source : &PairedSource;
+ Sreg2 = StartWithX10 ? &PairedSource : FirstPair.Source;
} else {
Sreg1 = StartWithX10 ? FirstPair.Destination : PairedRegs.Destination;
Sreg2 = StartWithX10 ? PairedRegs.Destination : FirstPair.Destination;
|
lenary
reviewed
Aug 14, 2025
Comment on lines
165
to
166
| Sreg1 = StartWithX10 ? FirstPair.Destination : PairedRegs.Destination; | ||
| Sreg2 = StartWithX10 ? PairedRegs.Destination : FirstPair.Destination; |
Member
There was a problem hiding this comment.
I don't understand why this case doesn't move a copy earlier, can you explain that further?
Collaborator
Author
There was a problem hiding this comment.
It does but we don’t copy the source operands. They’re implicit uses on the new instruction.
Member
There was a problem hiding this comment.
And the key thing is the implicit uses are not marked as a kill. Cool.
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/21236 Here is the relevant piece of the build log for the reference |
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 18, 2025
…on that reads the register. (llvm#153644) If we're moving the second copy before another instruction that reads the copied register, we need to clear the kill flag on the combined move. Fixes llvm#153598. (cherry picked from commit defbbf0)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If we're moving the second copy before another instruction that reads the copied register, we need to clear the kill flag on the combined move.
Fixes #153598.