[llvm] [CodeGen] Fix handling dead redefs in finalizeBundle (PR #157427)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-aarch64

Author: Jay Foad (jayfoad)

<details>
<summary>Changes</summary>

A dead redefinition should override any earlier non-dead definition
inside a bundle.

Also remove KilledDefSet since it can be folded into DeadDefSet.


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


3 Files Affected:

- (modified) llvm/lib/CodeGen/MachineInstrBundle.cpp (+7-9) 
- (modified) llvm/test/CodeGen/AArch64/blr-bti-preserves-operands.mir (+1-1) 
- (modified) llvm/test/CodeGen/AMDGPU/finalizebundle.mir (+18) 


``````````diff
diff --git a/llvm/lib/CodeGen/MachineInstrBundle.cpp b/llvm/lib/CodeGen/MachineInstrBundle.cpp
index 5830ecfe4aa20..da29ffc9d2fed 100644
--- a/llvm/lib/CodeGen/MachineInstrBundle.cpp
+++ b/llvm/lib/CodeGen/MachineInstrBundle.cpp
@@ -133,7 +133,6 @@ void llvm::finalizeBundle(MachineBasicBlock &MBB,
   SmallSetVector<Register, 32> LocalDefs;
   BitVector LocalDefsP(TRI->getNumRegUnits());
   SmallSet<Register, 8> DeadDefSet;
-  SmallSet<Register, 16> KilledDefSet;
   SmallSetVector<Register, 8> ExternUses;
   SmallSet<Register, 8> KilledUseSet;
   SmallSet<Register, 8> UndefUseSet;
@@ -151,7 +150,7 @@ void llvm::finalizeBundle(MachineBasicBlock &MBB,
         MO.setIsInternalRead();
         if (MO.isKill()) {
           // Internal def is now killed.
-          KilledDefSet.insert(Reg);
+          DeadDefSet.insert(Reg);
         }
       } else {
         if (ExternUses.insert(Reg)) {
@@ -171,19 +170,18 @@ void llvm::finalizeBundle(MachineBasicBlock &MBB,
         continue;
 
       if (LocalDefs.insert(Reg)) {
-        if (MO.isDead())
-          DeadDefSet.insert(Reg);
-        else if (Reg.isPhysical())
+        if (!MO.isDead() && Reg.isPhysical()) {
           for (MCRegUnit Unit : TRI->regunits(Reg.asMCReg()))
             LocalDefsP.set(Unit);
+        }
       } else {
-        // Re-defined inside the bundle, it's no longer killed.
-        KilledDefSet.erase(Reg);
         if (!MO.isDead()) {
-          // Previously defined but dead.
+          // Re-defined inside the bundle, it's no longer dead.
           DeadDefSet.erase(Reg);
         }
       }
+      if (MO.isDead())
+        DeadDefSet.insert(Reg);
     }
 
     // Set FrameSetup/FrameDestroy for the bundle. If any of the instructions
@@ -196,7 +194,7 @@ void llvm::finalizeBundle(MachineBasicBlock &MBB,
 
   for (Register Reg : LocalDefs) {
     // If it's not live beyond end of the bundle, mark it dead.
-    bool isDead = DeadDefSet.contains(Reg) || KilledDefSet.contains(Reg);
+    bool isDead = DeadDefSet.contains(Reg);
     MIB.addReg(Reg, getDefRegState(true) | getDeadRegState(isDead) |
                         getImplRegState(true));
   }
diff --git a/llvm/test/CodeGen/AArch64/blr-bti-preserves-operands.mir b/llvm/test/CodeGen/AArch64/blr-bti-preserves-operands.mir
index f41e590c870a4..a4a392c41ec9a 100644
--- a/llvm/test/CodeGen/AArch64/blr-bti-preserves-operands.mir
+++ b/llvm/test/CodeGen/AArch64/blr-bti-preserves-operands.mir
@@ -8,7 +8,7 @@
 # The arguments to the call must become implicit arguments, because the branch
 # only expects to get 1 explicit operand which is the branch target.
 
-# CHECK:    BUNDLE implicit-def $lr, implicit-def $sp, implicit $sp, implicit $x0, implicit $w1 {
+# CHECK:    BUNDLE implicit-def dead $lr, implicit-def $sp, implicit $sp, implicit $x0, implicit $w1 {
 # CHECK:      BL @_setjmp, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0, implicit $w1, implicit-def dead $lr, implicit $sp, implicit-def $sp
 # CHECK:      HINT 36
 # CHECK:    }
diff --git a/llvm/test/CodeGen/AMDGPU/finalizebundle.mir b/llvm/test/CodeGen/AMDGPU/finalizebundle.mir
index 46ca6d347781e..0548bcf304c32 100644
--- a/llvm/test/CodeGen/AMDGPU/finalizebundle.mir
+++ b/llvm/test/CodeGen/AMDGPU/finalizebundle.mir
@@ -16,3 +16,21 @@ body: |
     $vgpr2_vgpr3 = V_LSHLREV_B64_pseudo_e32 1, $vgpr0_vgpr1, implicit $exec
     $vgpr3_vgpr4 = V_LSHLREV_B64_pseudo_e32 1, $vgpr1_vgpr2, implicit $exec
 ...
+
+---
+name: test_dead_redef
+body: |
+  bb.0:
+    liveins: $vgpr0
+    ; CHECK-LABEL: name: test_dead_redef
+    ; CHECK: liveins: $vgpr0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: BUNDLE implicit-def dead $vgpr1, implicit-def $vgpr0, implicit $vgpr0, implicit $exec {
+    ; CHECK-NEXT:   $vgpr1 = V_MOV_B32_e32 $vgpr0, implicit $exec
+    ; CHECK-NEXT:   $vgpr0 = V_MOV_B32_e32 internal $vgpr1, implicit $exec
+    ; CHECK-NEXT:   dead $vgpr1 = V_MOV_B32_e32 internal $vgpr0, implicit $exec
+    ; CHECK-NEXT: }
+    $vgpr1 = V_MOV_B32_e32 $vgpr0, implicit $exec
+    $vgpr0 = V_MOV_B32_e32 $vgpr1, implicit $exec
+    dead $vgpr1 = V_MOV_B32_e32 $vgpr0, implicit $exec
+...

``````````

</details>


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