mirror of
https://github.com/RPCS3/llvm-mirror.git
synced 2024-10-19 11:02:59 +02:00
[ARM][GISel] PR35965 Constrain RegClasses of nested instructions built from Dst Pattern
Summary: Apparently, we missed on constraining register classes of VReg-operands of all the instructions built from a destination pattern but the root (top-level) one. The issue exposed itself while selecting G_FPTOSI for armv7: the corresponding pattern generates VTOSIZS wrapped into COPY_TO_REGCLASS, so top-level COPY_TO_REGCLASS gets properly constrained, while nested VTOSIZS (or rather its destination virtual register to be exact) does not. Fixing this by issuing GIR_ConstrainSelectedInstOperands for every nested GIR_BuildMI. https://bugs.llvm.org/show_bug.cgi?id=35965 rdar://problem/36886530 Patch by Roman Tereshin Reviewers: dsanders, qcolombet, rovka, bogner, aditya_nandakumar, volkan Reviewed By: dsanders, qcolombet, rovka Subscribers: aemerson, javed.absar, kristof.beyls, llvm-commits Differential Revision: https://reviews.llvm.org/D42565 llvm-svn: 323692
This commit is contained in:
parent
15ec263224
commit
407184bd30
@ -53,6 +53,24 @@ unsigned llvm::constrainOperandRegClass(
|
|||||||
"PhysReg not implemented");
|
"PhysReg not implemented");
|
||||||
|
|
||||||
const TargetRegisterClass *RegClass = TII.getRegClass(II, OpIdx, &TRI, MF);
|
const TargetRegisterClass *RegClass = TII.getRegClass(II, OpIdx, &TRI, MF);
|
||||||
|
// Some of the target independent instructions, like COPY, may not impose any
|
||||||
|
// register class constraints on some of their operands:
|
||||||
|
if (!RegClass) {
|
||||||
|
assert(!isTargetSpecificOpcode(II.getOpcode()) &&
|
||||||
|
"Only target independent instructions are allowed to have operands "
|
||||||
|
"with no register class constraints");
|
||||||
|
// FIXME: Just bailing out like this here could be not enough, unless we
|
||||||
|
// expect the users of this function to do the right thing for PHIs and
|
||||||
|
// COPY:
|
||||||
|
// v1 = COPY v0
|
||||||
|
// v2 = COPY v1
|
||||||
|
// v1 here may end up not being constrained at all. Please notice that to
|
||||||
|
// reproduce the issue we likely need a destination pattern of a selection
|
||||||
|
// rule producing such extra copies, not just an input GMIR with them as
|
||||||
|
// every existing target using selectImpl handles copies before calling it
|
||||||
|
// and they never reach this function.
|
||||||
|
return Reg;
|
||||||
|
}
|
||||||
return constrainRegToClass(MRI, TII, RBI, InsertPt, Reg, *RegClass);
|
return constrainRegToClass(MRI, TII, RBI, InsertPt, Reg, *RegClass);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -60,6 +78,8 @@ bool llvm::constrainSelectedInstRegOperands(MachineInstr &I,
|
|||||||
const TargetInstrInfo &TII,
|
const TargetInstrInfo &TII,
|
||||||
const TargetRegisterInfo &TRI,
|
const TargetRegisterInfo &TRI,
|
||||||
const RegisterBankInfo &RBI) {
|
const RegisterBankInfo &RBI) {
|
||||||
|
assert(!isPreISelGenericOpcode(I.getOpcode()) &&
|
||||||
|
"A selected instruction is expected");
|
||||||
MachineBasicBlock &MBB = *I.getParent();
|
MachineBasicBlock &MBB = *I.getParent();
|
||||||
MachineFunction &MF = *MBB.getParent();
|
MachineFunction &MF = *MBB.getParent();
|
||||||
MachineRegisterInfo &MRI = MF.getRegInfo();
|
MachineRegisterInfo &MRI = MF.getRegInfo();
|
||||||
|
@ -134,6 +134,9 @@ ARMLegalizerInfo::ARMLegalizerInfo(const ARMSubtarget &ST) {
|
|||||||
setAction({G_PTRTOINT, s32}, Legal);
|
setAction({G_PTRTOINT, s32}, Legal);
|
||||||
setAction({G_PTRTOINT, 1, p0}, Legal);
|
setAction({G_PTRTOINT, 1, p0}, Legal);
|
||||||
|
|
||||||
|
setAction({G_FPTOSI, s32}, Legal);
|
||||||
|
setAction({G_FPTOSI, 1, s32}, Legal);
|
||||||
|
|
||||||
for (unsigned Op : {G_ASHR, G_LSHR, G_SHL})
|
for (unsigned Op : {G_ASHR, G_LSHR, G_SHL})
|
||||||
setAction({Op, s32}, Legal);
|
setAction({Op, s32}, Legal);
|
||||||
|
|
||||||
|
@ -0,0 +1,30 @@
|
|||||||
|
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
|
||||||
|
# RUN: llc -mtriple armv7-gnueabihf -run-pass instruction-select \
|
||||||
|
# RUN: -verify-machineinstrs -o - %s | FileCheck %s
|
||||||
|
---
|
||||||
|
# Test that we constrain register classes of temporary virtual registers
|
||||||
|
# defined by nested instructions built from a Dst Pattern
|
||||||
|
#
|
||||||
|
# G_FPTOSI selects to a (COPY_TO_REGCLASS (VTOSIZS SPR:$a), GPR), where
|
||||||
|
# COPY_TO_REGCLASS doesn't constrain its source register class. It exposes the
|
||||||
|
# bug as we create a tmp reg for VTOSIZS' result and don't constrain its
|
||||||
|
# register class as COPY_TO_REGCLASS' source (which is fine) nor as VTOSIZS'
|
||||||
|
# destination (which is not).
|
||||||
|
#
|
||||||
|
# https://bugs.llvm.org/show_bug.cgi?id=35965
|
||||||
|
name: test_fptosi
|
||||||
|
legalized: true
|
||||||
|
regBankSelected: true
|
||||||
|
body: |
|
||||||
|
bb.1:
|
||||||
|
; CHECK-LABEL: name: test_fptosi
|
||||||
|
; CHECK: [[COPY:%[0-9]+]]:spr = COPY %s0
|
||||||
|
; CHECK: [[VTOSIZS:%[0-9]+]]:spr = VTOSIZS [[COPY]], 14, %noreg
|
||||||
|
; CHECK: [[COPY1:%[0-9]+]]:gpr = COPY [[VTOSIZS]]
|
||||||
|
; CHECK: %r0 = COPY [[COPY1]]
|
||||||
|
; CHECK: MOVPCLR 14, %noreg, implicit %r0
|
||||||
|
%0:fprb(s32) = COPY %s0
|
||||||
|
%1:gprb(s32) = G_FPTOSI %0(s32)
|
||||||
|
%r0 = COPY %1(s32)
|
||||||
|
MOVPCLR 14, %noreg, implicit %r0
|
||||||
|
...
|
@ -308,6 +308,7 @@ def : Pat<(select GPR32:$src1, (complex_rr GPR32:$src2a, GPR32:$src2b),
|
|||||||
// CHECK-NEXT: GIR_ComplexRenderer, /*InsnID*/1, /*RendererID*/1,
|
// CHECK-NEXT: GIR_ComplexRenderer, /*InsnID*/1, /*RendererID*/1,
|
||||||
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/1, /*RendererID*/2, /*SubOperand*/0, // src5a
|
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/1, /*RendererID*/2, /*SubOperand*/0, // src5a
|
||||||
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/1, /*RendererID*/2, /*SubOperand*/1, // src5b
|
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/1, /*RendererID*/2, /*SubOperand*/1, // src5b
|
||||||
|
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
|
||||||
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::INSN3,
|
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::INSN3,
|
||||||
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
|
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
|
||||||
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // src1
|
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // src1
|
||||||
|
@ -610,8 +610,8 @@ public:
|
|||||||
/// Generates code to check that a match rule matches.
|
/// Generates code to check that a match rule matches.
|
||||||
class RuleMatcher : public Matcher {
|
class RuleMatcher : public Matcher {
|
||||||
public:
|
public:
|
||||||
using ActionVec = std::vector<std::unique_ptr<MatchAction>>;
|
using ActionList = std::list<std::unique_ptr<MatchAction>>;
|
||||||
using action_iterator = ActionVec::iterator;
|
using action_iterator = ActionList::iterator;
|
||||||
|
|
||||||
protected:
|
protected:
|
||||||
/// A list of matchers that all need to succeed for the current rule to match.
|
/// A list of matchers that all need to succeed for the current rule to match.
|
||||||
@ -622,7 +622,7 @@ protected:
|
|||||||
|
|
||||||
/// A list of actions that need to be taken when all predicates in this rule
|
/// A list of actions that need to be taken when all predicates in this rule
|
||||||
/// have succeeded.
|
/// have succeeded.
|
||||||
ActionVec Actions;
|
ActionList Actions;
|
||||||
|
|
||||||
using DefinedInsnVariablesMap =
|
using DefinedInsnVariablesMap =
|
||||||
std::map<const InstructionMatcher *, unsigned>;
|
std::map<const InstructionMatcher *, unsigned>;
|
||||||
@ -2125,6 +2125,7 @@ public:
|
|||||||
BuildMIAction(unsigned InsnID, const CodeGenInstruction *I)
|
BuildMIAction(unsigned InsnID, const CodeGenInstruction *I)
|
||||||
: InsnID(InsnID), I(I), Matched(nullptr) {}
|
: InsnID(InsnID), I(I), Matched(nullptr) {}
|
||||||
|
|
||||||
|
unsigned getInsnID() const { return InsnID; }
|
||||||
const CodeGenInstruction *getCGI() const { return I; }
|
const CodeGenInstruction *getCGI() const { return I; }
|
||||||
|
|
||||||
void chooseInsnToMutate(RuleMatcher &Rule) {
|
void chooseInsnToMutate(RuleMatcher &Rule) {
|
||||||
@ -3199,7 +3200,7 @@ Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
|
|||||||
|
|
||||||
Expected<action_iterator>
|
Expected<action_iterator>
|
||||||
GlobalISelEmitter::createAndImportSubInstructionRenderer(
|
GlobalISelEmitter::createAndImportSubInstructionRenderer(
|
||||||
action_iterator InsertPt, RuleMatcher &M, const TreePatternNode *Dst,
|
const action_iterator InsertPt, RuleMatcher &M, const TreePatternNode *Dst,
|
||||||
unsigned TempRegID) {
|
unsigned TempRegID) {
|
||||||
auto InsertPtOrError = createInstructionRenderer(InsertPt, M, Dst);
|
auto InsertPtOrError = createInstructionRenderer(InsertPt, M, Dst);
|
||||||
|
|
||||||
@ -3207,7 +3208,6 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
|
|||||||
|
|
||||||
if (auto Error = InsertPtOrError.takeError())
|
if (auto Error = InsertPtOrError.takeError())
|
||||||
return std::move(Error);
|
return std::move(Error);
|
||||||
InsertPt = InsertPtOrError.get();
|
|
||||||
|
|
||||||
BuildMIAction &DstMIBuilder =
|
BuildMIAction &DstMIBuilder =
|
||||||
*static_cast<BuildMIAction *>(InsertPtOrError.get()->get());
|
*static_cast<BuildMIAction *>(InsertPtOrError.get()->get());
|
||||||
@ -3215,10 +3215,13 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
|
|||||||
// Assign the result to TempReg.
|
// Assign the result to TempReg.
|
||||||
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
|
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
|
||||||
|
|
||||||
InsertPtOrError = importExplicitUseRenderers(InsertPt, M, DstMIBuilder, Dst);
|
InsertPtOrError =
|
||||||
|
importExplicitUseRenderers(InsertPtOrError.get(), M, DstMIBuilder, Dst);
|
||||||
if (auto Error = InsertPtOrError.takeError())
|
if (auto Error = InsertPtOrError.takeError())
|
||||||
return std::move(Error);
|
return std::move(Error);
|
||||||
|
|
||||||
|
M.insertAction<ConstrainOperandsToDefinitionAction>(InsertPt,
|
||||||
|
DstMIBuilder.getInsnID());
|
||||||
return InsertPtOrError.get();
|
return InsertPtOrError.get();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user