[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
Tue Sep 16 08:19:37 PDT 2025
https://github.com/MacDue created https://github.com/llvm/llvm-project/pull/159110
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
>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/2] 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/2] [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
More information about the llvm-commits
mailing list