[llvm] [RegisterCoalescer] Don't commute two-address instructions which only define a subregister (PR #169031)

[via llvm-commits]

https://github.com/KRM7 created https://github.com/llvm/llvm-project/pull/169031

Currently, the register coalescer may try to commute an instruction like:
```
%0.sub_lo32:gpr64 = AND %0.sub_lo32:gpr64(tied-def 0), %1.sub_lo32:gpr64
USE %0:gpr64
```
resulting in:
```
%1.sub_lo32:gpr64 = AND %1.sub_lo32:gpr64(tied-def 0), %0.sub_lo32:gpr64
USE %1:gpr64
```
However, this is not correct if the instruction doesn't define the entire register, as the value of the upper 32-bits
of the register used in `USE` will not be the same.

>From 45c425d3cb807e01e10cee965651edd665581053 Mon Sep 17 00:00:00 2001
From: Krisztian Rugasi <Krisztian.Rugasi at hightec-rt.com>
Date: Thu, 20 Nov 2025 17:27:33 +0100
Subject: [PATCH 1/2] [RegisterCoalescer] Add reg coalescer tests for commuting
 instructions with tied-def of subreg

---
 .../coalesce-commutative-tied-def-subreg.mir  | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir

diff --git a/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir b/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir
new file mode 100644
index 0000000000000..9640778740676
--- /dev/null
+++ b/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir
@@ -0,0 +1,42 @@
+# 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
+
+# The COPY can't be removed by commuting the AND since the instruction only partially defines the register
+---
+name: commute_tied_subreg_def_part
+tracksRegLiveness: true
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: commute_tied_subreg_def_part
+    ; CHECK: %A:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (volatile load (s64))
+    ; CHECK-NEXT: %B:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64))
+    ; CHECK-NEXT: %B.sub_32bit:gr64_with_sub_8bit = AND32rr %B.sub_32bit, %A.sub_32bit, implicit-def dead $eflags
+    ; CHECK-NEXT: MOV64mr $noreg, 1, $noreg, 0, $noreg, %B :: (store (s64))
+    ; CHECK-NEXT: RET 0, implicit %B
+    %A:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (volatile load (s64))
+    %B:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64))
+    %A.sub_32bit:gr64_with_sub_8bit = AND32rr %A.sub_32bit, %B.sub_32bit, implicit-def dead $eflags
+    MOV64mr $noreg, 1, $noreg, 0, $noreg, %A :: (store (s64))
+    %B:gr64_with_sub_8bit = COPY %A
+    RET 0, implicit %B
+...
+
+# The COPY can be removed by commuting the AND since the instruction defines the entire register
+---
+name: commute_tied_subreg_def_full
+tracksRegLiveness: true
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: commute_tied_subreg_def_full
+    ; CHECK: %A:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (volatile load (s64))
+    ; CHECK-NEXT: %B:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64))
+    ; CHECK-NEXT: %B.sub_32bit:gr64_with_sub_8bit = AND32rr %B.sub_32bit, %A.sub_32bit, implicit-def dead $eflags, implicit-def %B
+    ; CHECK-NEXT: MOV64mr $noreg, 1, $noreg, 0, $noreg, %B :: (store (s64))
+    ; CHECK-NEXT: RET 0, implicit %B
+    %A:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (volatile load (s64))
+    %B:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64))
+    %A.sub_32bit:gr64_with_sub_8bit = AND32rr %A.sub_32bit, %B.sub_32bit, implicit-def dead $eflags, implicit-def %A
+    MOV64mr $noreg, 1, $noreg, 0, $noreg, %A :: (store (s64))
+    %B:gr64_with_sub_8bit = COPY %A
+    RET 0, implicit %B
+...

>From bc813dfc0e6c09549f4957cb60026ff766043a24 Mon Sep 17 00:00:00 2001
From: Krisztian Rugasi <Krisztian.Rugasi at hightec-rt.com>
Date: Thu, 20 Nov 2025 17:33:15 +0100
Subject: [PATCH 2/2] [RegisterCoalescer] Don't commute two-address
 instructions which only define a subregister

---
 llvm/lib/CodeGen/RegisterCoalescer.cpp                    | 8 ++++++++
 .../CodeGen/X86/coalesce-commutative-tied-def-subreg.mir  | 5 +++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 25c4375a73ce0..4560afa07d114 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -869,6 +869,14 @@ RegisterCoalescer::removeCopyByCommutingDef(const CoalescerPair &CP,
   if (!DefMI->isRegTiedToUseOperand(DefIdx, &UseOpIdx))
     return {false, false};
 
+  // If DefMI only defines the register partially, we can't replace uses of the
+  // full register with the new destination register after commuting it.
+  if (IntA.reg().isVirtual() &&
+      none_of(DefMI->all_defs(), [&](const MachineOperand &DefMO) {
+        return DefMO.getReg() == IntA.reg() && !DefMO.getSubReg();
+      }))
+    return {false, false};
+
   // FIXME: The code below tries to commute 'UseOpIdx' operand with some other
   // commutable operand which is expressed by 'CommuteAnyOperandIndex'value
   // passed to the method. That _other_ operand is chosen by
diff --git a/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir b/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir
index 9640778740676..59560f318ff3d 100644
--- a/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir
+++ b/llvm/test/CodeGen/X86/coalesce-commutative-tied-def-subreg.mir
@@ -10,8 +10,9 @@ body: |
     ; CHECK-LABEL: name: commute_tied_subreg_def_part
     ; CHECK: %A:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (volatile load (s64))
     ; CHECK-NEXT: %B:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64))
-    ; CHECK-NEXT: %B.sub_32bit:gr64_with_sub_8bit = AND32rr %B.sub_32bit, %A.sub_32bit, implicit-def dead $eflags
-    ; CHECK-NEXT: MOV64mr $noreg, 1, $noreg, 0, $noreg, %B :: (store (s64))
+    ; CHECK-NEXT: %A.sub_32bit:gr64_with_sub_8bit = AND32rr %A.sub_32bit, %B.sub_32bit, implicit-def dead $eflags
+    ; CHECK-NEXT: MOV64mr $noreg, 1, $noreg, 0, $noreg, %A :: (store (s64))
+    ; CHECK-NEXT: %B:gr64_with_sub_8bit = COPY %A
     ; CHECK-NEXT: RET 0, implicit %B
     %A:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (volatile load (s64))
     %B:gr64_with_sub_8bit = MOV64rm $noreg, 1, $noreg, 0, $noreg :: (load (s64))