From 178dfc15d72871b5e760580c4eb789617ba80901 Mon Sep 17 00:00:00 2001 From: Sander de Smalen Date: Thu, 18 Sep 2025 16:14:32 +0000 Subject: [PATCH] [CodeGen][ARM] Test validity of SrcReg + Idx in shouldRewriteCopySrc 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 --- .../include/llvm/CodeGen/TargetRegisterInfo.h | 5 +++ llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp | 14 --------- llvm/lib/Target/ARM/ARMBaseRegisterInfo.h | 5 --- llvm/test/CodeGen/ARM/pr159343.mir | 31 +++++++++++++++++++ 4 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 llvm/test/CodeGen/ARM/pr159343.mir 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 +...