[PATCH] D127516: RegisterCoalescer: Fix verifier error when merging copy of undef

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:1648
+  // copy introduced a live value.
+  if ((V && V->isPHIDef()) || (!V && !DstLI.liveAt(Idx) && !SrcLI.liveAt(Idx))) {
+    CopyMI->setDesc(TII->get(TargetOpcode::IMPLICIT_DEF));
----------------
MatzeB wrote:
> - This `DstLI.liveAt(Idx)` can only really be true if we assign to a subregister, can't it? (Otherwise the fact that we are about to (re-)assign the register means that it wouldn't be alive immediately before the copy (except for degenerate cases where the source and destination register of the COPY is the same)...
> 
> - Can `SrcLI.liveAt(Idx)` ever be true? Don't we filter for that above in lines 1624-1636?
In this case there are subranges in the destination register, but not the source.

You're right, this works with the SrcLI.liveAt check removed


================
Comment at: llvm/test/CodeGen/AMDGPU/undef-subreg-use-after-coalesce.mir:35-36
     ; CHECK-NEXT: dead %0.sub1:vreg_128 = V_MOV_B32_e32 0, implicit $exec
-    ; CHECK-NEXT: S_ENDPGM 0, implicit undef %2.sub1:vreg_64
+    ; CHECK-NEXT: [[DEF:%[0-9]+]]:vreg_64 = IMPLICIT_DEF
+    ; CHECK-NEXT: S_ENDPGM 0, implicit [[DEF]].sub1
     undef %0.sub0:vreg_128 = V_MOV_B32_e32 0, implicit $exec
----------------
MatzeB wrote:
> This isn't a case of a live-out value though? Shouldn't it continue to use an `undef` flag over an `IMPLICIT_DEF`?
This is the minor regression I mentioned


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127516/new/

https://reviews.llvm.org/D127516