[llvm] 362d5ff - [CodeGen] commuteInstruction should update implicit-def (#131361)

[via llvm-commits]

Author: Sander de Smalen
Date: 2025-07-10T15:30:02+01:00
New Revision: 362d5ffa8d2f521fc2912665270eba15f3c1bf56

URL: https://github.com/llvm/llvm-project/commit/362d5ffa8d2f521fc2912665270eba15f3c1bf56
DIFF: https://github.com/llvm/llvm-project/commit/362d5ffa8d2f521fc2912665270eba15f3c1bf56.diff

LOG: [CodeGen] commuteInstruction should update implicit-def (#131361)

When the RegisterCoalescer adds an implicit-def when coalescing
a SUBREG_TO_REG (#123632), this causes issues when removing other
COPY nodes by commuting the instruction because it doesn't take
the implicit-def into consideration. This PR fixes that.

Added: 
    llvm/test/CodeGen/X86/coalesce-commutative-implicit-def.mir

Modified: 
    llvm/lib/CodeGen/TargetInstrInfo.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp
index 8b82deb2a9d83..660a1a4d7ec47 100644
--- a/llvm/lib/CodeGen/TargetInstrInfo.cpp
+++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp
@@ -238,7 +238,14 @@ MachineInstr *TargetInstrInfo::commuteInstructionImpl(MachineInstr &MI,
   }
 
   if (HasDef) {
-    CommutedMI->getOperand(0).setReg(Reg0);
+    // Use `substituteRegister` so that for a case like this:
+    //   %0.sub = INST %0.sub(tied), %1.sub, implicit-def %0
+    // the implicit-def is also updated, to result in:
+    //   %1.sub = INST %1.sub(tied), %0.sub, implicit-def %1
+    const TargetRegisterInfo &TRI =
+        *MI.getMF()->getSubtarget().getRegisterInfo();
+    Register FromReg = CommutedMI->getOperand(0).getReg();
+    CommutedMI->substituteRegister(FromReg, Reg0, /*SubRegIdx=*/0, TRI);
     CommutedMI->getOperand(0).setSubReg(SubReg0);
   }
   CommutedMI->getOperand(Idx2).setReg(Reg1);

diff  --git a/llvm/test/CodeGen/X86/coalesce-commutative-implicit-def.mir b/llvm/test/CodeGen/X86/coalesce-commutative-implicit-def.mir
new file mode 100644
index 0000000000000..fe1235fe94f85
--- /dev/null
+++ b/llvm/test/CodeGen/X86/coalesce-commutative-implicit-def.mir
@@ -0,0 +1,37 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=x86_64 -run-pass=register-coalescer -o - %s | FileCheck %s
+
+# When the coalescer removes the COPY by commuting the operands of the AND, it should also update the `implicit-def` of the destination register.
+---
+name: implicit_def_dst
+tracksRegLiveness: true
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: implicit_def_dst
+    ; CHECK: [[MOV64rm:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    ; CHECK-NEXT: [[MOV64rm1:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    ; CHECK-NEXT: [[MOV64rm:%[0-9]+]].sub_32bit:gr64_with_sub_8bit = AND32rr [[MOV64rm]].sub_32bit, [[MOV64rm1]].sub_32bit, implicit-def dead $eflags, implicit-def [[MOV64rm]]
+    ; CHECK-NEXT: RET 0, implicit [[MOV64rm]]
+    %0:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    %1:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    %1.sub_32bit:gr64_with_sub_8bit = AND32rr %1.sub_32bit:gr64_with_sub_8bit, %0.sub_32bit:gr64_with_sub_8bit, implicit-def dead $eflags, implicit-def %1:gr64_with_sub_8bit
+    %0:gr64_with_sub_8bit = COPY %1:gr64_with_sub_8bit
+    RET 0, implicit %0
+...
+# In case the MIR for some reason contains more than one implicit-def of the destination reg, then all should be updated.
+---
+name: two_implicit_defs_dst
+tracksRegLiveness: true
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: two_implicit_defs_dst
+    ; CHECK: [[MOV64rm:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    ; CHECK-NEXT: [[MOV64rm1:%[0-9]+]]:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    ; CHECK-NEXT: [[MOV64rm:%[0-9]+]].sub_32bit:gr64_with_sub_8bit = AND32rr [[MOV64rm]].sub_32bit, [[MOV64rm1]].sub_32bit, implicit-def dead $eflags, implicit-def [[MOV64rm]], implicit-def [[MOV64rm]]
+    ; CHECK-NEXT: RET 0, implicit [[MOV64rm]]
+    %0:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    %1:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64) from `ptr null`)
+    %1.sub_32bit:gr64_with_sub_8bit = AND32rr %1.sub_32bit:gr64_with_sub_8bit, %0.sub_32bit:gr64_with_sub_8bit, implicit-def dead $eflags, implicit-def %1:gr64_with_sub_8bit, implicit-def %1:gr64_with_sub_8bit
+    %0:gr64_with_sub_8bit = COPY %1:gr64_with_sub_8bit
+    RET 0, implicit %0
+...