[llvm] [SystemZ] Replace SELRMux with COPY in case of identical operands. (PR #125108)

via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 30 11:42:32 PST 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-systemz

Author: Jonas Paulsson (JonPsson1)

<details>
<summary>Changes</summary>

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 =>
```
  %45:grx32bit = AHIMuxK %26:gr32bit, -1, implicit-def dead $cc
  %44:grx32bit = AHIMuxK %26:gr32bit, -1, implicit-def dead $cc
  CHIMux %41:grx32bit, 9, implicit-def $cc
  %106:grx32bit = SELRMux %44:grx32bit, %45:grx32bit, 14, 8, implicit $cc
```
Machine Common Subexpression Elimination =>
```
  %45:grx32bit = AHIMuxK %26:gr32bit, -1, implicit-def dead $cc
  CHIMux %41:grx32bit, 9, implicit-def $cc
  %106:grx32bit = SELRMux %45:grx32bit, %45:grx32bit, 14, 8, implicit $cc
```
Regalloc =>
```
2528B     renamable $r1l = AHIMuxK killed renamable $r1l, -1, implicit-def dead $cc
2544B     CHIMux renamable $r5h, 9, implicit-def $cc
2560B     renamable $r14h = SELRMux killed renamable $r1l, renamable $r1l, 14, 8, implicit $cc
```
SystemZ Post Rewrite =>  // Broken: First COPY kills $r1, but is still live into and used in successor block.
```
  renamable $r1l = AHIMuxK killed renamable $r1l, -1, implicit-def dead $cc
  CHIMux renamable $r5h, 9, implicit-def $cc
  $r14h = COPY killed renamable $r1l
  BRC 14, 6, %bb.26, implicit $cc

bb.27.cleanup22.i:
; predecessors: %bb.14
  successors: %bb.26(0x80000000); %bb.26(100.00%)
  liveins: $r1l, $r2d, $r2l, $r2h, $r4d, $r4l, $r4h, $r6d, $r6l, $r6h, $r8d, $r8l, $r8h, $r10d, $r10l, $r10h, $r0h, $r13h, $r14h, $r3l, $r5l, $r12q, $r12d, $r12l, $r12h, $r13d, $r13l, $r5h, $cc
  $r14h = COPY renamable $r1l

bb.26.cleanup22.i:
```
Machine Copy Propagation =>  // Acts on the kill flag with wrong-code resulting:
```
  $r14h = AHIMuxK killed renamable $r1l, -1, implicit-def dead $cc
  CHIMux renamable $r5h, 9, implicit-def $cc
  BRC 14, 6, %bb.26, implicit $cc

bb.27.cleanup22.i:
; predecessors: %bb.14
  successors: %bb.26(0x80000000); %bb.26(100.00%)
  liveins: $r1l, $r2d, $r2l, $r2h, $r4d, $r4l, $r4h, $r6d, $r6l, $r6h, $r8d, $r8l, $r8h, $r10d, $r10l, $r10h, $r0h, $r13h, $r14h, $r3l, $r5l, $r12q, $r12d, $r12l, $r12h, $r13d, $r13l, $r5h, $cc
  $r14h = COPY renamable $r1l

bb.26.cleanup22.i:
```

- This is very rare, so no particular optimization would be needed, although it seems most simple to replace with a COPY. The Cmp will remain, but as this is very rare, it probably isn't worth handling it.
- With LOCRMux, if both sources are the same, then all three are the same, so this does not happen with LOC.

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:

- (modified) llvm/lib/Target/SystemZ/SystemZPostRewrite.cpp (+12) 
- (added) llvm/test/CodeGen/SystemZ/cond-move-10.mir (+21) 


``````````diff
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
+...

``````````

</details>


https://github.com/llvm/llvm-project/pull/125108


More information about the llvm-commits mailing list