[SystemZ] Replace SELRMux with COPY in case of identical operands. #125108
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
|
@llvm/pr-subscribers-backend-systemz Author: Jonas Paulsson (JonPsson1) ChangesIf both operands of a SELRMux use the same register which is killed, and the SELRMux is expanded to a jump Early IfConvert => Machine Common Subexpression Elimination => Regalloc => SystemZ Post Rewrite => // Broken: First COPY kills $r1, but is still live into and used in successor block. Machine Copy Propagation => // Acts on the kill flag with wrong-code resulting:
This is the same test case as first reported with #124542 and then #124768 with suspicions that turned out to be false. @dominik-steenken Full diff: https://github.com/llvm/llvm-project/pull/125108.diff 2 Files Affected:
diff --git a/llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp b/llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp
index e15f9027cc20956..6b6ec5b6b9dc81e 100644
--- a/llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp
@@ -107,6 +107,18 @@ void SystemZPostRewrite::selectSELRMux(MachineBasicBlock &MBB,
bool Src1IsHigh = SystemZ::isHighReg(Src1Reg);
bool Src2IsHigh = SystemZ::isHighReg(Src2Reg);
+ // In rare cases both sources are the same register (after
+ // machine-cse). This must be handled as it may lead to wrong-code (after
+ // machine-cp) if the kill flag on Src1 isn't cleared (with
+ // expandCondMove()).
+ if (Src1Reg == Src2Reg) {
+ BuildMI(*MBBI->getParent(), MBBI, MBBI->getDebugLoc(),
+ TII->get(SystemZ::COPY), DestReg)
+ .addReg(MBBI->getOperand(1).getReg(), getRegState(MBBI->getOperand(1)));
+ MBBI->eraseFromParent();
+ return;
+ }
+
// If sources and destination aren't all high or all low, we may be able to
// simplify the operation by moving one of the sources to the destination
// first. But only if this doesn't clobber the other source.
diff --git a/llvm/test/CodeGen/SystemZ/cond-move-10.mir b/llvm/test/CodeGen/SystemZ/cond-move-10.mir
new file mode 100644
index 000000000000000..1db960829729eab
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/cond-move-10.mir
@@ -0,0 +1,21 @@
+# RUN: llc -o - %s -mtriple=s390x-linux-gnu -mcpu=z15 -run-pass=systemz-post-rewrite \
+# RUN: 2>&1 | FileCheck %s
+
+# The SELRMux has two identical sources - replace with a copy instruction.
+# CHECK: name: fun0
+# CHECK: renamable $r1l = AHIMuxK killed renamable $r1l, -1, implicit-def dead $cc
+# CHECK-NEXT: CHIMux renamable $r5h, 9, implicit-def $cc
+# CHECK-NEXT: $r14h = COPY killed renamable $r1l
+---
+name: fun0
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $r1l, $r5h
+ renamable $r1l = AHIMuxK killed renamable $r1l, -1, implicit-def dead $cc
+ CHIMux renamable $r5h, 9, implicit-def $cc
+ renamable $r14h = SELRMux killed renamable $r1l, renamable $r1l, 14, 8, implicit $cc
+ $r14l = COPY killed renamable $r14h
+ $r14d = LGFR $r14l
+ Return implicit $r14d
+...
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
uweigand
approved these changes
Jan 31, 2025
|
/cherry-pick eb1a571 |
|
/pull-request #125236 |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 10, 2025
…lvm#125108) If both operands of a SELRMux use the same register which is killed, and the SELRMux is expanded to a jump sequence, a broken MIR results if the kill flag is not removed. This patch replaces the SELRMux with a COPY in these cases. (cherry picked from commit eb1a571)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 both operands of a SELRMux use the same register which is killed, and the SELRMux is expanded to a jump
sequence, a broken MIR results if the kill flag is not removed. In the CSmith test case, it happened like this:
Early IfConvert =>
Machine Common Subexpression Elimination =>
Regalloc =>
SystemZ Post Rewrite => // Broken: First COPY kills $r1, but is still live into and used in successor block.
Machine Copy Propagation => // Acts on the kill flag with wrong-code resulting:
This is the same test case as first reported with #124542 and then #124768 with suspicions that turned out to be false.
@dominik-steenken