Skip to content

[CodeGen][ARM] Test validity of SubRC + SubReg in shouldRewriteCopySrc #159600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[CodeGen][ARM] Test validity of SubRC + SubReg in shouldRewriteCopySrc #159600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/include/llvm/CodeGen/TargetRegisterInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +704 to +706
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, this does feel like the wrong place to handle this. This should either be sunk into findCommonRegClass, or pulled out into the caller. Did you find the place that originated the SrcRC + SubReg combination? Did it happen to be from the UncoalescableRewriter?


// If this source does not incur a cross register bank copy, use it.
return findCommonRegClass(DefRC, DefSubReg, SrcRC, SrcSubReg) != nullptr;
}
Expand Down
14 changes: 0 additions & 14 deletions llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
5 changes: 0 additions & 5 deletions llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
};

Expand Down
31 changes: 31 additions & 0 deletions llvm/test/CodeGen/ARM/pr159343.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
...