[MCP] Disable BackwardCopyPropagateBlock for copies with implicit registers.

[topperc]

If there's an implicit-def of a super register, the propagation
must preserve this implicit-def. Knowing how and when to do this
may require target specific knowledge so just disable it for now.

Prior to 2def1c4, we checked that the copy had explicit 2 operands
when that was removed we started allowing implicit operands through.
This patch adds a check for implicit operands, but still allows
extra explicit operands which was the goal of 2def1c4.

Fixes #131478.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-backend-systemz

Author: Craig Topper (topperc)

Changes

If there's an implicit-def of a super register, the propagation
must preserve this implicit-def. Knowing how and when to do this
may require target specific knowledge so just disable it for now.

Prior to e17f07c, we checked that the copy had explicit 2 operands
when that was removed we started allowing implicit operands through.
This patch adds a check for implicit operands, but still allows
extra explicit operands which was the goal of e17f07c.

Fixes #137667.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/MachineCopyPropagation.cpp (+1-1)
• (added) llvm/test/CodeGen/SystemZ/pr137667.mir (+21)

diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index ff75b87b23128..9e82b5caed0d0 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -1200,7 +1200,7 @@ void MachineCopyPropagation::BackwardCopyPropagateBlock(
     // Ignore non-trivial COPYs.
     std::optional<DestSourcePair> CopyOperands =
         isCopyInstr(MI, *TII, UseCopyInstr);
-    if (CopyOperands) {
+    if (CopyOperands && MI.getNumImplicitOperands() == 0) {
       Register DefReg = CopyOperands->Destination->getReg();
       Register SrcReg = CopyOperands->Source->getReg();
 
diff --git a/llvm/test/CodeGen/SystemZ/pr137667.mir b/llvm/test/CodeGen/SystemZ/pr137667.mir
new file mode 100644
index 0000000000000..66038f9dce22f
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/pr137667.mir
@@ -0,0 +1,21 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc %s -mtriple=s390x-unknown-linux-gnu -run-pass=machine-cp -verify-machineinstrs -o - | FileCheck %s
+
+---
+name:            t
+tracksRegLiveness: true
+noPhis:          true
+isSSA:           false
+noVRegs:         true
+hasFakeUses:     false
+tracksDebugUserValues: true
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: t
+    ; CHECK: renamable $r14d = LLILL 0
+    ; CHECK-NEXT: renamable $r12d = COPY killed renamable $r14d, implicit-def $r12q
+    ; CHECK-NEXT: Return implicit $r12q
+    renamable $r14d = LLILL 0
+    renamable $r12d = COPY killed renamable $r14d, implicit-def $r12q
+    Return implicit $r12q
+...

[s-barannikov]

The links are incorrect?

[topperc]

The links are incorrect?

Oops. Looks like I mixed up tabs in the browser while working on this. Right links are added now and the test is renamed.

[arsenm]

I thought isCopyInstr was already filtering out these hard cases

[arsenm]

I guess it does in the target instruction implementations, not the base copy handling

[arsenm]

Might as well just not use isCopyInstr, you're probably filtering out cases the target code decided are OK

[topperc]

For RISC-V, we want to support the canonical copy instruction that appears after ExpandPostRAPseudos, "ADDI rd, rs, 0". What are you suggesting we use here instead of isCopyInstr?

[preames]

LGTM