Skip to content

Commit 11fbc6e

Browse files
committed
[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 llvm#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: llvm@059cead llvm@69cd121
1 parent 49cb25a commit 11fbc6e

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

llvm/lib/CodeGen/RegisterCoalescer.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,8 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
14751475
// The implicit-def of the super register may have been reduced to
14761476
// subregisters depending on the uses.
14771477

1478-
bool NewMIDefinesFullReg = false;
1478+
TinyPtrVector<MachineOperand *> NewMIImpDefDestReg;
1479+
[[maybe_unused]] unsigned NewMIOpCount = NewMI.getNumOperands();
14791480

14801481
SmallVector<MCRegister, 4> NewMIImplDefs;
14811482
for (unsigned i = NewMI.getDesc().getNumOperands(),
@@ -1486,7 +1487,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
14861487
assert(MO.isImplicit());
14871488
if (MO.getReg().isPhysical()) {
14881489
if (MO.getReg() == DstReg)
1489-
NewMIDefinesFullReg = true;
1490+
NewMIImpDefDestReg.push_back(&MO);
14901491

14911492
assert(MO.isImplicit() && MO.getReg().isPhysical() &&
14921493
(MO.isDead() ||
@@ -1640,9 +1641,32 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
16401641
// been asked for. If so it must implicitly define the whole thing.
16411642
assert(DstReg.isPhysical() &&
16421643
"Only expect virtual or physical registers in remat");
1644+
1645+
// When we're rematerializing into a not-quite-right register we already add
1646+
// the real definition as an implicit-def, but we should also be marking the
1647+
// "official" register as dead, since nothing else is going to use it as a
1648+
// result of this remat. Not doing this can affect pressure tracking.
16431649
NewMI.getOperand(0).setIsDead(true);
16441650

1645-
if (!NewMIDefinesFullReg) {
1651+
bool HasDefMatchingCopy = false;
1652+
if (!NewMIImpDefDestReg.empty()) {
1653+
// Assert to check MachineOperand*s have not been invalidated.
1654+
assert(
1655+
NewMIOpCount == NewMI.getNumOperands() &&
1656+
"Expected NewMI operands not to be appended/removed at this point");
1657+
// If NewMI has an implicit-def of a super-register of the CopyDstReg,
1658+
// we must also mark that as dead since it is not going to used as a
1659+
// result of this remat.
1660+
for (MachineOperand *MO : NewMIImpDefDestReg) {
1661+
if (MO->getReg() != CopyDstReg)
1662+
MO->setIsDead(true);
1663+
else
1664+
HasDefMatchingCopy = true;
1665+
}
1666+
}
1667+
1668+
// If NewMI does not already have an implicit-def CopyDstReg add one now.
1669+
if (!HasDefMatchingCopy) {
16461670
NewMI.addOperand(MachineOperand::CreateReg(
16471671
CopyDstReg, true /*IsDef*/, true /*IsImp*/, false /*IsKill*/));
16481672
}

llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,16 +167,14 @@ body: |
167167
RET 0, $rax
168168
...
169169
---
170-
; FIXME: `$al = COPY killed %4` should rematerialize as `dead $eax = MOV32r0 ... implicit-def $al`
171-
; not `dead $eax = MOV32r0 ... implicit-def $rax` (as the full $rax is not used).
172170
name: rematerialize_superregister_into_subregister_def_with_impdef_physreg
173171
body: |
174172
bb.0.entry:
175173
; CHECK-LABEL: name: rematerialize_superregister_into_subregister_def_with_impdef_physreg
176174
; CHECK: dead $esi = MOV32r0 implicit-def dead $eflags, implicit-def $rsi
177175
; CHECK-NEXT: dead $edx = MOV32r0 implicit-def dead $eflags, implicit-def $rdx
178176
; CHECK-NEXT: FAKE_USE implicit killed $rsi, implicit killed $rdx
179-
; CHECK-NEXT: dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $rax
177+
; CHECK-NEXT: dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def dead $rax, implicit-def $al
180178
; CHECK-NEXT: FAKE_USE implicit killed $al
181179
; CHECK-NEXT: $eax = MOV32r0 implicit-def dead $eflags
182180
; CHECK-NEXT: RET 0, $eax

0 commit comments

Comments
 (0)