[llvm] [CodeGen] TwoAddress: only keep undef REG_SEQUENCE COPY when tracking subreg liveness (PR #202599)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-arm

Author: HelloWorldU

<details>
<summary>Changes</summary>

This patch fixes a register coalescer assertion that fires when lowering a `REG_SEQUENCE` with `undef` operands on targets that do not track subregister liveness.

Fixes #<!-- -->202263, a regression from the fix for #<!-- -->175596 (#<!-- -->175598).

## Background

In the optimized register-allocation pipeline, `TwoAddressInstructionPass` runs *before* `LiveIntervals` (the `EarlyLiveIntervals` pass ordering is off by default):

    PHIElimination
    if (EarlyLiveIntervals) LiveIntervals   // only with -early-live-intervals; off by default
    TwoAddressInstruction                    // -> LIS == null here
    RegisterCoalescer                        // LiveIntervals is computed as its dependency

So `eliminateRegSequence` normally lowers `REG_SEQUENCE` with no `LiveIntervals` available, and in that `!LIS` case it emits no `COPY` for an `undef` operand. On a target that tracks subregister liveness (e.g. AMDGPU), `LiveIntervals` later builds per-lane subranges, so the use of that undefined lane has no live subrange:

    *** Bad machine code: No live subrange at use ***

That was #<!-- -->175596, fixed by #<!-- -->175598: when LIS is unavailable, scan the use list and, if an `undef` lane is actually used later, keep the `COPY` so the lane gets a definition:

    undef %1.dsub_0:qpr = COPY undef %2:dpr

This works, but it quietly makes correctness depend on a second factor — **liveness granularity**, i.e. whether the target tracks subregister liveness:

* **Per-lane liveness** (tracked, e.g. AMDGPU): each lane has its own live range, so a used undef lane needs its *own* definition — keeping the `COPY` is necessary.
* **Whole-register liveness** (not tracked, e.g. ARM without MVE): the lane has no live range of its own; it is already covered by the register's liveness once the first partial def is marked read-undef — so the `COPY` is unnecessary.

On a non-tracking target the kept `COPY` is also actively harmful: the register coalescer erases a `COPY` of an undef value, which removes the lane's only definition again, recreating a use-without-def and asserting in `JoinVals::analyzeValue`:

      (TrackSubRegLiveness || V.RedefVNI) && "Instruction is reading nonexistent value"

That is #<!-- -->202263.


## Fix: only keep the COPY where it can survive

In `eliminateRegSequence`, gate the `!LIS` use-list scan on `MRI->shouldTrackSubRegLiveness(DstReg)` — the second factor above. Non-tracking targets then skip the `undef` COPY as they did before #<!-- -->175598 (the long-standing, correct behavior); tracking targets are unchanged.

On the ARM reproducer, the lowering goes from (asserts later):

    undef %1.dsub_0:qpr = COPY undef %2:dpr   ; erased by the coalescer -> crash
    %1.dsub_1:qpr       = COPY %0
    %3:dpr              = COPY %1.dsub_0

to (no spurious COPY; whole-register liveness covers dsub_0):

    undef %1.dsub_1:qpr = COPY %0
    %3:dpr              = COPY %1.dsub_0

## Testing & validation

* New `llvm/test/CodeGen/ARM/twoaddr-regsequence-undef-subreg-use.mir`, which fails without this change.
* Existing AMDGPU `twoaddr-regsequence-keep-copy-on-use.mir` still passes (tracking targets are unaffected).
* #<!-- -->175598 landed without modifying any existing test, so this path is barely exercised by the suite; restoring the non-tracking behavior is very unlikely to change existing test output. Full cross-target coverage is left to premerge CI.

## Related work

#<!-- -->178387 and #<!-- -->189153 explore an IMPLICIT_DEF-based approach (emit a subregister `IMPLICIT_DEF` and preserve it through coalescing). They target the AMDGPU / subregister-liveness path and operate on subranges, so they are largely
orthogonal to the non-tracking-target path fixed here. A similar IMPLICIT_DEF approach could also let used undef lanes work on non-tracking targets, but that is a larger change; this patch restores the known-good behavior with minimal risk.

## Note on the approach

Gating the scan on `shouldTrackSubRegLiveness` in the shared `TwoAddressInstructionPass` is a bit blunt and may not be the ideal long-term fix. Since the assertion really fires in the coalescer — on MIR that was well-formed until it erased the undef COPY — the more robust place to absorb this might be the coalescer itself, the direction @<!-- -->arsenm has leaned toward.
This patch is the smaller, low-risk regression fix, but I'm happy to move it into the coalescer if preferred.



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


2 Files Affected:

- (modified) llvm/lib/CodeGen/TwoAddressInstructionPass.cpp (+6-1) 
- (added) llvm/test/CodeGen/ARM/twoaddr-regsequence-undef-subreg-use.mir (+24) 


``````````diff
diff --git a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
index fb3014d87f40a..c5292580dccec 100644
--- a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
+++ b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
@@ -2057,8 +2057,13 @@ void TwoAddressInstructionImpl::eliminateRegSequence(
 
   // If there are no live intervals information, we scan the use list once
   // in order to find which subregisters are used.
+  // Only do this when the target tracks subregister liveness. Otherwise
+  // liveness is tracked per whole register, so a used undef lane is already
+  // covered by the register's liveness and does not need its own def. Keeping
+  // a COPY for it would be pointless: the register coalescer erases such an
+  // undef COPY, recreating a use-without-def and crashing (issue #202263).
   LaneBitmask UsedLanes = LaneBitmask::getNone();
-  if (!LIS) {
+  if (!LIS && MRI->shouldTrackSubRegLiveness(DstReg)) {
     for (MachineOperand &Use : MRI->use_nodbg_operands(DstReg)) {
       if (unsigned SubReg = Use.getSubReg())
         UsedLanes |= TRI->getSubRegIndexLaneMask(SubReg);
diff --git a/llvm/test/CodeGen/ARM/twoaddr-regsequence-undef-subreg-use.mir b/llvm/test/CodeGen/ARM/twoaddr-regsequence-undef-subreg-use.mir
new file mode 100644
index 0000000000000..f15e1a8f6c19c
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/twoaddr-regsequence-undef-subreg-use.mir
@@ -0,0 +1,24 @@
+# RUN: llc -mtriple=armv8a-unknown-linux-gnueabi -run-pass=twoaddressinstruction -o - %s | FileCheck %s
+
+# On a target that does not track subregister liveness, lowering a REG_SEQUENCE
+# with an undef operand must skip the COPY for that operand even when the
+# subregister is used later: liveness is tracked per whole register, so the
+# undef lane is already covered by the register's liveness and needs no def of
+# its own. Emitting a COPY of the undef value would later be erased by the
+# register coalescer, recreating a use-without-def and crashing (issue #202263).
+
+---
+name:            undef_regsequence_subreg_use
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: undef_regsequence_subreg_use
+    ; CHECK: [[VMOV:%[0-9]+]]:dpr = VMOVv2i32
+    ; CHECK-NEXT: undef [[REG:%[0-9]+]].dsub_1:qpr = COPY [[VMOV]]
+    ; CHECK-NEXT: %{{[0-9]+}}:dpr = COPY [[REG]].dsub_0
+    ; CHECK-NOT: COPY undef
+    %0:dpr = VMOVv2i32 0, 14 /* CC::al */, $noreg
+    %1:qpr = REG_SEQUENCE undef %2:dpr, %subreg.dsub_0, %0:dpr, %subreg.dsub_1
+    %3:dpr = COPY %1.dsub_0
+    VST1d64 $noreg, 0, %3:dpr, 14 /* CC::al */, $noreg
+...

``````````

</details>


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