[PATCH] D156343: RegisterCoalescer: Avoid redundant implicit-def on rematerialize

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: qcolombet, MatzeB, kparzysz, bogner, lhames, craig.topper, RKSimon.
Herald added subscribers: pengfei, tpr, hiraditya.
Herald added a project: All.
arsenm requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

If this was coalescing a def of a subregister with a def of the super
register, it was introducing a redundant super-register def and
marking the subregister def as dead.

      

Resulting in something like:

  
  dead $eax = MOVr0, implicit-def $rax, implicit-def $rax


Avoid this by checking if the new instruction already has the super
def, so we end up with this instead:

  
  dead $eax = MOVr0, implicit-def $rax


The dead flag looks suspicious to me, seems like it's easy to buggily
interpret dead def of subreg and a non-dead def of an aliasing
register. It seems to be intentional though.


https://reviews.llvm.org/D156343

Files:
  llvm/lib/CodeGen/RegisterCoalescer.cpp
  llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir


Index: llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
===================================================================
--- llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
+++ llvm/test/CodeGen/X86/rematerialize-sub-super-reg.mir
@@ -116,8 +116,6 @@
 
 # Handle that rematerializing an instruction with an implicit def of a
 # virtual super register into a physical register works.
-#
-# FIXME: Resulting rematerializing has a redundant implicit-def
 ---
 name:            rematerialize_subregister_into_superreg_def_with_impdef_physreg
 tracksRegLiveness: true
@@ -134,7 +132,7 @@
   ; CHECK-NEXT: bb.1:
   ; CHECK-NEXT:   successors: %bb.1(0x80000000)
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT:   dead $eax = MOV32ri -11, implicit-def $rax, implicit-def $rax
+  ; CHECK-NEXT:   dead $eax = MOV32ri -11, implicit-def $rax
   ; CHECK-NEXT:   CMP64ri8 %t3, 1, implicit-def $eflags
   ; CHECK-NEXT:   JCC_1 %bb.1, 4, implicit killed $eflags
   ; CHECK-NEXT:   RET 0, $rax
Index: llvm/lib/CodeGen/RegisterCoalescer.cpp
===================================================================
--- llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1416,6 +1416,8 @@
   // $edi = MOV32r0 implicit-def dead $eflags, implicit-def $rdi
   // undef %0.sub_32bit = MOV32r0 implicit-def dead $eflags, implicit-def %0
 
+  bool NewMIDefinesFullReg = false;
+
   SmallVector<MCRegister, 4> NewMIImplDefs;
   for (unsigned i = NewMI.getDesc().getNumOperands(),
                 e = NewMI.getNumOperands();
@@ -1424,6 +1426,9 @@
     if (MO.isReg() && MO.isDef()) {
       assert(MO.isImplicit());
       if (MO.getReg().isPhysical()) {
+        if (MO.getReg() == DstReg)
+          NewMIDefinesFullReg = true;
+
         assert(MO.isImplicit() && MO.getReg().isPhysical() &&
                (MO.isDead() ||
                 (DefSubIdx && (TRI->getSubReg(MO.getReg(), DefSubIdx) ==
@@ -1552,8 +1557,12 @@
     assert(DstReg.isPhysical() &&
            "Only expect virtual or physical registers in remat");
     NewMI.getOperand(0).setIsDead(true);
-    NewMI.addOperand(MachineOperand::CreateReg(
-        CopyDstReg, true /*IsDef*/, true /*IsImp*/, false /*IsKill*/));
+
+    if (!NewMIDefinesFullReg) {
+      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.
     // Otherwise, variables that live through may miss some


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156343.544399.patch
Type: text/x-patch
Size: 2536 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230726/505f3852/attachment.bin>