[llvm] [RegisterCoalescer] Mark implicit-defs of super-registers as dead in remat (PR #159110)
Benjamin Maxwell via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 25 08:20:32 PDT 2025
https://github.com/MacDue updated https://github.com/llvm/llvm-project/pull/159110
>From 49cb25a210309b8b90e0ad5333c5eef146969ee5 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Tue, 16 Sep 2025 13:57:41 +0000
Subject: [PATCH 1/4] Precommit test
---
.../X86/rematerialize-sub-super-reg.mir | 24 ++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir b/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
index b99c5fc8df0cb..13b90e715a7ad 100644
--- a/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
+++ b/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
@@ -165,5 +165,27 @@ body: |
bb.3:
$rax = COPY %t3
RET 0, $rax
-
...
+---
+; FIXME: `$al = COPY killed %4` should rematerialize as `dead $eax = MOV32r0 ... implicit-def $al`
+; not `dead $eax = MOV32r0 ... implicit-def $rax` (as the full $rax is not used).
+name: rematerialize_superregister_into_subregister_def_with_impdef_physreg
+body: |
+ bb.0.entry:
+ ; CHECK-LABEL: name: rematerialize_superregister_into_subregister_def_with_impdef_physreg
+ ; CHECK: dead $esi = MOV32r0 implicit-def dead $eflags, implicit-def $rsi
+ ; CHECK-NEXT: dead $edx = MOV32r0 implicit-def dead $eflags, implicit-def $rdx
+ ; CHECK-NEXT: FAKE_USE implicit killed $rsi, implicit killed $rdx
+ ; CHECK-NEXT: dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $rax
+ ; CHECK-NEXT: FAKE_USE implicit killed $al
+ ; CHECK-NEXT: $eax = MOV32r0 implicit-def dead $eflags
+ ; CHECK-NEXT: RET 0, $eax
+ undef %1.sub_32bit:gr64_with_sub_8bit = MOV32r0 implicit-def dead $eflags, implicit-def %1
+ $rsi = COPY %1
+ $rdx = COPY %1
+ FAKE_USE implicit killed $rsi, implicit killed $rdx
+ %4:gr8 = COPY killed %1.sub_8bit
+ $al = COPY killed %4
+ FAKE_USE implicit killed $al
+ $eax = MOV32r0 implicit-def dead $eflags
+ RET 0, killed $eax
>From 11fbc6e2509e4af3e6de23cb7b3f2c5882caa999 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Tue, 16 Sep 2025 14:42:02 +0000
Subject: [PATCH 2/4] [RegisterCoalescer] Mark implicit-defs of super-registers
as dead in remat
Currently, something like:
```
$eax = MOV32ri -11, implicit-def $rax
%al = COPY $eax
```
Can be rematerialized as:
```
dead $eax = MOV32ri -11, implicit-def $rax
```
Which marks the full $rax as used, not just $al.
With this change, this is rematerialized as:
```
dead $eax = MOV32ri -11, implicit-def dead $rax, implicit-def $al
```
To indicate that only $al is used. This issue is latent right now, but
is exposed when #134408 is applied, as it results in the register
pressure being incorrectly calculated.
I think this change is in line with past fixes in this area, notably:
https://github.com/llvm/llvm-project/commit/059cead5ed7aa11ce1eae0bcc751ea0d1e23ea75
https://github.com/llvm/llvm-project/commit/69cd121dd9945429b565b6a5eb8719130de880a7
---
llvm/lib/CodeGen/RegisterCoalescer.cpp | 30 +++++++++++++++++--
.../X86/rematerialize-sub-super-reg.mir | 4 +--
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index b8486f6560c5f..d117edb6838bd 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1475,7 +1475,8 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
// The implicit-def of the super register may have been reduced to
// subregisters depending on the uses.
- bool NewMIDefinesFullReg = false;
+ TinyPtrVector<MachineOperand *> NewMIImpDefDestReg;
+ [[maybe_unused]] unsigned NewMIOpCount = NewMI.getNumOperands();
SmallVector<MCRegister, 4> NewMIImplDefs;
for (unsigned i = NewMI.getDesc().getNumOperands(),
@@ -1486,7 +1487,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
assert(MO.isImplicit());
if (MO.getReg().isPhysical()) {
if (MO.getReg() == DstReg)
- NewMIDefinesFullReg = true;
+ NewMIImpDefDestReg.push_back(&MO);
assert(MO.isImplicit() && MO.getReg().isPhysical() &&
(MO.isDead() ||
@@ -1640,9 +1641,32 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
// been asked for. If so it must implicitly define the whole thing.
assert(DstReg.isPhysical() &&
"Only expect virtual or physical registers in remat");
+
+ // When we're rematerializing into a not-quite-right register we already add
+ // the real definition as an implicit-def, but we should also be marking the
+ // "official" register as dead, since nothing else is going to use it as a
+ // result of this remat. Not doing this can affect pressure tracking.
NewMI.getOperand(0).setIsDead(true);
- if (!NewMIDefinesFullReg) {
+ bool HasDefMatchingCopy = false;
+ if (!NewMIImpDefDestReg.empty()) {
+ // Assert to check MachineOperand*s have not been invalidated.
+ assert(
+ NewMIOpCount == NewMI.getNumOperands() &&
+ "Expected NewMI operands not to be appended/removed at this point");
+ // If NewMI has an implicit-def of a super-register of the CopyDstReg,
+ // we must also mark that as dead since it is not going to used as a
+ // result of this remat.
+ for (MachineOperand *MO : NewMIImpDefDestReg) {
+ if (MO->getReg() != CopyDstReg)
+ MO->setIsDead(true);
+ else
+ HasDefMatchingCopy = true;
+ }
+ }
+
+ // If NewMI does not already have an implicit-def CopyDstReg add one now.
+ if (!HasDefMatchingCopy) {
NewMI.addOperand(MachineOperand::CreateReg(
CopyDstReg, true /*IsDef*/, true /*IsImp*/, false /*IsKill*/));
}
diff --git a/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir b/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
index 13b90e715a7ad..44a2aecdc3672 100644
--- a/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
+++ b/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
@@ -167,8 +167,6 @@ body: |
RET 0, $rax
...
---
-; FIXME: `$al = COPY killed %4` should rematerialize as `dead $eax = MOV32r0 ... implicit-def $al`
-; not `dead $eax = MOV32r0 ... implicit-def $rax` (as the full $rax is not used).
name: rematerialize_superregister_into_subregister_def_with_impdef_physreg
body: |
bb.0.entry:
@@ -176,7 +174,7 @@ body: |
; CHECK: dead $esi = MOV32r0 implicit-def dead $eflags, implicit-def $rsi
; CHECK-NEXT: dead $edx = MOV32r0 implicit-def dead $eflags, implicit-def $rdx
; CHECK-NEXT: FAKE_USE implicit killed $rsi, implicit killed $rdx
- ; CHECK-NEXT: dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $rax
+ ; CHECK-NEXT: dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def dead $rax, implicit-def $al
; CHECK-NEXT: FAKE_USE implicit killed $al
; CHECK-NEXT: $eax = MOV32r0 implicit-def dead $eflags
; CHECK-NEXT: RET 0, $eax
>From af055e9ce0bff71b4213ac5a75a930af3ae2bde3 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Wed, 24 Sep 2025 12:01:21 +0000
Subject: [PATCH 3/4] Rework
---
llvm/lib/CodeGen/RegisterCoalescer.cpp | 40 +++++++++-----------------
1 file changed, 14 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index d117edb6838bd..105d25f5577f3 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1474,11 +1474,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
//
// The implicit-def of the super register may have been reduced to
// subregisters depending on the uses.
-
- TinyPtrVector<MachineOperand *> NewMIImpDefDestReg;
- [[maybe_unused]] unsigned NewMIOpCount = NewMI.getNumOperands();
-
- SmallVector<MCRegister, 4> NewMIImplDefs;
+ SmallVector<std::pair<unsigned, MCRegister>, 4> NewMIImplDefs;
for (unsigned i = NewMI.getDesc().getNumOperands(),
e = NewMI.getNumOperands();
i != e; ++i) {
@@ -1486,9 +1482,6 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
if (MO.isReg() && MO.isDef()) {
assert(MO.isImplicit());
if (MO.getReg().isPhysical()) {
- if (MO.getReg() == DstReg)
- NewMIImpDefDestReg.push_back(&MO);
-
assert(MO.isImplicit() && MO.getReg().isPhysical() &&
(MO.isDead() ||
(DefSubIdx &&
@@ -1496,7 +1489,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
MCRegister((unsigned)NewMI.getOperand(0).getReg())) ||
TRI->isSubRegisterEq(NewMI.getOperand(0).getReg(),
MO.getReg())))));
- NewMIImplDefs.push_back(MO.getReg().asMCReg());
+ NewMIImplDefs.push_back({i, MO.getReg().asMCReg()});
} else {
assert(MO.getReg() == NewMI.getOperand(0).getReg());
@@ -1649,27 +1642,22 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
NewMI.getOperand(0).setIsDead(true);
bool HasDefMatchingCopy = false;
- if (!NewMIImpDefDestReg.empty()) {
- // Assert to check MachineOperand*s have not been invalidated.
- assert(
- NewMIOpCount == NewMI.getNumOperands() &&
- "Expected NewMI operands not to be appended/removed at this point");
- // If NewMI has an implicit-def of a super-register of the CopyDstReg,
- // we must also mark that as dead since it is not going to used as a
- // result of this remat.
- for (MachineOperand *MO : NewMIImpDefDestReg) {
- if (MO->getReg() != CopyDstReg)
- MO->setIsDead(true);
- else
- HasDefMatchingCopy = true;
- }
+ for (auto [OpIndex, Reg] : NewMIImplDefs) {
+ if (Reg != DstReg.asMCReg())
+ continue;
+ // Also, if CopyDstReg is a sub-register of DstReg (and it is defined), we
+ // must mark DstReg as dead since it is not going to used as a result of
+ // this remat.
+ if (DstReg != CopyDstReg)
+ NewMI.getOperand(OpIndex).setIsDead(true);
+ else
+ HasDefMatchingCopy = true;
}
// If NewMI does not already have an implicit-def CopyDstReg add one now.
- if (!HasDefMatchingCopy) {
+ if (!HasDefMatchingCopy)
NewMI.addOperand(MachineOperand::CreateReg(
CopyDstReg, true /*IsDef*/, true /*IsImp*/, false /*IsKill*/));
- }
// Record small dead def live-ranges for all the subregisters
// of the destination register.
@@ -1700,7 +1688,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
NewMI.addOperand(MO);
SlotIndex NewMIIdx = LIS->getInstructionIndex(NewMI);
- for (MCRegister Reg : NewMIImplDefs) {
+ for (MCRegister Reg : make_second_range(NewMIImplDefs)) {
for (MCRegUnit Unit : TRI->regunits(Reg))
if (LiveRange *LR = LIS->getCachedRegUnit(Unit))
LR->createDeadDef(NewMIIdx.getRegSlot(), LIS->getVNInfoAllocator());
>From 72282a82c81cf99934dd20cdd7f124ab9f2d6660 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Thu, 25 Sep 2025 15:19:42 +0000
Subject: [PATCH 4/4] Fixups
---
llvm/lib/CodeGen/RegisterCoalescer.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 105d25f5577f3..f7165c535b1d2 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1474,7 +1474,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
//
// The implicit-def of the super register may have been reduced to
// subregisters depending on the uses.
- SmallVector<std::pair<unsigned, MCRegister>, 4> NewMIImplDefs;
+ SmallVector<std::pair<unsigned, Register>, 4> NewMIImplDefs;
for (unsigned i = NewMI.getDesc().getNumOperands(),
e = NewMI.getNumOperands();
i != e; ++i) {
@@ -1489,7 +1489,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
MCRegister((unsigned)NewMI.getOperand(0).getReg())) ||
TRI->isSubRegisterEq(NewMI.getOperand(0).getReg(),
MO.getReg())))));
- NewMIImplDefs.push_back({i, MO.getReg().asMCReg()});
+ NewMIImplDefs.push_back({i, MO.getReg()});
} else {
assert(MO.getReg() == NewMI.getOperand(0).getReg());
@@ -1643,7 +1643,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
bool HasDefMatchingCopy = false;
for (auto [OpIndex, Reg] : NewMIImplDefs) {
- if (Reg != DstReg.asMCReg())
+ if (Reg != DstReg)
continue;
// Also, if CopyDstReg is a sub-register of DstReg (and it is defined), we
// must mark DstReg as dead since it is not going to used as a result of
@@ -1688,8 +1688,8 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
NewMI.addOperand(MO);
SlotIndex NewMIIdx = LIS->getInstructionIndex(NewMI);
- for (MCRegister Reg : make_second_range(NewMIImplDefs)) {
- for (MCRegUnit Unit : TRI->regunits(Reg))
+ for (Register Reg : make_second_range(NewMIImplDefs)) {
+ for (MCRegUnit Unit : TRI->regunits(Reg.asMCReg()))
if (LiveRange *LR = LIS->getCachedRegUnit(Unit))
LR->createDeadDef(NewMIIdx.getRegSlot(), LIS->getVNInfoAllocator());
}
More information about the llvm-commits
mailing list