Skip to content

[CodeGen] Fix handling dead redefs in finalizeBundle #157427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 10, 2025
Merged

[CodeGen] Fix handling dead redefs in finalizeBundle #157427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39f358f
Precommit test
jayfoad Sep 8, 2025
4ed7239
[CodeGen] Fix handling dead redefs in finalizeBundle
jayfoad Sep 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions llvm/lib/CodeGen/MachineInstrBundle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) {
Expand All @@ -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
Expand All @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/AArch64/blr-bti-preserves-operands.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an unexpected change. I am not sure what it means for BL to have both implicit-def $lr and implicit-def dead $lr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a definite lack of rules about how to interpret dead flags. In particular I don't know what dead means when you have aliasing register defs with mixed dead flags

# CHECK: HINT 36
# CHECK: }
Expand Down
18 changes: 18 additions & 0 deletions llvm/test/CodeGen/AMDGPU/finalizebundle.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
...