[llvm] [CodeGen] Preserve IMPLICIT_DEF for subregister joins (PR #218348)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-backend-x86

Author: Karl Hylen (Hylen)

<details>
<summary>Changes</summary>

This is a followup to https://github.com/llvm/llvm-project/pull/204867.

In our downstream target using subregister liveness we observed LiveIntervals after register coalescing that made sense looking at the input before the coalescer but would have been recomputed differently, with a value reaching a use that it shouldn't. This was because of IMPLICIT_DEF removal. As a result of this we saw assertions in the later pass `RenameIndependentSubregs`.

I managed to write an X86 MIR test that shows the problem. Running it before the fix the IMPLICIT_DEF in bb.3 is removed and the `MOV16ri` value from bb.1 looks like it reaches the `sub_8bit_hi` use of `TEST8rr` in bb.5. But the intervals indicate the reachability represented by the input to the coalescer. I get `L0000000000000002 [48r,176B:0)`  with:
``` 
32B	bb.1:
...
48B	  %1:gr16_abcd = MOV16ri 1
...
176B	bb.3:
...
288B	bb.5:
...
320B	  TEST8rr %1.sub_8bit_hi:gr16_abcd, %1.sub_8bit_hi:gr16_abcd, implicit-def dead $eflags
```

Preserving the IMPLICIT_DEF makes the output of the coalescer agree with the live intervals and the input MIR.

Assisted-by: OpenAI Codex

I reviewed, understand, and validated all tool-generated changes and take responsibility for this contribution.

---
Full diff: https://github.com/llvm/llvm-project/pull/218348.diff


2 Files Affected:

- (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+7-4) 
- (added) llvm/test/CodeGen/X86/coalescer-preserve-subreg-implicit-def.mir (+60) 


``````````diff
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index fae504c7c9cba..e3c5dc2bf635a 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -2855,7 +2855,8 @@ JoinVals::ConflictResolution JoinVals::analyzeValue(unsigned ValNo,
     if (SubRangeJoin) {
       // We don't care about the lanes when joining subregister ranges.
       V.WriteLanes = V.ValidLanes = LaneBitmask::getLane(0);
-      if (DefMI->isImplicitDef()) {
+      // Preserve lane definitions in MIR when tracking subregister liveness.
+      if (DefMI->isImplicitDef() && !TrackSubRegLiveness) {
         V.ValidLanes = LaneBitmask::getNone();
         V.ErasableImplicitDef = true;
       }
@@ -2888,8 +2889,10 @@ JoinVals::ConflictResolution JoinVals::analyzeValue(unsigned ValNo,
         }
       }
 
-      // An IMPLICIT_DEF writes undef values.
-      if (DefMI->isImplicitDef()) {
+      // An IMPLICIT_DEF writes undef values. When tracking subregister
+      // liveness, treat it like an ordinary definition so the lane conflict
+      // checks below decide whether the join can preserve it.
+      if (DefMI->isImplicitDef() && !TrackSubRegLiveness) {
         // We normally expect IMPLICIT_DEF values to be live only until the end
         // of their block. If the value is really live longer and gets pruned in
         // another block, this flag is cleared again.
@@ -2995,7 +2998,7 @@ JoinVals::ConflictResolution JoinVals::analyzeValue(unsigned ValNo,
     return CR_Replace;
 
   // Check for simple erasable conflicts.
-  if (DefMI->isImplicitDef())
+  if (DefMI->isImplicitDef() && V.ErasableImplicitDef)
     return CR_Erase;
 
   // Include the non-conflict where DefMI is a coalescable copy that kills
diff --git a/llvm/test/CodeGen/X86/coalescer-preserve-subreg-implicit-def.mir b/llvm/test/CodeGen/X86/coalescer-preserve-subreg-implicit-def.mir
new file mode 100644
index 0000000000000..01f4a6ca61553
--- /dev/null
+++ b/llvm/test/CodeGen/X86/coalescer-preserve-subreg-implicit-def.mir
@@ -0,0 +1,60 @@
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass=register-coalescer \
+# RUN:   -enable-subreg-liveness -verify-coalescing -o - %s | FileCheck %s
+
+# An IMPLICIT_DEF must be preserved when a coalesced register tracks
+# subregister liveness. Otherwise the value defining the high byte in bb.1
+# also reaches the use in bb.5 along the bb.3 edge, where the high byte was
+# explicitly made undef. The low-byte use in bb.3 keeps another lane live.
+
+# CHECK-LABEL: name: preserve_subreg_implicit_def
+# CHECK: bb.3:
+# CHECK: [[REG:%[0-9]+]].sub_8bit_hi:gr16_abcd = IMPLICIT_DEF
+# CHECK-NEXT: TEST8rr [[REG]].sub_8bit, [[REG]].sub_8bit, implicit-def $eflags
+# CHECK: bb.5:
+# CHECK: TEST8rr [[REG]].sub_8bit_hi, [[REG]].sub_8bit_hi, implicit-def dead $eflags
+
+---
+name: preserve_subreg_implicit_def
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    successors: %bb.1(0x80000000)
+
+    JMP_1 %bb.1
+
+  bb.1:
+    successors: %bb.2(0x40000000), %bb.3(0x40000000)
+
+    %0:gr16_abcd = MOV16ri 1
+    TEST8rr %0.sub_8bit, %0.sub_8bit, implicit-def $eflags
+    JCC_1 %bb.3, 5, implicit killed $eflags
+
+  bb.2:
+    successors: %bb.5(0x80000000)
+
+    undef %1.sub_8bit_hi:gr16_abcd = COPY %0.sub_8bit_hi
+    %2:gr8_abcd_h = COPY %1.sub_8bit_hi
+    %3:gr8_abcd_h = COPY %2
+    JMP_1 %bb.5
+
+  bb.3:
+    successors: %bb.5(0x30000000), %bb.4(0x50000000)
+
+    %3:gr8_abcd_h = IMPLICIT_DEF
+    TEST8rr %0.sub_8bit, %0.sub_8bit, implicit-def $eflags
+    JCC_1 %bb.5, 5, implicit killed $eflags
+
+  bb.4:
+    successors: %bb.5(0x80000000)
+
+    %4:gr8_abcd_h = MOV8ri 2
+    %3:gr8_abcd_h = COPY %4
+
+  bb.5:
+    successors: %bb.1(0x80000000)
+
+    %5:gr8_abcd_h = COPY %3
+    TEST8rr %5, %5, implicit-def dead $eflags
+    JMP_1 %bb.1
+
+...

``````````

</details>


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