[llvm] 1f5e9a3 - [MCP] Do not try forward non-existent sub-register of a copy

[Sergei Barannikov via llvm-commits]

Author: Sergei Barannikov
Date: 2023-03-30T06:11:00+03:00
New Revision: 1f5e9a3502119f93cfc3a7cf67f43300f759997e

URL: https://github.com/llvm/llvm-project/commit/1f5e9a3502119f93cfc3a7cf67f43300f759997e
DIFF: https://github.com/llvm/llvm-project/commit/1f5e9a3502119f93cfc3a7cf67f43300f759997e.diff

LOG: [MCP] Do not try forward non-existent sub-register of a copy

In this example:
```
$d14 = COPY killed $d18
$s0 = MI $s28
```

$s28 is a sub-register of $d14. However, $d18 does not have
sub-registers and thus cannot be forwarded. Previously, this resulted
in $noreg being substituted in place of the use of $s28, which later
led to an assertion failure.

Fixes https://github.com/llvm/llvm-project/issues/60908, a regression
that was introduced in D141747.

Reviewed By: arsenm

Differential Revision: https://reviews.llvm.org/D146930

Added: 
    llvm/test/CodeGen/ARM/pr60908.mir

Modified: 
    llvm/lib/CodeGen/MachineCopyPropagation.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index 218f293a40855..855c7444b4b7f 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -643,15 +643,19 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
     const MachineOperand &CopySrc = *CopyOperands->Source;
     Register CopySrcReg = CopySrc.getReg();
 
-    // When the use is a subregister of the COPY destination,
-    // record the subreg index.
-    unsigned SubregIdx = 0;
-
-    // This can only occur when we are dealing with physical registers.
+    Register ForwardedReg = CopySrcReg;
+    // MI might use a sub-register of the Copy destination, in which case the
+    // forwarded register is the matching sub-register of the Copy source.
     if (MOUse.getReg() != CopyDstReg) {
-      SubregIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
-      if (!SubregIdx)
+      unsigned SubRegIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
+      assert(SubRegIdx &&
+             "MI source is not a sub-register of Copy destination");
+      ForwardedReg = TRI->getSubReg(CopySrcReg, SubRegIdx);
+      if (!ForwardedReg) {
+        LLVM_DEBUG(dbgs() << "MCP: Copy source does not have sub-register "
+                          << TRI->getSubRegIndexName(SubRegIdx) << '\n');
         continue;
+      }
     }
 
     // Don't forward COPYs of reserved regs unless they are constant.
@@ -681,13 +685,10 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
     }
 
     LLVM_DEBUG(dbgs() << "MCP: Replacing " << printReg(MOUse.getReg(), TRI)
-                      << "\n     with " << printReg(CopySrcReg, TRI)
+                      << "\n     with " << printReg(ForwardedReg, TRI)
                       << "\n     in " << MI << "     from " << *Copy);
 
-    if (SubregIdx)
-      MOUse.setReg(TRI->getSubReg(CopySrcReg, SubregIdx));
-    else
-      MOUse.setReg(CopySrcReg);
+    MOUse.setReg(ForwardedReg);
 
     if (!CopySrc.isRenamable())
       MOUse.setIsRenamable(false);

diff  --git a/llvm/test/CodeGen/ARM/pr60908.mir b/llvm/test/CodeGen/ARM/pr60908.mir
new file mode 100644
index 0000000000000..b2fada24a2b23
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/pr60908.mir
@@ -0,0 +1,45 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+# RUN: llc -mtriple=armv7 -run-pass=machine-cp %s -o - | FileCheck %s
+
+# Positive test: $d15 has sub-register $s30, which should be propagated.
+---
+name:            test_d15
+tracksRegLiveness: true
+liveins:
+  - { reg: '$d15' }
+body:             |
+  bb.0.entry:
+    liveins: $d15
+
+    ; CHECK-LABEL: name: test_d15
+    ; CHECK: liveins: $d15
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $s0 = COPY $s30
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
+    renamable $d14 = COPY killed $d15
+    $s0 = COPY killed renamable $s28
+    BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
+
+...
+
+# Negative test: $d18 does not have sub-registers.
+---
+name:            test_d18
+tracksRegLiveness: true
+liveins:
+  - { reg: '$d18' }
+body:             |
+  bb.0.entry:
+    liveins: $d18
+
+    ; CHECK-LABEL: name: test_d18
+    ; CHECK: liveins: $d18
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: renamable $d14 = COPY killed $d18
+    ; CHECK-NEXT: $s0 = COPY killed renamable $s28
+    ; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
+    renamable $d14 = COPY killed $d18
+    $s0 = COPY killed renamable $s28
+    BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
+
+...