[AArch64][SVE] Support copy of PPR2 register class in copyPhysReg by kmclaughlin-arm · Pull Request #216303 · llvm/llvm-project

kmclaughlin-arm · 2026-08-14T11:07:00Z

This fixes the following crash, which was observed after landing #209484:
https://clang.godbolt.org/z/YG1r63oT3

This fixes the following crash, which was observed after landing llvm#209484: llvm#209484

llvmorg-github-actions · 2026-08-14T11:07:37Z

@llvm/pr-subscribers-backend-aarch64

Author: Kerry McLaughlin (kmclaughlin-arm)

Changes

This fixes the following crash, which was observed after landing #209484:
https://clang.godbolt.org/z/YG1r63oT3

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

2 Files Affected:

(modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+12)
(added) llvm/test/CodeGen/AArch64/sve-copy-pprpair.mir (+24)

diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 9e128651598f3..1502724ffa713 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -5771,6 +5771,8 @@ void AArch64InstrInfo::copyPhysRegTuple(MachineBasicBlock &MBB,
     const MachineInstrBuilder MIB = BuildMI(MBB, I, DL, get(Opcode));
     AddSubReg(MIB, DestReg, Indices[SubReg], RegState::Define, TRI);
     AddSubReg(MIB, SrcReg, Indices[SubReg], {}, TRI);
+    if (Opcode == AArch64::ORR_PPzPP)
+      AddSubReg(MIB, SrcReg, Indices[SubReg], {}, TRI);
     AddSubReg(MIB, SrcReg, Indices[SubReg], getKillRegState(KillSrc), TRI);
   }
 }
@@ -5991,6 +5993,16 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
     return;
   }
 
+  if (AArch64::PPR2RegClass.contains(DestReg) &&
+      AArch64::PPR2RegClass.contains(SrcReg)) {
+    assert(Subtarget.isSVEorStreamingSVEAvailable() &&
+           "Unexpected SVE predicate register.");
+    static const unsigned Indices[] = {AArch64::psub0, AArch64::psub1};
+    copyPhysRegTuple(MBB, I, DL, DestReg, SrcReg, KillSrc, AArch64::ORR_PPzPP,
+                     Indices);
+    return;
+  }
+
   // Copy a Z register by ORRing with itself.
   if (AArch64::ZPRRegClass.contains(DestReg) &&
       AArch64::ZPRRegClass.contains(SrcReg)) {
diff --git a/llvm/test/CodeGen/AArch64/sve-copy-pprpair.mir b/llvm/test/CodeGen/AArch64/sve-copy-pprpair.mir
new file mode 100644
index 0000000000000..336886f699b49
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/sve-copy-pprpair.mir
@@ -0,0 +1,24 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
+# RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve -run-pass=postrapseudos -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name:            copy_ppr2
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$p0_p1' }
+frameInfo:
+  maxCallFrameSize: 0
+body:             |
+  bb.0:
+    liveins: $p0_p1
+    ; CHECK-LABEL: name: copy_ppr2
+    ; CHECK: liveins: $p0_p1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $p2 = ORR_PPzPP $p0, $p0, killed $p0
+    ; CHECK-NEXT: $p3 = ORR_PPzPP $p1, $p1, killed $p1
+    ; CHECK-NEXT: RET_ReallyLR
+    $p2_p3 = COPY killed renamable $p0_p1
+    RET_ReallyLR
+
+...

david-arm · 2026-08-14T12:10:39Z

    const MachineInstrBuilder MIB = BuildMI(MBB, I, DL, get(Opcode));
    AddSubReg(MIB, DestReg, Indices[SubReg], RegState::Define, TRI);
    AddSubReg(MIB, SrcReg, Indices[SubReg], {}, TRI);
+    if (Opcode == AArch64::ORR_PPzPP)


Why do we need to check the opcode here? I was just looking at the ZPR2RegClass case, also handled in copyPhysReg, and we don't seem to need opcode checking in that case. I assume there is a problem with getKillRegState(KillSrc)?

Oh, we still call AddSubReg(MIB, SrcReg, Indices[SubReg], getKillRegState(KillSrc), TRI); anyway even if the opcode is AArch64::ORR_PPzPP. So looks like we call it twice?

The ORR_PPzPP instruction takes an extra operand because it has an additional governing predicate operand, which the ORR_ZZZ instruction used by ZPR classes do not (this now matches how we handle copies of PPRRegClass above).

Another alternative is to just call copyPhysReg directly at this point, as that function already correctly handles the scalar copies. (and that also simplifies the calls to copyPhysRegTuple)

Thanks @sdesmalen-arm , that does make things a bit simpler here.

Calling copyPhysReg directly has caused one of the existing tests to change (arm64-copy-tuple.ll). This is because for some tuples (e.g. DDD) we are no longer passing in a specific opcode and copyPhysReg can choose to use something else (e.g. FMOV) instead for the subregister class, such as FPR64RegClass. I think the changes are still correct, so I've included this in the latest commit.

sdesmalen-arm · 2026-08-14T12:17:47Z

+    ; CHECK-NEXT: $p2 = ORR_PPzPP $p0, $p0, killed $p0
+    ; CHECK-NEXT: $p3 = ORR_PPzPP $p1, $p1, killed $p1
+    ; CHECK-NEXT: RET_ReallyLR
+    $p2_p3 = COPY killed renamable $p0_p1


nit: maybe also add a test where there is some overlap, e.g. $p1_p2 = COPY killed $p0_p1
(and the test for the case I mentioned above, $p0_p1 = COPY killed $p15_p0)

sdesmalen-arm · 2026-08-14T12:19:23Z

+    assert(Subtarget.isSVEorStreamingSVEAvailable() &&
+           "Unexpected SVE predicate register.");
+    static const unsigned Indices[] = {AArch64::psub0, AArch64::psub1};
+    copyPhysRegTuple(MBB, I, DL, DestReg, SrcReg, KillSrc, AArch64::ORR_PPzPP,


forwardCopyWillClobberTuple considers 32 the maximum number of registers, but for predicates it is 16, so this goes wrong when copying $p15_p0 to $p0_p1, for example.

I've changed forwardCopyWillClobberTuple to consider 16 the max for predicates.

- Changed forwardCopyWillClobberTuple to consider number of predicate registers - Added tests with overlap & test for forwardCopyWillClobberTuple

sdesmalen-arm · 2026-08-17T07:22:14Z

  // easily obtainable with a mask.
-  return ((DestReg - SrcReg) & 0x1f) < NumRegs;
+  unsigned MaxRegs = IsPred ? 0xf : 0x1f;


Can this be generalised to take the max regs from the number of registers in the register class, rather than hardcoding it? (e.g. using the largest register class for src/dst)

sdesmalen-arm · 2026-08-17T07:25:53Z

                                        ArrayRef<unsigned> Indices) const {
  assert(Subtarget.hasNEON() && "Unexpected register copy without NEON");
  const TargetRegisterInfo *TRI = &getRegisterInfo();
  uint16_t DestEncoding = TRI->getEncodingValue(DestReg);
  uint16_t SrcEncoding = TRI->getEncodingValue(SrcReg);
  unsigned NumRegs = Indices.size();
+  bool IsPred = AArch64::PPR2RegClass.contains(DestReg);


It's probably better to check that the register class of the sub-register is a PPR, otherwise, the current mechanism doesn't support PPR2Mul2 or any other tuple register (classes) we may want to add in the future.

sdesmalen-arm · 2026-08-18T10:22:56Z

-    AddSubReg(MIB, SrcReg, Indices[SubReg], getKillRegState(KillSrc), TRI);
+    MCRegister DestSubReg = TRI->getSubReg(DestReg, Indices[SubReg]);
+    MCRegister SrcSubReg = TRI->getSubReg(SrcReg, Indices[SubReg]);
+    copyPhysReg(MBB, I, DL, DestSubReg, SrcSubReg, KillSrc);


I forgot to mention, copyPhysReg increments ++NumCopyInstrs for each COPY, which would be different now. You could consider moving most of the code to a copyPhysRegImpl that doesn't do the increment.

[AArch64][SVE] Support copy of PPR2 register class in copyPhysReg.

851e31f

This fixes the following crash, which was observed after landing llvm#209484: llvm#209484

kmclaughlin-arm requested review from david-arm, fhahn and sdesmalen-arm August 14, 2026 11:07

llvmorg-github-actions Bot added the backend:AArch64 label Aug 14, 2026

david-arm reviewed Aug 14, 2026

View reviewed changes

sdesmalen-arm reviewed Aug 14, 2026

View reviewed changes

- Call copyPhysReg directly from copyPhysRegTuple

c841375

- Changed forwardCopyWillClobberTuple to consider number of predicate registers - Added tests with overlap & test for forwardCopyWillClobberTuple

kmclaughlin-arm mentioned this pull request Aug 14, 2026

[LV] Use wide lane masks as the canonical form when tail-folding & interleaving #209484

Merged

sdesmalen-arm reviewed Aug 17, 2026

View reviewed changes

sdesmalen-arm reviewed Aug 18, 2026

View reviewed changes

Navigation Menu

Uh oh!

[AArch64][SVE] Support copy of PPR2 register class in copyPhysReg - #216303

[AArch64][SVE] Support copy of PPR2 register class in copyPhysReg#216303
kmclaughlin-arm wants to merge 2 commits into
llvm:mainfrom
kmclaughlin-arm:copy-ppr2

kmclaughlin-arm commented Aug 14, 2026

Uh oh!

llvmorg-github-actions Bot commented Aug 14, 2026

Uh oh!

david-arm Aug 14, 2026

Uh oh!

david-arm Aug 14, 2026

Uh oh!

kmclaughlin-arm Aug 14, 2026

Uh oh!

sdesmalen-arm Aug 14, 2026

Uh oh!

kmclaughlin-arm Aug 14, 2026

Uh oh!

Uh oh!

sdesmalen-arm Aug 14, 2026

Uh oh!

sdesmalen-arm Aug 14, 2026

Uh oh!

kmclaughlin-arm Aug 14, 2026

Uh oh!

sdesmalen-arm Aug 17, 2026

Uh oh!

sdesmalen-arm Aug 17, 2026

Uh oh!

sdesmalen-arm Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

kmclaughlin-arm commented Aug 14, 2026

Uh oh!

llvmorg-github-actions Bot commented Aug 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants