[AArch64] Remove redundant COPY from loadRegFromStackSlot

[sdesmalen-arm]

This removes a redundant 'COPY' instruction that #81716 probably forgot to remove.

This redundant COPY led to an issue because because code in LiveRangeSplitting expects that the instruction emitted by loadRegFromStackSlot is an instruction that accesses memory, which isn't the case for the COPY instruction.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: Sander de Smalen (sdesmalen-arm)

Changes

This removes a redundant 'COPY' instruction that #81716 probably forgot to remove.

This redundant COPY led to an issue because because code in LiveRangeSplitting expects that the instruction emitted by loadRegFromStackSlot is an instruction that accesses memory, which isn't the case for the COPY instruction.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (-4)
• (modified) llvm/test/CodeGen/AArch64/spillfill-sve.mir (+36-1)

diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 70f69f34a4a7d4..3b38a5f78dee51 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -5559,10 +5559,6 @@ void AArch64InstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
   if (PNRReg.isValid() && !PNRReg.isVirtual())
     MI.addDef(PNRReg, RegState::Implicit);
   MI.addMemOperand(MMO);
-
-  if (PNRReg.isValid() && PNRReg.isVirtual())
-    BuildMI(MBB, MBBI, DebugLoc(), get(TargetOpcode::COPY), PNRReg)
-        .addReg(DestReg);
 }
 
 bool llvm::isNZCVTouchedInInstructionRange(const MachineInstr &DefMI,
diff --git a/llvm/test/CodeGen/AArch64/spillfill-sve.mir b/llvm/test/CodeGen/AArch64/spillfill-sve.mir
index 11cf388e385312..83c9b73c575708 100644
--- a/llvm/test/CodeGen/AArch64/spillfill-sve.mir
+++ b/llvm/test/CodeGen/AArch64/spillfill-sve.mir
@@ -11,6 +11,7 @@
   define aarch64_sve_vector_pcs void @spills_fills_stack_id_ppr2mul2() #0 { entry: unreachable }
   define aarch64_sve_vector_pcs void @spills_fills_stack_id_pnr() #1 { entry: unreachable }
   define aarch64_sve_vector_pcs void @spills_fills_stack_id_virtreg_pnr() #1 { entry: unreachable }
+  define aarch64_sve_vector_pcs void @spills_fills_stack_id_virtreg_ppr_to_pnr() #1 { entry: unreachable }
   define aarch64_sve_vector_pcs void @spills_fills_stack_id_zpr() #0 { entry: unreachable }
   define aarch64_sve_vector_pcs void @spills_fills_stack_id_zpr2() #0 { entry: unreachable }
   define aarch64_sve_vector_pcs void @spills_fills_stack_id_zpr2strided() #0 { entry: unreachable }
@@ -216,7 +217,7 @@ body:             |
     ; EXPAND: STR_PXI killed renamable $pn8, $sp, 7
     ;
     ; EXPAND: renamable $pn8 = LDR_PXI $sp, 7
-    ; EXPAND: $p0 = PEXT_PCI_B killed renamable $pn8, 0
+    ; EXPAND-NEXT: $p0 = PEXT_PCI_B killed renamable $pn8, 0
 
 
     %0:pnr_p8to15 = WHILEGE_CXX_B undef $x0, undef $x0, 0, implicit-def dead $nzcv
@@ -242,6 +243,40 @@ body:             |
     RET_ReallyLR
 ...
 ---
+name: spills_fills_stack_id_virtreg_ppr_to_pnr
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: ppr }
+  - { id: 1, class: pnr_p8to15 }
+stack:
+body:             |
+  bb.0.entry:
+    liveins: $p0
+
+    %0:ppr = COPY $p0
+
+    $pn0 = IMPLICIT_DEF
+    $pn1 = IMPLICIT_DEF
+    $pn2 = IMPLICIT_DEF
+    $pn3 = IMPLICIT_DEF
+    $pn4 = IMPLICIT_DEF
+    $pn5 = IMPLICIT_DEF
+    $pn6 = IMPLICIT_DEF
+    $pn7 = IMPLICIT_DEF
+    $pn8 = IMPLICIT_DEF
+    $pn9 = IMPLICIT_DEF
+    $pn10 = IMPLICIT_DEF
+    $pn11 = IMPLICIT_DEF
+    $pn12 = IMPLICIT_DEF
+    $pn13 = IMPLICIT_DEF
+    $pn14 = IMPLICIT_DEF
+    $pn15 = IMPLICIT_DEF
+
+    %1:pnr_p8to15 = COPY %0
+    $p0 = PEXT_PCI_B %1, 0
+    RET_ReallyLR
+...
+---
 name: spills_fills_stack_id_zpr
 tracksRegLiveness: true
 registers:

[SamTebbs33]

Nice, thanks for finding this.

[aemerson]

Assuming that this is really redundant, LGTM. Since this was found in the wild I'd also like to see the IR test added to make sure the original case is fixed too.

Definitely wanted for LLVM 19 if possible.

[sdesmalen-arm]

Since this was found in the wild I'd also like to see the IR test added to make sure the original case is fixed too.

When I add the IR test to this patch it makes cherry-picking for LLVM 19 harder, because there is then a dependence on #107406. I can push the IR test as a separate NFC patch after merging this PR, are you okay with that?

[aemerson]

Sure that's fine with me.

[sdesmalen-arm]

/cherry-pick 91a3c6f

[llvmbot]

/pull-request #107435