[llvm] [CodeGen][ARM] Test validity of SubRC + SubReg in shouldRewriteCopySrc (PR #159600)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-arm

Author: Sander de Smalen (sdesmalen-arm)

<details>
<summary>Changes</summary>

For the ARM target, registers S0..S31 overlap with D0..D15. Subreg indices `ssub_0` and `ssub_1` are only valid for a register class that covers D0..D15, but the register class passed to `shouldRewriteCopySrc` is DPRRegClass which covers D0..D15 as well as D16..D31, meaning that `ssub_0` and `ssub_1` can't be used.

By fixing this in `TargetRegisterInfo::shouldRewriteCopySrc`, there's no longer a need to override shouldRewriteCopySrc for ARM.

This fixes #<!-- -->159343

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


4 Files Affected:

- (modified) llvm/include/llvm/CodeGen/TargetRegisterInfo.h (+5) 
- (modified) llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp (-14) 
- (modified) llvm/lib/Target/ARM/ARMBaseRegisterInfo.h (-5) 
- (added) llvm/test/CodeGen/ARM/pr159343.mir (+31) 


``````````diff
diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index bf133f0332bcb..8d126af67f2a7 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -700,6 +700,11 @@ class LLVM_ABI TargetRegisterInfo : public MCRegisterInfo {
                                     unsigned DefSubReg,
                                     const TargetRegisterClass *SrcRC,
                                     unsigned SrcSubReg) const {
+    // Validate that SrcRC and SrcSubReg is actually a valid combination.
+    const TargetRegisterClass *SubRC = getSubClassWithSubReg(SrcRC, SrcSubReg);
+    if (!SubRC || SubRC != SrcRC)
+      return false;
+
     // If this source does not incur a cross register bank copy, use it.
     return findCommonRegClass(DefRC, DefSubReg, SrcRC, SrcSubReg) != nullptr;
   }
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index e94220af05a0d..2e8a676269a74 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -960,17 +960,3 @@ bool ARMBaseRegisterInfo::shouldCoalesce(MachineInstr *MI,
   }
   return false;
 }
-
-bool ARMBaseRegisterInfo::shouldRewriteCopySrc(const TargetRegisterClass *DefRC,
-                                               unsigned DefSubReg,
-                                               const TargetRegisterClass *SrcRC,
-                                               unsigned SrcSubReg) const {
-  // We can't extract an SPR from an arbitary DPR (as opposed to a DPR_VFP2).
-  if (DefRC == &ARM::SPRRegClass && DefSubReg == 0 &&
-      SrcRC == &ARM::DPRRegClass &&
-      (SrcSubReg == ARM::ssub_0 || SrcSubReg == ARM::ssub_1))
-    return false;
-
-  return TargetRegisterInfo::shouldRewriteCopySrc(DefRC, DefSubReg,
-                                                  SrcRC, SrcSubReg);
-}
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
index 5b67b34089d7e..03b0fa0d1ee08 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
@@ -158,11 +158,6 @@ class ARMBaseRegisterInfo : public ARMGenRegisterInfo {
                       const TargetRegisterClass *NewRC,
                       LiveIntervals &LIS) const override;
 
-  bool shouldRewriteCopySrc(const TargetRegisterClass *DefRC,
-                            unsigned DefSubReg,
-                            const TargetRegisterClass *SrcRC,
-                            unsigned SrcSubReg) const override;
-
   int getSEHRegNum(unsigned i) const { return getEncodingValue(i); }
 };
 
diff --git a/llvm/test/CodeGen/ARM/pr159343.mir b/llvm/test/CodeGen/ARM/pr159343.mir
new file mode 100644
index 0000000000000..f9ad8af2cfff6
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/pr159343.mir
@@ -0,0 +1,31 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
+# RUN: llc -run-pass=peephole-opt -verify-machineinstrs -mtriple=thumbv7-unknown-linux-android29 %s -o - | FileCheck %s
+---
+name: Test_shouldRewriteCopySrc_Invalid_SubReg
+tracksRegLiveness: true
+body:             |
+  bb.1:
+    liveins: $r0, $r1
+
+    ; CHECK-LABEL: name: Test_shouldRewriteCopySrc_Invalid_SubReg
+    ; CHECK: liveins: $r0, $r1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[DEF:%[0-9]+]]:dpair = IMPLICIT_DEF
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:dpr = COPY [[DEF]].dsub_0
+    ; CHECK-NEXT: [[VMOVRRD:%[0-9]+]]:gpr, [[VMOVRRD1:%[0-9]+]]:gpr = VMOVRRD killed [[COPY]], 14 /* CC::al */, $noreg
+    ; CHECK-NEXT: [[VMOVSR:%[0-9]+]]:spr = VMOVSR killed [[VMOVRRD1]], 14 /* CC::al */, $noreg
+    ; CHECK-NEXT: [[DEF1:%[0-9]+]]:spr = IMPLICIT_DEF
+    ; CHECK-NEXT: [[DEF2:%[0-9]+]]:spr = IMPLICIT_DEF
+    ; CHECK-NEXT: [[DEF3:%[0-9]+]]:spr = IMPLICIT_DEF
+    ; CHECK-NEXT: [[REG_SEQUENCE:%[0-9]+]]:mqpr = REG_SEQUENCE killed [[DEF2]], %subreg.ssub_0, killed [[DEF1]], %subreg.ssub_1, killed [[DEF3]], %subreg.ssub_2, killed [[VMOVSR]], %subreg.ssub_3
+    ; CHECK-NEXT: VST1q64 $r1, 0, killed [[REG_SEQUENCE]], 14 /* CC::al */, $noreg
+    %0:dpair = IMPLICIT_DEF
+    %1:dpr = COPY %0.dsub_0
+    %2:gpr, %3:gpr = VMOVRRD killed %1, 14 /* CC::al */, $noreg
+    %4:spr = VMOVSR killed %3, 14 /* CC::al */, $noreg
+    %5:spr = IMPLICIT_DEF
+    %6:spr = IMPLICIT_DEF
+    %7:spr = IMPLICIT_DEF
+    %8:mqpr = REG_SEQUENCE killed %6, %subreg.ssub_0, killed %5, %subreg.ssub_1, killed %7, %subreg.ssub_2, killed %4, %subreg.ssub_3
+    VST1q64 $r1, 0, killed %8, 14 /* CC::al */, $noreg
+...

``````````

</details>


https://github.com/llvm/llvm-project/pull/159600