From 13bdae8541c3fc5acf6ee7de78ec5ab8446848e4 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Thu, 19 Sep 2019 12:33:07 +0000 Subject: Revert r372285 "GlobalISel: Don't materialize immarg arguments to intrinsics" This broke the Chromium build, causing it to fail with e.g. fatal error: error in backend: Cannot select: t362: v4i32 = X86ISD::VSHLI t392, Constant:i8<15> See llvm-commits thread of r372285 for details. This also reverts r372286, r372287, r372288, r372289, r372290, r372291, r372292, r372293, r372296, and r372297, which seemed to depend on the main commit. > Encode them directly as an imm argument to G_INTRINSIC*. > > Since now intrinsics can now define what parameters are required to be > immediates, avoid using registers for them. Intrinsics could > potentially want a constant that isn't a legal register type. Also, > since G_CONSTANT is subject to CSE and legalization, transforms could > potentially obscure the value (and create extra work for the > selector). The register bank of a G_CONSTANT is also meaningful, so > this could throw off future folding and legalization logic for AMDGPU. > > This will be much more convenient to work with than needing to call > getConstantVRegVal and checking if it may have failed for every > constant intrinsic parameter. AMDGPU has quite a lot of intrinsics wth > immarg operands, many of which need inspection during lowering. Having > to find the value in a register is going to add a lot of boilerplate > and waste compile time. > > SelectionDAG has always provided TargetConstant for constants which > should not be legalized or materialized in a register. The distinction > between Constant and TargetConstant was somewhat fuzzy, and there was > no automatic way to force usage of TargetConstant for certain > intrinsic parameters. They were both ultimately ConstantSDNode, and it > was inconsistently used. It was quite easy to mis-select an > instruction requiring an immediate. For SelectionDAG, start emitting > TargetConstant for these arguments, and using timm to match them. > > Most of the work here is to cleanup target handling of constants. Some > targets process intrinsics through intermediate custom nodes, which > need to preserve TargetConstant usage to match the intrinsic > expectation. Pattern inputs now need to distinguish whether a constant > is merely compatible with an operand or whether it is mandatory. > > The GlobalISelEmitter needs to treat timm as a special case of a leaf > node, simlar to MachineBasicBlock operands. This should also enable > handling of patterns for some G_* instructions with immediates, like > G_FENCE or G_EXTRACT. > > This does include a workaround for a crash in GlobalISelEmitter when > ARM tries to uses "imm" in an output with a "timm" pattern source. llvm-svn: 372314 --- llvm/utils/TableGen/GlobalISelEmitter.cpp | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) (limited to 'llvm/utils/TableGen/GlobalISelEmitter.cpp') diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp index d8d4c9f4f55..fbf0b4bd9dd 100644 --- a/llvm/utils/TableGen/GlobalISelEmitter.cpp +++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp @@ -1062,7 +1062,6 @@ public: IPM_Opcode, IPM_NumOperands, IPM_ImmPredicate, - IPM_Imm, IPM_AtomicOrderingMMO, IPM_MemoryLLTSize, IPM_MemoryVsLLTSize, @@ -1341,23 +1340,6 @@ public: } }; -class ImmOperandMatcher : public OperandPredicateMatcher { -public: - ImmOperandMatcher(unsigned InsnVarID, unsigned OpIdx) - : OperandPredicateMatcher(IPM_Imm, InsnVarID, OpIdx) {} - - static bool classof(const PredicateMatcher *P) { - return P->getKind() == IPM_Imm; - } - - void emitPredicateOpcodes(MatchTable &Table, - RuleMatcher &Rule) const override { - Table << MatchTable::Opcode("GIM_CheckIsImm") << MatchTable::Comment("MI") - << MatchTable::IntValue(InsnVarID) << MatchTable::Comment("Op") - << MatchTable::IntValue(OpIdx) << MatchTable::LineBreak; - } -}; - /// Generates code to check that an operand is a G_CONSTANT with a particular /// int. class ConstantIntOperandMatcher : public OperandPredicateMatcher { @@ -3812,10 +3794,6 @@ Error GlobalISelEmitter::importChildMatcher(RuleMatcher &Rule, OM.addPredicate(); return Error::success(); } - if (SrcChild->getOperator()->getName() == "timm") { - OM.addPredicate(); - return Error::success(); - } } } @@ -3965,10 +3943,7 @@ Expected GlobalISelEmitter::importExplicitUseRenderer( // rendered as operands. // FIXME: The target should be able to choose sign-extended when appropriate // (e.g. on Mips). - if (DstChild->getOperator()->getName() == "timm") { - DstMIBuilder.addRenderer(DstChild->getName()); - return InsertPt; - } else if (DstChild->getOperator()->getName() == "imm") { + if (DstChild->getOperator()->getName() == "imm") { DstMIBuilder.addRenderer(DstChild->getName()); return InsertPt; } else if (DstChild->getOperator()->getName() == "fpimm") { -- cgit v1.2.3