[llvm] [RegisterCoalescer] Mark implicit-defs of super-registers as dead in remat (PR #159110)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-regalloc

Author: Benjamin Maxwell (MacDue)

<details>
<summary>Changes</summary>

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


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


2 Files Affected:

- (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+27-3) 
- (modified) llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir (+21-1) 


``````````diff
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index b8486f6560c5f0..d117edb6838bd9 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 b99c5fc8df0cb6..44a2aecdc36722 100644
--- a/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
+++ b/llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
@@ -165,5 +165,25 @@ body:             |
   bb.3:
     $rax = COPY %t3
     RET 0, $rax
-
 ...
+---
+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 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
+    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

``````````

</details>


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