[PHIElimination] Fix bug around $noreg assignment

[guy-david]

PR which introduced the bug: #131837.
Fixes a crash around dead registers which started in f5c62ee by verifying that the reused incoming register is also virtual.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-backend-aarch64

Author: Guy David (guy-david)

Changes

PR which introduced the bug: #131837.
Fixes a crash around dead registers which started in f5c62ee by verifying that the reused incoming register is also virtual.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/PHIElimination.cpp (+3-2)
• (modified) llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir (+51)

diff --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 959a1711727a4..640f3678d23f1 100644
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -582,8 +582,9 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
     }
 
     // Reuse an existing copy in the block if possible.
-    if (MachineInstr *DefMI = MRI->getUniqueVRegDef(SrcReg)) {
-      if (DefMI->isCopy() && DefMI->getParent() == &opBlock &&
+    if (IncomingReg.isVirtual()) {
+      MachineInstr *DefMI = MRI->getUniqueVRegDef(SrcReg);
+      if (DefMI && DefMI->isCopy() && DefMI->getParent() == &opBlock &&
           MRI->use_empty(SrcReg)) {
         DefMI->getOperand(0).setReg(IncomingReg);
         continue;
diff --git a/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir b/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir
index 75283d9a7b14a..6b026fbabc65e 100644
--- a/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir
+++ b/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir
@@ -66,3 +66,54 @@ body: |
     %b:gpr32 = PHI %a:gpr32, %bb.1, undef %undef:gpr32, %bb.0
 ...
 
+---
+name: copy_to_dead
+tracksRegLiveness: true
+body: |
+  ; CHECK-LABEL: name: copy_to_dead
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
+  ; CHECK-NEXT:   liveins: $wzr, $xzr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr32 = COPY $wzr
+  ; CHECK-NEXT:   dead [[COPY1:%[0-9]+]]:gpr64 = COPY $xzr
+  ; CHECK-NEXT:   TBZW killed [[COPY]], 0, %bb.2
+  ; CHECK-NEXT:   B %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.2(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[DEF:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+  ; CHECK-NEXT:   [[DEF1:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+  ; CHECK-NEXT:   B %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[DEF2:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+  ; CHECK-NEXT:   [[DEF3:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+  ; CHECK-NEXT:   B %bb.1
+  bb.0:
+    liveins: $wzr, $xzr
+
+    %9:gpr32 = COPY $wzr
+    dead %5:gpr64 = COPY $xzr
+    TBZW killed %9:gpr32, 0, %bb.2
+    B %bb.1
+
+  bb.1:
+    successors: %bb.2(0x80000000); %bb.2(100.00%)
+
+    dead %1:gpr64 = PHI undef %3:gpr64, %bb.2, undef %5:gpr64, %bb.0
+    dead %2:gpr64 = PHI undef %4:gpr64, %bb.2, undef %5:gpr64, %bb.0
+    B %bb.2
+
+  bb.2:
+    successors: %bb.1(0x80000000); %bb.1(100.00%)
+
+    dead %3:gpr64 = PHI undef %1:gpr64, %bb.1, undef %5:gpr64, %bb.0
+    dead %4:gpr64 = PHI undef %2:gpr64, %bb.1, undef %5:gpr64, %bb.0
+    B %bb.1
+
+...
+

[mikaelholmen]

Thanks for the quick fix!
I've verified that this solves the problem I reported in #131837 (comment)

Maybe you want someone who reviewed the original change to look at this too, but as far as I'm concerned this looks ok.

[fhahn]

LGTM, thanks