[llvm] r323692 - [ARM][GISel] PR35965 Constrain RegClasses of nested instructions built from Dst Pattern
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 29 13:09:13 PST 2018
Author: dsanders
Date: Mon Jan 29 13:09:12 2018
New Revision: 323692
URL: http://llvm.org/viewvc/llvm-project?rev=323692&view=rev
Log:
[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
Added:
llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-select-copy_to_regclass-of-fptosi.mir
Modified:
llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp
llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp
llvm/trunk/test/TableGen/GlobalISelEmitter.td
llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
Modified: llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp?rev=323692&r1=323691&r2=323692&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/Utils.cpp Mon Jan 29 13:09:12 2018
@@ -53,6 +53,24 @@ unsigned llvm::constrainOperandRegClass(
"PhysReg not implemented");
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);
}
@@ -60,6 +78,8 @@ bool llvm::constrainSelectedInstRegOpera
const TargetInstrInfo &TII,
const TargetRegisterInfo &TRI,
const RegisterBankInfo &RBI) {
+ assert(!isPreISelGenericOpcode(I.getOpcode()) &&
+ "A selected instruction is expected");
MachineBasicBlock &MBB = *I.getParent();
MachineFunction &MF = *MBB.getParent();
MachineRegisterInfo &MRI = MF.getRegInfo();
Modified: llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp?rev=323692&r1=323691&r2=323692&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp Mon Jan 29 13:09:12 2018
@@ -134,6 +134,9 @@ ARMLegalizerInfo::ARMLegalizerInfo(const
setAction({G_PTRTOINT, s32}, 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})
setAction({Op, s32}, Legal);
Added: llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-select-copy_to_regclass-of-fptosi.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-select-copy_to_regclass-of-fptosi.mir?rev=323692&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-select-copy_to_regclass-of-fptosi.mir (added)
+++ llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-select-copy_to_regclass-of-fptosi.mir Mon Jan 29 13:09:12 2018
@@ -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
+...
Modified: llvm/trunk/test/TableGen/GlobalISelEmitter.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=323692&r1=323691&r2=323692&view=diff
==============================================================================
--- llvm/trunk/test/TableGen/GlobalISelEmitter.td (original)
+++ llvm/trunk/test/TableGen/GlobalISelEmitter.td Mon Jan 29 13:09:12 2018
@@ -308,6 +308,7 @@ def : Pat<(select GPR32:$src1, (complex_
// 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*/1, // src5b
+// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
// 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*/1, // src1
Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=323692&r1=323691&r2=323692&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Mon Jan 29 13:09:12 2018
@@ -610,8 +610,8 @@ public:
/// Generates code to check that a match rule matches.
class RuleMatcher : public Matcher {
public:
- using ActionVec = std::vector<std::unique_ptr<MatchAction>>;
- using action_iterator = ActionVec::iterator;
+ using ActionList = std::list<std::unique_ptr<MatchAction>>;
+ using action_iterator = ActionList::iterator;
protected:
/// 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
/// have succeeded.
- ActionVec Actions;
+ ActionList Actions;
using DefinedInsnVariablesMap =
std::map<const InstructionMatcher *, unsigned>;
@@ -2125,6 +2125,7 @@ public:
BuildMIAction(unsigned InsnID, const CodeGenInstruction *I)
: InsnID(InsnID), I(I), Matched(nullptr) {}
+ unsigned getInsnID() const { return InsnID; }
const CodeGenInstruction *getCGI() const { return I; }
void chooseInsnToMutate(RuleMatcher &Rule) {
@@ -3199,7 +3200,7 @@ Expected<BuildMIAction &> GlobalISelEmit
Expected<action_iterator>
GlobalISelEmitter::createAndImportSubInstructionRenderer(
- action_iterator InsertPt, RuleMatcher &M, const TreePatternNode *Dst,
+ const action_iterator InsertPt, RuleMatcher &M, const TreePatternNode *Dst,
unsigned TempRegID) {
auto InsertPtOrError = createInstructionRenderer(InsertPt, M, Dst);
@@ -3207,7 +3208,6 @@ GlobalISelEmitter::createAndImportSubIns
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);
- InsertPt = InsertPtOrError.get();
BuildMIAction &DstMIBuilder =
*static_cast<BuildMIAction *>(InsertPtOrError.get()->get());
@@ -3215,10 +3215,13 @@ GlobalISelEmitter::createAndImportSubIns
// Assign the result to TempReg.
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
- InsertPtOrError = importExplicitUseRenderers(InsertPt, M, DstMIBuilder, Dst);
+ InsertPtOrError =
+ importExplicitUseRenderers(InsertPtOrError.get(), M, DstMIBuilder, Dst);
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);
+ M.insertAction<ConstrainOperandsToDefinitionAction>(InsertPt,
+ DstMIBuilder.getInsnID());
return InsertPtOrError.get();
}
More information about the llvm-commits
mailing list