[llvm] r315884 - [globalisel][tablegen] Implement unindexed load, non-extending load, and MemVT checks
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 16 21:18:49 PDT 2017
Hi Reid,
Sorry for the slow reply, I was out of the office most of the day. Thanks for fixing this.
> On 16 Oct 2017, at 13:33, Reid Kleckner <rnk at google.com> wrote:
>
> This broke the MSVC 2017 build. It has something to do with the relative order of instantiation and specialization of addPredicate. I tried to fix this in r315930, which was broken, and r315932, which should fix it without breaking the build elsewhere.
>
> On Sun, Oct 15, 2017 at 5:56 PM, Daniel Sanders via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: dsanders
> Date: Sun Oct 15 17:56:30 2017
> New Revision: 315884
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315884&view=rev <http://llvm.org/viewvc/llvm-project?rev=315884&view=rev>
> Log:
> [globalisel][tablegen] Implement unindexed load, non-extending load, and MemVT checks
>
> Summary:
> This includes some context-sensitivity in the MVT to LLT conversion so that
> pointer types are tested correctly.
> FIXME: I'm not happy with the way this is done since everything is a
> special-case. I've yet to find a reasonable way to implement it.
>
> select-load.mir fails because <1 x s64> loads in tablegen get priority over s64
> loads. This is fixed in the next patch and as such they should be committed
> together, I've posted them separately to help with the review.
>
> Depends on D37456
>
> Reviewers: ab, qcolombet, t.p.northover, rovka, aditya_nandakumar
>
> Subscribers: kristof.beyls, javed.absar, llvm-commits, igorb
>
> Differential Revision: https://reviews.llvm.org/D37457 <https://reviews.llvm.org/D37457>
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
> llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
> llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-load.mir
> llvm/trunk/test/TableGen/GlobalISelEmitter.td
> llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h?rev=315884&r1=315883&r2=315884&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h?rev=315884&r1=315883&r2=315884&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h Sun Oct 15 17:56:30 2017
> @@ -117,6 +117,11 @@ enum {
> /// - OpIdx - Operand index
> /// - Expected type
> GIM_CheckType,
> + /// Check the type of a pointer to any address space.
> + /// - InsnID - Instruction ID
> + /// - OpIdx - Operand index
> + /// - SizeInBits - The size of the pointer value in bits.
> + GIM_CheckPointerToAny,
> /// Check the register bank for the specified operand
> /// - InsnID - Instruction ID
> /// - OpIdx - Operand index
>
> Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h?rev=315884&r1=315883&r2=315884&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h?rev=315884&r1=315883&r2=315884&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h Sun Oct 15 17:56:30 2017
> @@ -244,7 +244,21 @@ bool InstructionSelector::executeMatchTa
> }
> break;
> }
> -
> + case GIM_CheckPointerToAny: {
> + int64_t InsnID = MatchTable[CurrentIdx++];
> + int64_t OpIdx = MatchTable[CurrentIdx++];
> + int64_t SizeInBits = MatchTable[CurrentIdx++];
> + DEBUG(dbgs() << CurrentIdx << ": GIM_CheckPointerToAny(MIs[" << InsnID
> + << "]->getOperand(" << OpIdx
> + << "), SizeInBits=" << SizeInBits << ")\n");
> + assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
> + const LLT &Ty = MRI.getType(State.MIs[InsnID]->getOperand(OpIdx).getReg());
> + if (!Ty.isPointer() || Ty.getSizeInBits() != SizeInBits) {
> + if (handleReject() == RejectAndGiveUp)
> + return false;
> + }
> + break;
> + }
> case GIM_CheckRegBankForClass: {
> int64_t InsnID = MatchTable[CurrentIdx++];
> int64_t OpIdx = MatchTable[CurrentIdx++];
>
> Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-load.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-load.mir?rev=315884&r1=315883&r2=315884&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-load.mir?rev=315884&r1=315883&r2=315884&view=diff>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-load.mir (original)
> +++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-load.mir Sun Oct 15 17:56:30 2017
> @@ -1,5 +1,9 @@
> # RUN: llc -mtriple=aarch64-- -run-pass=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s
>
> +# This patch temporarily causes LD1Onev1d to match instead of LDRDui on a
> +# couple functions. A patch to support iPTR will follow that fixes this.
> +# XFAIL: *
> +
> --- |
> target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
>
> @@ -28,6 +32,7 @@
> define void @load_gep_64_s16_fpr(i16* %addr) { ret void }
> define void @load_gep_32_s8_fpr(i8* %addr) { ret void }
>
> + define void @load_v2s32(i64 *%addr) { ret void }
> ...
>
> ---
> @@ -513,3 +518,28 @@ body: |
> %3(s8) = G_LOAD %2 :: (load 1 from %ir.addr)
> %b0 = COPY %3
> ...
> +---
> +# CHECK-LABEL: name: load_v2s32
> +name: load_v2s32
> +legalized: true
> +regBankSelected: true
> +
> +# CHECK: registers:
> +# CHECK-NEXT: - { id: 0, class: gpr64sp, preferred-register: '' }
> +# CHECK-NEXT: - { id: 1, class: fpr64, preferred-register: '' }
> +registers:
> + - { id: 0, class: gpr }
> + - { id: 1, class: fpr }
> +
> +# CHECK: body:
> +# CHECK: %0 = COPY %x0
> +# CHECK: %1 = LD1Onev2s %0
> +# CHECK: %d0 = COPY %1
> +body: |
> + bb.0:
> + liveins: %x0
> +
> + %0(p0) = COPY %x0
> + %1(<2 x s32>) = G_LOAD %0 :: (load 4 from %ir.addr)
> + %d0 = COPY %1(<2 x s32>)
> +...
>
> Modified: llvm/trunk/test/TableGen/GlobalISelEmitter.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=315884&r1=315883&r2=315884&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=315884&r1=315883&r2=315884&view=diff>
> ==============================================================================
> --- llvm/trunk/test/TableGen/GlobalISelEmitter.td (original)
> +++ llvm/trunk/test/TableGen/GlobalISelEmitter.td Sun Oct 15 17:56:30 2017
> @@ -814,9 +814,29 @@ def MOVimm : I<(outs GPR32:$dst), (ins i
> def fpimmz : FPImmLeaf<f32, [{ return Imm->isExactlyValue(0.0); }]>;
> def MOVfpimmz : I<(outs FPR32:$dst), (ins f32imm:$imm), [(set FPR32:$dst, fpimmz:$imm)]>;
>
> -//===- Test a pattern with an MBB operand. --------------------------------===//
> +//===- Test a simple pattern with inferred pointer operands. ---------------===//
>
> // CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 22*/ [[LABEL:[0-9]+]],
> +// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
> +// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_LOAD,
> +// CHECK-NEXT: GIM_CheckNonAtomic, /*MI*/0,
> +// CHECK-NEXT: // MIs[0] dst
> +// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s32,
> +// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
> +// CHECK-NEXT: // MIs[0] src1
> +// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/1, /*SizeInBits*/32,
> +// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/1, /*RC*/MyTarget::GPR32RegClassID,
> +// CHECK-NEXT: // (ld:{ *:[i32] } GPR32:{ *:[i32] }:$src1)<<P:Predicate_unindexedload>><<P:Predicate_load>> => (LOAD:{ *:[i32] } GPR32:{ *:[i32] }:$src1)
> +// CHECK-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/MyTarget::LOAD,
> +// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
> +// CHECK-NEXT: GIR_Done,
> +// CHECK-NEXT: // Label 22: @[[LABEL]]
> +
> +def LOAD : I<(outs GPR32:$dst), (ins GPR32:$src1),
> + [(set GPR32:$dst, (load GPR32:$src1))]>;
> +//===- Test a pattern with an MBB operand. --------------------------------===//
> +
> +// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 23*/ [[LABEL:[0-9]+]],
> // CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/1,
> // CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_BR,
> // CHECK-NEXT: // MIs[0] target
> @@ -825,7 +845,7 @@ def MOVfpimmz : I<(outs FPR32:$dst), (in
> // CHECK-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/MyTarget::BR,
> // CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/0,
> // CHECK-NEXT: GIR_Done,
> -// CHECK-NEXT: // Label 22: @[[LABEL]]
> +// CHECK-NEXT: // Label 23: @[[LABEL]]
>
> def BR : I<(outs), (ins unknown:$target),
> [(br bb:$target)]>;
>
> Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=315884&r1=315883&r2=315884&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=315884&r1=315883&r2=315884&view=diff>
> ==============================================================================
> --- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Sun Oct 15 17:56:30 2017
> @@ -103,6 +103,12 @@ public:
> OS << "GILLT_v" << Ty.getNumElements() << "s" << Ty.getScalarSizeInBits();
> return;
> }
> + if (Ty.isPointer()) {
> + OS << "GILLT_p" << Ty.getAddressSpace();
> + if (Ty.getSizeInBits() > 0)
> + OS << "s" << Ty.getSizeInBits();
> + return;
> + }
> llvm_unreachable("Unhandled LLT");
> }
>
> @@ -116,6 +122,11 @@ public:
> << Ty.getScalarSizeInBits() << ")";
> return;
> }
> + if (Ty.isPointer() && Ty.getSizeInBits() > 0) {
> + OS << "LLT::pointer(" << Ty.getAddressSpace() << ", "
> + << Ty.getSizeInBits() << ")";
> + return;
> + }
> llvm_unreachable("Unhandled LLT");
> }
>
> @@ -152,9 +163,11 @@ class InstructionMatcher;
> /// MVTs that don't map cleanly to an LLT (e.g., iPTR, *any, ...).
> static Optional<LLTCodeGen> MVTToLLT(MVT::SimpleValueType SVT) {
> MVT VT(SVT);
> +
> if (VT.isVector() && VT.getVectorNumElements() != 1)
> return LLTCodeGen(
> LLT::vector(VT.getVectorNumElements(), VT.getScalarSizeInBits()));
> +
> if (VT.isInteger() || VT.isFloatingPoint())
> return LLTCodeGen(LLT::scalar(VT.getSizeInBits()));
> return None;
> @@ -228,6 +241,11 @@ static Error isTrivialOperatorNode(const
> if (Predicate.isImmediatePattern())
> continue;
>
> + if (Predicate.isLoad() && Predicate.isUnindexed())
> + continue;
> +
> + if (Predicate.isNonExtLoad())
> + continue;
> HasUnsupportedPredicate = true;
> Explanation = Separator + "Has a predicate (" + explainPredicates(N) + ")";
> Separator = ", ";
> @@ -661,6 +679,7 @@ public:
> OPM_Int,
> OPM_LiteralInt,
> OPM_LLT,
> + OPM_PointerToAny,
> OPM_RegBank,
> OPM_MBB,
> };
> @@ -748,6 +767,37 @@ public:
>
> std::set<LLTCodeGen> LLTOperandMatcher::KnownTypes;
>
> +/// Generates code to check that an operand is a pointer to any address space.
> +///
> +/// In SelectionDAG, the types did not describe pointers or address spaces. As a
> +/// result, iN is used to describe a pointer of N bits to any address space and
> +/// PatFrag predicates are typically used to constrain the address space. There's
> +/// no reliable means to derive the missing type information from the pattern so
> +/// imported rules must test the components of a pointer separately.
> +///
> +/// SizeInBits must be non-zero and the matched pointer must be that size.
> +/// TODO: Add support for iPTR via SizeInBits==0 and a subtarget query.
> +class PointerToAnyOperandMatcher : public OperandPredicateMatcher {
> +protected:
> + unsigned SizeInBits;
> +
> +public:
> + PointerToAnyOperandMatcher(unsigned SizeInBits)
> + : OperandPredicateMatcher(OPM_PointerToAny), SizeInBits(SizeInBits) {}
> +
> + static bool classof(const OperandPredicateMatcher *P) {
> + return P->getKind() == OPM_PointerToAny;
> + }
> +
> + void emitPredicateOpcodes(MatchTable &Table, RuleMatcher &Rule,
> + unsigned InsnVarID, unsigned OpIdx) const override {
> + Table << MatchTable::Opcode("GIM_CheckPointerToAny") << MatchTable::Comment("MI")
> + << MatchTable::IntValue(InsnVarID) << MatchTable::Comment("Op")
> + << MatchTable::IntValue(OpIdx) << MatchTable::Comment("SizeInBits")
> + << MatchTable::IntValue(SizeInBits) << MatchTable::LineBreak;
> + }
> +};
> +
> /// Generates code to check that an operand is a particular target constant.
> class ComplexPatternOperandMatcher : public OperandPredicateMatcher {
> protected:
> @@ -927,6 +977,22 @@ public:
>
> InstructionMatcher &getInstructionMatcher() const { return Insn; }
>
> + Error addTypeCheckPredicate(const TypeSetByHwMode &VTy,
> + bool OperandIsAPointer) {
> + auto OpTyOrNone = VTy.isMachineValueType()
> + ? MVTToLLT(VTy.getMachineValueType().SimpleTy)
> + : None;
> + if (!OpTyOrNone)
> + return failedImport("unsupported type");
> +
> + if (OperandIsAPointer)
> + addPredicate<PointerToAnyOperandMatcher>(
> + OpTyOrNone->get().getSizeInBits());
> + else
> + addPredicate<LLTOperandMatcher>(*OpTyOrNone);
> + return Error::success();
> + }
> +
> /// Emit MatchTable opcodes to capture instructions into the MIs table.
> void emitCaptureOpcodes(MatchTable &Table, RuleMatcher &Rule,
> unsigned InsnVarID) const {
> @@ -2070,7 +2136,8 @@ private:
> Error importComplexPatternOperandMatcher(OperandMatcher &OM, Record *R,
> unsigned &TempOpIdx) const;
> Error importChildMatcher(RuleMatcher &Rule, InstructionMatcher &InsnMatcher,
> - const TreePatternNode *SrcChild, unsigned OpIdx,
> + const TreePatternNode *SrcChild,
> + bool OperandIsAPointer, unsigned OpIdx,
> unsigned &TempOpIdx) const;
> Expected<BuildMIAction &>
> createAndImportInstructionRenderer(RuleMatcher &M, const TreePatternNode *Dst,
> @@ -2164,17 +2231,12 @@ Expected<InstructionMatcher &> GlobalISe
>
> unsigned OpIdx = 0;
> for (const TypeSetByHwMode &VTy : Src->getExtTypes()) {
> - auto OpTyOrNone = VTy.isMachineValueType()
> - ? MVTToLLT(VTy.getMachineValueType().SimpleTy)
> - : None;
> - if (!OpTyOrNone)
> - return failedImport(
> - "Result of Src pattern operator has an unsupported type");
> -
> // Results don't have a name unless they are the root node. The caller will
> // set the name if appropriate.
> OperandMatcher &OM = InsnMatcher.addOperand(OpIdx++, "", TempOpIdx);
> - OM.addPredicate<LLTOperandMatcher>(*OpTyOrNone);
> + if (auto Error = OM.addTypeCheckPredicate(VTy, false /* OperandIsAPointer */))
> + return failedImport(toString(std::move(Error)) +
> + " for result of Src pattern operator");
> }
>
> for (const auto &Predicate : Src->getPredicateFns()) {
> @@ -2186,6 +2248,25 @@ Expected<InstructionMatcher &> GlobalISe
> continue;
> }
>
> + // No check required. A G_LOAD is an unindexed load.
> + if (Predicate.isLoad() && Predicate.isUnindexed())
> + continue;
> +
> + // No check required. G_LOAD by itself is a non-extending load.
> + if (Predicate.isNonExtLoad())
> + continue;
> +
> + if (Predicate.isLoad() && Predicate.getMemoryVT() != nullptr) {
> + Optional<LLTCodeGen> MemTyOrNone =
> + MVTToLLT(getValueType(Predicate.getMemoryVT()));
> +
> + if (!MemTyOrNone)
> + return failedImport("MemVT could not be converted to LLT");
> +
> + InsnMatcher.getOperand(0).addPredicate<LLTOperandMatcher>(MemTyOrNone.getValue());
> + continue;
> + }
> +
> return failedImport("Src pattern child has predicate (" +
> explainPredicates(Src) + ")");
> }
> @@ -2217,6 +2298,13 @@ Expected<InstructionMatcher &> GlobalISe
> for (unsigned i = 0, e = Src->getNumChildren(); i != e; ++i) {
> TreePatternNode *SrcChild = Src->getChild(i);
>
> + // SelectionDAG allows pointers to be represented with iN since it doesn't
> + // distinguish between pointers and integers but they are different types in GlobalISel.
> + // Coerce integers to pointers to address space 0 if the context indicates a pointer.
> + // TODO: Find a better way to do this, SDTCisPtrTy?
> + bool OperandIsAPointer =
> + SrcGIOrNull->TheDef->getName() == "G_LOAD" && i == 0;
> +
> // For G_INTRINSIC/G_INTRINSIC_W_SIDE_EFFECTS, the operand immediately
> // following the defs is an intrinsic ID.
> if ((SrcGIOrNull->TheDef->getName() == "G_INTRINSIC" ||
> @@ -2232,8 +2320,9 @@ Expected<InstructionMatcher &> GlobalISe
> return failedImport("Expected IntInit containing instrinsic ID)");
> }
>
> - if (auto Error = importChildMatcher(Rule, InsnMatcher, SrcChild, OpIdx++,
> - TempOpIdx))
> + if (auto Error =
> + importChildMatcher(Rule, InsnMatcher, SrcChild, OperandIsAPointer,
> + OpIdx++, TempOpIdx))
> return std::move(Error);
> }
> }
> @@ -2256,6 +2345,7 @@ Error GlobalISelEmitter::importComplexPa
> Error GlobalISelEmitter::importChildMatcher(RuleMatcher &Rule,
> InstructionMatcher &InsnMatcher,
> const TreePatternNode *SrcChild,
> + bool OperandIsAPointer,
> unsigned OpIdx,
> unsigned &TempOpIdx) const {
> OperandMatcher &OM =
> @@ -2278,12 +2368,10 @@ Error GlobalISelEmitter::importChildMatc
> }
> }
>
> - Optional<LLTCodeGen> OpTyOrNone = None;
> - if (ChildTypes.front().isMachineValueType())
> - OpTyOrNone = MVTToLLT(ChildTypes.front().getMachineValueType().SimpleTy);
> - if (!OpTyOrNone)
> - return failedImport("Src operand has an unsupported type (" + to_string(*SrcChild) + ")");
> - OM.addPredicate<LLTOperandMatcher>(*OpTyOrNone);
> + if (auto Error =
> + OM.addTypeCheckPredicate(ChildTypes.front(), OperandIsAPointer))
> + return failedImport(toString(std::move(Error)) + " for Src operand (" +
> + to_string(*SrcChild) + ")");
>
> // Check for nested instructions.
> if (!SrcChild->isLeaf()) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171016/82bd1a51/attachment.html>
More information about the llvm-commits
mailing list