[llvm] r365114 - [TableGen] Allow DAG isel patterns to override default operands.
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 5 08:39:01 PDT 2019
Hi,
Thanks for working on this problem. I ran into this recently, but after your patch I’m still having some issues with OperandWithDefaultOps. It seems the type of the defaulted operand isn’t being properly used. If I try to convert this case to using it:
diff --git a/lib/Target/AMDGPU/SIInstrInfo.td b/lib/Target/AMDGPU/SIInstrInfo.td
index b53b04abefc..1fd9046a2be 100644
--- a/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/lib/Target/AMDGPU/SIInstrInfo.td
@@ -839,6 +839,12 @@ class NamedOperandBit<string Name, AsmOperandClass MatchClass> : Operand<i1> {
let ParserMatchClass = MatchClass;
}
+class NamedOperandBitDefault0<string Name, AsmOperandClass MatchClass> :
+ OperandWithDefaultOps<i1, (ops (i1 0))> {
+ let PrintMethod = "print"#Name;
+ let ParserMatchClass = MatchClass;
+}
+
class NamedOperandU8<string Name, AsmOperandClass MatchClass> : Operand<i8> {
let PrintMethod = "print"#Name;
let ParserMatchClass = MatchClass;
@@ -885,7 +891,7 @@ def offset1 : NamedOperandU8<"Offset1", NamedMatchClass<"Offset1">>;
def gds : NamedOperandBit<"GDS", NamedMatchClass<"GDS">>;
def omod : NamedOperandU32<"OModSI", NamedMatchClass<"OModSI">>;
-def clampmod : NamedOperandBit<"ClampSI", NamedMatchClass<"ClampSI">>;
+def clampmod : NamedOperandBitDefault0<"ClampSI", NamedMatchClass<"ClampSI">>;
def highmod : NamedOperandBit<"High", NamedMatchClass<"High">>;
def DLC : NamedOperandBit<"DLC", NamedMatchClass<"DLC">>;
I hit this assert:
Type set is empty for each HW mode:
possible type contradiction in the pattern below (use -print-records with llvm-tblgen to see all expanded records).
anonymous_6086(src0_modifiers, src0, src1_modifiers, src1, src2_modifiers, src2): (V_MAD_MIXLO_F16:{ *:[i16 i32 f16 f32 v2i16 v2f16] } ?:{ *:[i32] }:$src0_modifiers, ?:{ *:[i16 i32 f16 f32 v2i16 v2f16] }:$src0, ?:{ *:[i32] }:$src1_modifiers, ?:{ *:[i16 i32 f16 f32 v2i16 v2f16] }:$src1, ?:{ *:[i32] }:$src2_modifiers, ?:{ *:[i16 i32 f16 f32 v2i16 v2f16] }:$src2, 0:{ *:[] }, (IMPLICIT_DEF:{ *:[i32] }))
UNREACHABLE executed at ../utils/TableGen/CodeGenDAGPatterns.cpp:821!
0 llvm-tblgen 0x000000010564b6cc llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 60
1 llvm-tblgen 0x000000010564bc59 PrintStackTraceSignalHandler(void*) + 25
2 llvm-tblgen 0x0000000105649776 llvm::sys::RunSignalHandlers() + 118
3 llvm-tblgen 0x000000010564f8a2 SignalHandler(int) + 210
4 libsystem_platform.dylib 0x00007fff688bdb5d _sigtramp + 29
5 libsystem_platform.dylib 000000000000000000 _sigtramp + 18446603338762167488
6 libsystem_c.dylib 0x00007fff687776a6 abort + 127
7 llvm-tblgen 0x00000001055ad340 llvm::install_out_of_memory_new_handler() + 0
8 llvm-tblgen 0x0000000105171382 llvm::TypeInfer::ValidateOnExit::~ValidateOnExit() + 114
9 llvm-tblgen 0x0000000105166695 llvm::TypeInfer::ValidateOnExit::~ValidateOnExit() + 21
10 llvm-tblgen 0x00000001051664c1 llvm::TypeInfer::MergeInTypeInfo(llvm::TypeSetByHwMode&, llvm::TypeSetByHwMode const&) + 337
11 llvm-tblgen 0x0000000105176eb1 llvm::TreePatternNode::UpdateNodeType(unsigned int, llvm::TypeSetByHwMode const&, llvm::TreePattern&) + 113
12 llvm-tblgen 0x000000010517730b llvm::TreePatternNode::UpdateNodeTypeFromInst(unsigned int, llvm::Record*, llvm::TreePattern&) + 923
13 llvm-tblgen 0x000000010517d872 llvm::TreePatternNode::ApplyTypeConstraints(llvm::TreePattern&, bool) + 6002
14 llvm-tblgen 0x0000000105182a4b llvm::TreePattern::InferAllTypes(llvm::StringMap<llvm::SmallVector<llvm::TreePatternNode*, 1u>, llvm::MallocAllocator> const*) + 187
15 llvm-tblgen 0x000000010518c56d llvm::CodeGenDAGPatterns::ParseOnePattern(llvm::Record*, llvm::TreePattern&, llvm::TreePattern&, std::__1::vector<llvm::Record*, std::__1::allocator<llvm::Record*> > const&) + 205
16 llvm-tblgen 0x0000000105185edb llvm::CodeGenDAGPatterns::ParsePatterns() + 699
17 llvm-tblgen 0x0000000105183d9b llvm::CodeGenDAGPatterns::CodeGenDAGPatterns(llvm::RecordKeeper&, std::__1::function<void (llvm::TreePattern*)>) + 587
18 llvm-tblgen 0x0000000105187e2d llvm::CodeGenDAGPatterns::CodeGenDAGPatterns(llvm::RecordKeeper&, std::__1::function<void (llvm::TreePattern*)>) + 29
19 llvm-tblgen 0x0000000105418404 (anonymous namespace)::InstrInfoEmitter::InstrInfoEmitter(llvm::RecordKeeper&) + 100
20 llvm-tblgen 0x00000001054176dd (anonymous namespace)::InstrInfoEmitter::InstrInfoEmitter(llvm::RecordKeeper&) + 29
21 llvm-tblgen 0x000000010541765a llvm::EmitInstrInfo(llvm::RecordKeeper&, llvm::raw_ostream&) + 58
22 llvm-tblgen 0x000000010550dc9c (anonymous namespace)::LLVMTableGenMain(llvm::raw_ostream&, llvm::RecordKeeper&) + 172
23 llvm-tblgen 0x000000010565c3ed llvm::TableGenMain(char*, bool (*)(llvm::raw_ostream&, llvm::RecordKeeper&)) + 781
24 llvm-tblgen 0x000000010550db90 main + 192
25 libdyld.dylib 0x00007fff686d23d5 start + 1
Any ideas?
> On Jul 4, 2019, at 04:43, Simon Tatham via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: statham
> Date: Thu Jul 4 01:43:20 2019
> New Revision: 365114
>
> URL: http://llvm.org/viewvc/llvm-project?rev=365114&view=rev
> Log:
> [TableGen] Allow DAG isel patterns to override default operands.
>
> When a Tablegen instruction description uses `OperandWithDefaultOps`,
> isel patterns for that instruction don't have to fill in the default
> value for the operand in question. But the flip side is that they
> actually //can't// override the defaults even if they want to.
>
> This will be very inconvenient for the Arm backend, when we start
> wanting to write isel patterns that generate the many MVE predicated
> vector instructions, in the form with predication actually enabled. So
> this small Tablegen fix makes it possible to write an isel pattern
> either with or without values for a defaulted operand, and have the
> default values filled in only if they are not overridden.
>
> If all the defaulted operands come at the end of the instruction's
> operand list, there's a natural way to match them up to the arguments
> supplied in the pattern: consume pattern arguments until you run out,
> then fill in any missing instruction operands with their default
> values. But if defaulted and non-defaulted operands are interleaved,
> it's less clear what to do. This does happen in existing targets (the
> first example I came across was KILLGT, in the AMDGPU/R600 backend),
> and of course they expect the previous behaviour (that the default for
> those operands is used and a pattern argument is not consumed), so for
> backwards compatibility I've stuck with that.
>
> Reviewers: nhaehnle, hfinkel, dmgreen
>
> Subscribers: mehdi_amini, javed.absar, tpr, kristof.beyls, steven_wu, dexonsmith, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D63814
>
> Added:
> llvm/trunk/test/TableGen/DAGDefaultOps.td
> Modified:
> llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
> llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h
> llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp
>
> Added: llvm/trunk/test/TableGen/DAGDefaultOps.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/DAGDefaultOps.td?rev=365114&view=auto
> ==============================================================================
> --- llvm/trunk/test/TableGen/DAGDefaultOps.td (added)
> +++ llvm/trunk/test/TableGen/DAGDefaultOps.td Thu Jul 4 01:43:20 2019
> @@ -0,0 +1,108 @@
> +// RUN: llvm-tblgen -gen-dag-isel -I %p/../../include %s -o %t
> +// RUN: FileCheck --check-prefix=ADD %s < %t
> +// RUN: FileCheck --check-prefix=ADDINT %s < %t
> +// RUN: FileCheck --check-prefix=SUB %s < %t
> +// RUN: FileCheck --check-prefix=MULINT %s < %t
> +
> +include "llvm/Target/Target.td"
> +
> +def TestInstrInfo : InstrInfo;
> +def TestTarget : Target {
> + let InstructionSet = TestInstrInfo;
> +}
> +
> +class TestEncoding : Instruction {
> + field bits<32> Inst;
> +}
> +
> +class TestReg<int index> : Register<"R"#index, []> {
> + let HWEncoding{15-4} = 0;
> + let HWEncoding{3-0} = !cast<bits<4>>(index);
> +}
> +foreach i = 0-15 in
> + def "R"#i : TestReg<i>;
> +
> +def Reg : RegisterClass<"TestTarget", [i32], 32, (sequence "R%d", 0, 15)>;
> +
> +def IntOperand: Operand<i32>;
> +def OptionalIntOperand: OperandWithDefaultOps<i32, (ops (i32 0))>;
> +
> +class RRI<string Mnemonic, bits<4> Opcode> : TestEncoding {
> + dag OutOperandList = (outs Reg:$dest);
> + dag InOperandList = (ins Reg:$src1, Reg:$src2, OptionalIntOperand:$imm);
> + string AsmString = Mnemonic # " $dest1, $src1, $src2, #$imm";
> + string AsmVariantName = "";
> + field bits<4> dest;
> + field bits<4> src1;
> + field bits<4> src2;
> + field bits<16> imm;
> + let Inst{31-28} = Opcode;
> + let Inst{27-24} = dest;
> + let Inst{23-20} = src1;
> + let Inst{19-16} = src2;
> + let Inst{15-0} = imm;
> +}
> +
> +def AddRRI : RRI<"add", 0b0001>;
> +
> +// I define one of these intrinsics with IntrNoMem and the other
> +// without it, so that they'll match different top-level DAG opcodes
> +// (INTRINSIC_WO_CHAIN and INTRINSIC_W_CHAIN), which makes the
> +// FileCheck-herding easier, because every case I want to detect
> +// should show up as a separate top-level switch element.
> +def int_addplus1 : Intrinsic<
> + [llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +def int_mul3 : Intrinsic<
> + [llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty]>;
> +
> +def AddPat : Pat<(add i32:$x, i32:$y),
> + (AddRRI Reg:$x, Reg:$y)>;
> +def Add1Pat : Pat<(int_addplus1 i32:$x, i32:$y),
> + (AddRRI Reg:$x, Reg:$y, (i32 1))>;
> +
> +def SubRRI : RRI<"sub", 0b0010> {
> + let Pattern = [(set Reg:$dest, (sub Reg:$src1, Reg:$src2))];
> +}
> +
> +def MulRRI : RRI<"mul", 0b0011> {
> + let Pattern = [(set Reg:$dest, (int_mul3 Reg:$src1, Reg:$src2, i32:$imm))];
> +}
> +
> +def MulIRR : RRI<"mul2", 0b0100> {
> + let InOperandList = (ins OptionalIntOperand:$imm, Reg:$src1, Reg:$src2);
> +}
> +def MulIRRPat : Pat<(mul i32:$x, i32:$y), (MulIRR Reg:$x, Reg:$y)>;
> +
> +// ADD: SwitchOpcode{{.*}}TARGET_VAL(ISD::ADD)
> +// ADD-NEXT: OPC_RecordChild0
> +// ADD-NEXT: OPC_RecordChild1
> +// ADD-NEXT: OPC_EmitInteger, MVT::i32, 0
> +// ADD-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::AddRRI)
> +
> +// ADDINT: SwitchOpcode{{.*}}TARGET_VAL(ISD::INTRINSIC_WO_CHAIN)
> +// ADDINT-NEXT: OPC_CheckChild0Integer
> +// ADDINT-NEXT: OPC_RecordChild1
> +// ADDINT-NEXT: OPC_RecordChild2
> +// ADDINT-NEXT: OPC_EmitInteger, MVT::i32, 1
> +// ADDINT-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::AddRRI)
> +
> +// SUB: SwitchOpcode{{.*}}TARGET_VAL(ISD::SUB)
> +// SUB-NEXT: OPC_RecordChild0
> +// SUB-NEXT: OPC_RecordChild1
> +// SUB-NEXT: OPC_EmitInteger, MVT::i32, 0
> +// SUB-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::SubRRI)
> +
> +// MULINT: SwitchOpcode{{.*}}TARGET_VAL(ISD::INTRINSIC_W_CHAIN)
> +// MULINT-NEXT: OPC_RecordNode
> +// MULINT-NEXT: OPC_CheckChild1Integer
> +// MULINT-NEXT: OPC_RecordChild2
> +// MULINT-NEXT: OPC_RecordChild3
> +// MULINT-NEXT: OPC_RecordChild4
> +// MULINT-NEXT: OPC_EmitMergeInputChains
> +// MULINT-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::MulRRI)
> +
> +// MUL: SwitchOpcode{{.*}}TARGET_VAL(ISD::MUL)
> +// MUL-NEXT: OPC_EmitInteger, MVT::i32, 0
> +// MUL-NEXT: OPC_RecordChild0
> +// MUL-NEXT: OPC_RecordChild1
> +// MUL-NEXT: OPC_MorphNodeTo1, TARGET_VAL(::MulRRI)
>
> Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=365114&r1=365113&r2=365114&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp (original)
> +++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp Thu Jul 4 01:43:20 2019
> @@ -2437,18 +2437,32 @@ bool TreePatternNode::ApplyTypeConstrain
> }
> }
>
> + // If one or more operands with a default value appear at the end of the
> + // formal operand list for an instruction, we allow them to be overridden
> + // by optional operands provided in the pattern.
> + //
> + // But if an operand B without a default appears at any point after an
> + // operand A with a default, then we don't allow A to be overridden,
> + // because there would be no way to specify whether the next operand in
> + // the pattern was intended to override A or skip it.
> + unsigned NonOverridableOperands = Inst.getNumOperands();
> + while (NonOverridableOperands > 0 &&
> + CDP.operandHasDefault(Inst.getOperand(NonOverridableOperands-1)))
> + --NonOverridableOperands;
> +
> unsigned ChildNo = 0;
> for (unsigned i = 0, e = Inst.getNumOperands(); i != e; ++i) {
> Record *OperandNode = Inst.getOperand(i);
>
> - // If the instruction expects a predicate or optional def operand, we
> - // codegen this by setting the operand to it's default value if it has a
> - // non-empty DefaultOps field.
> - if (OperandNode->isSubClassOf("OperandWithDefaultOps") &&
> - !CDP.getDefaultOperand(OperandNode).DefaultOps.empty())
> + // If the operand has a default value, do we use it? We must use the
> + // default if we've run out of children of the pattern DAG to consume,
> + // or if the operand is followed by a non-defaulted one.
> + if (CDP.operandHasDefault(OperandNode) &&
> + (i < NonOverridableOperands || ChildNo >= getNumChildren()))
> continue;
>
> - // Verify that we didn't run out of provided operands.
> + // If we have run out of child nodes and there _isn't_ a default
> + // value we can use for the next operand, give an error.
> if (ChildNo >= getNumChildren()) {
> emitTooFewOperandsError(TP, getOperator()->getName(), getNumChildren());
> return false;
>
> Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h?rev=365114&r1=365113&r2=365114&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h (original)
> +++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h Thu Jul 4 01:43:20 2019
> @@ -1282,6 +1282,11 @@ public:
>
> unsigned allocateScope() { return ++NumScopes; }
>
> + bool operandHasDefault(Record *Op) const {
> + return Op->isSubClassOf("OperandWithDefaultOps") &&
> + !getDefaultOperand(Op).DefaultOps.empty();
> + }
> +
> private:
> void ParseNodeInfo();
> void ParseNodeTransforms();
>
> Modified: llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp?rev=365114&r1=365113&r2=365114&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp (original)
> +++ llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp Thu Jul 4 01:43:20 2019
> @@ -794,14 +794,27 @@ EmitResultInstructionAsOperand(const Tre
> // 'execute always' values. Match up the node operands to the instruction
> // operands to do this.
> unsigned ChildNo = 0;
> +
> + // Similarly to the code in TreePatternNode::ApplyTypeConstraints, count the
> + // number of operands at the end of the list which have default values.
> + // Those can come from the pattern if it provides enough arguments, or be
> + // filled in with the default if the pattern hasn't provided them. But any
> + // operand with a default value _before_ the last mandatory one will be
> + // filled in with their defaults unconditionally.
> + unsigned NonOverridableOperands = NumFixedOperands;
> + while (NonOverridableOperands > NumResults &&
> + CGP.operandHasDefault(II.Operands[NonOverridableOperands-1].Rec))
> + --NonOverridableOperands;
> +
> for (unsigned InstOpNo = NumResults, e = NumFixedOperands;
> InstOpNo != e; ++InstOpNo) {
> // Determine what to emit for this operand.
> Record *OperandNode = II.Operands[InstOpNo].Rec;
> - if (OperandNode->isSubClassOf("OperandWithDefaultOps") &&
> - !CGP.getDefaultOperand(OperandNode).DefaultOps.empty()) {
> - // This is a predicate or optional def operand; emit the
> - // 'default ops' operands.
> + if (CGP.operandHasDefault(OperandNode) &&
> + (InstOpNo < NonOverridableOperands || ChildNo >= N->getNumChildren())) {
> + // This is a predicate or optional def operand which the pattern has not
> + // overridden, or which we aren't letting it override; emit the 'default
> + // ops' operands.
> const DAGDefaultOperand &DefaultOp
> = CGP.getDefaultOperand(OperandNode);
> for (unsigned i = 0, e = DefaultOp.DefaultOps.size(); i != e; ++i)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list