[llvm] r303678 - [globalisel][tablegen] Add support for (set $dst, 1) and test X86's OptForSize predicate.
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Tue May 23 23:13:04 PDT 2017
Hi Galina,
That test is new in this commit and it seems that optsize isn't having the desired effect on this machine. I'd hazard a guess that this buildbot has +slow-incdec set via the default processor and this is telling the instruction selector not to use inc/dec even when optimizing for size. I've tweaked the test to disable this feature using -mattr.
Sorry for leaving this broken overnight. I was keeping an eye on lab.llvm.org:8011/console <http://lab.llvm.org:8011/console> but that didn't report this builder.
> On 24 May 2017, at 00:11, Galina Kistanova <gkistanova at gmail.com> wrote:
>
> Hello Daniel,
>
> This commit broke tests on one of our builders:
>
> Failing Tests (1):
> LLVM :: CodeGen/X86/GlobalISel/select-leaf-constant.mir
>
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win <http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win>
>
> Please have a look at this?
>
> Thanks
>
> Galina
>
>
> On Tue, May 23, 2017 at 12:33 PM, Daniel Sanders via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: dsanders
> Date: Tue May 23 14:33:16 2017
> New Revision: 303678
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303678&view=rev <http://llvm.org/viewvc/llvm-project?rev=303678&view=rev>
> Log:
> [globalisel][tablegen] Add support for (set $dst, 1) and test X86's OptForSize predicate.
>
> Summary:
> It's rare but a small number of patterns use IntInit's at the root of the match.
> On X86, one such rule is enabled by the OptForSize predicate and causes the
> compiler to use the smaller:
> %0 = MOV32r1
> instead of the usual:
> %0 = MOV32ri 1
>
> This patch adds support for matching IntInit's at the root and uses this as a
> test case for the optsize attribute that was implemented in r301750
>
> Reviewers: qcolombet, ab, t.p.northover, rovka, kristof.beyls, aditya_nandakumar
>
> Reviewed By: qcolombet
>
> Subscribers: igorb, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D32791 <https://reviews.llvm.org/D32791>
>
> Added:
> llvm/trunk/test/CodeGen/X86/GlobalISel/select-leaf-constant.mir
> Modified:
> llvm/trunk/test/TableGen/GlobalISelEmitter.td
> llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>
> Added: llvm/trunk/test/CodeGen/X86/GlobalISel/select-leaf-constant.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/GlobalISel/select-leaf-constant.mir?rev=303678&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/GlobalISel/select-leaf-constant.mir?rev=303678&view=auto>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/GlobalISel/select-leaf-constant.mir (added)
> +++ llvm/trunk/test/CodeGen/X86/GlobalISel/select-leaf-constant.mir Tue May 23 14:33:16 2017
> @@ -0,0 +1,96 @@
> +# RUN: llc -mtriple=i586-linux-gnu -global-isel -run-pass=instruction-select %s -o - | FileCheck %s --check-prefix=CHECK
> +#
> +# This is necessary to test that attribute-based rule predicates work and that
> +# they properly reset between functions.
> +
> +--- |
> + define i32 @const_i32_1() {
> + ret i32 1
> + }
> +
> + define i32 @const_i32_1_optsize() #0 {
> + ret i32 1
> + }
> +
> + define i32 @const_i32_1b() {
> + ret i32 1
> + }
> +
> + define i32 @const_i32_1_optsizeb() #0 {
> + ret i32 1
> + }
> +
> + attributes #0 = { optsize }
> +...
> +---
> +name: const_i32_1
> +legalized: true
> +regBankSelected: true
> +selected: false
> +# CHECK-LABEL: name: const_i32_1
> +# CHECK: registers:
> +# CHECK-NEXT: - { id: 0, class: gr32 }
> +registers:
> + - { id: 0, class: gpr }
> +# CHECK: body:
> +# CHECK: %0 = MOV32ri 1
> +body: |
> + bb.1 (%ir-block.0):
> + %0(s32) = G_CONSTANT i32 1
> + %eax = COPY %0(s32)
> + RET 0, implicit %eax
> +...
> +---
> +name: const_i32_1_optsize
> +legalized: true
> +regBankSelected: true
> +selected: false
> +# CHECK-LABEL: name: const_i32_1_optsize
> +# CHECK: registers:
> +# CHECK-NEXT: - { id: 0, class: gr32 }
> +registers:
> + - { id: 0, class: gpr }
> +# CHECK: body:
> +# CHECK: %0 = MOV32r1
> +body: |
> + bb.1 (%ir-block.0):
> + %0(s32) = G_CONSTANT i32 1
> + %eax = COPY %0(s32)
> + RET 0, implicit %eax
> +...
> +---
> +name: const_i32_1b
> +legalized: true
> +regBankSelected: true
> +selected: false
> +# CHECK-LABEL: name: const_i32_1b
> +# CHECK: registers:
> +# CHECK-NEXT: - { id: 0, class: gr32 }
> +registers:
> + - { id: 0, class: gpr }
> +# CHECK: body:
> +# CHECK: %0 = MOV32ri 1
> +body: |
> + bb.1 (%ir-block.0):
> + %0(s32) = G_CONSTANT i32 1
> + %eax = COPY %0(s32)
> + RET 0, implicit %eax
> +...
> +---
> +name: const_i32_1_optsizeb
> +legalized: true
> +regBankSelected: true
> +selected: false
> +# CHECK-LABEL: name: const_i32_1_optsizeb
> +# CHECK: registers:
> +# CHECK-NEXT: - { id: 0, class: gr32 }
> +registers:
> + - { id: 0, class: gpr }
> +# CHECK: body:
> +# CHECK: %0 = MOV32r1
> +body: |
> + bb.1 (%ir-block.0):
> + %0(s32) = G_CONSTANT i32 1
> + %eax = COPY %0(s32)
> + RET 0, implicit %eax
> +...
>
> Modified: llvm/trunk/test/TableGen/GlobalISelEmitter.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=303678&r1=303677&r2=303678&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=303678&r1=303677&r2=303678&view=diff>
> ==============================================================================
> --- llvm/trunk/test/TableGen/GlobalISelEmitter.td (original)
> +++ llvm/trunk/test/TableGen/GlobalISelEmitter.td Tue May 23 14:33:16 2017
> @@ -462,6 +462,32 @@ def XORManyDefaults : I<(outs GPR32:$dst
> def ORN : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
> def : Pat<(not GPR32:$Wm), (ORN R0, GPR32:$Wm)>;
>
> +//===- Test a simple pattern with just a leaf immediate. ------------------===//
> +
> +// CHECK-LABEL: if ([&]() {
> +// CHECK-NEXT: MachineInstr &MI0 = I;
> +// CHECK-NEXT: if (MI0.getNumOperands() < 2)
> +// CHECK-NEXT: return false;
> +// CHECK-NEXT: if ((MI0.getOpcode() == TargetOpcode::G_CONSTANT) &&
> +// CHECK-NEXT: ((/* dst */ (MRI.getType(MI0.getOperand(0).getReg()) == (LLT::scalar(32))) &&
> +// CHECK-NEXT: ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(0).getReg(), MRI, TRI))))) &&
> +// CHECK-NEXT: ((/* Operand 1 */ (MI0.getOperand(1).isCImm() && MI0.getOperand(1).getCImm()->equalsInt(1))))) {
> +// CHECK-NEXT: // 1:i32 => (MOV1:i32)
> +// CHECK-NEXT: MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::MOV1));
> +// CHECK-NEXT: MIB.add(MI0.getOperand(0)/*dst*/);
> +// CHECK-NEXT: for (const auto *FromMI : {&MI0, })
> +// CHECK-NEXT: for (const auto &MMO : FromMI->memoperands())
> +// CHECK-NEXT: MIB.addMemOperand(MMO);
> +// CHECK-NEXT: I.eraseFromParent();
> +// CHECK-NEXT: MachineInstr &NewI = *MIB;
> +// CHECK-NEXT: constrainSelectedInstRegOperands(NewI, TII, TRI, RBI);
> +// CHECK-NEXT: return true;
> +// CHECK-NEXT: }
> +// CHECK-NEXT: return false;
> +// CHECK-NEXT: }()) { return true; }
> +
> +def MOV1 : I<(outs GPR32:$dst), (ins), [(set GPR32:$dst, 1)]>;
> +
> //===- Test a pattern with an MBB operand. --------------------------------===//
>
> // CHECK-LABEL: if ([&]() {
>
> Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=303678&r1=303677&r2=303678&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=303678&r1=303677&r2=303678&view=diff>
> ==============================================================================
> --- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Tue May 23 14:33:16 2017
> @@ -135,6 +135,9 @@ static Error isTrivialOperatorNode(const
> std::string Explanation = "";
> std::string Separator = "";
> if (N->isLeaf()) {
> + if (IntInit *Int = dyn_cast<IntInit>(N->getLeafValue()))
> + return Error::success();
> +
> Explanation = "Is a leaf";
> Separator = ", ";
> }
> @@ -272,6 +275,7 @@ public:
> OPM_ComplexPattern,
> OPM_Instruction,
> OPM_Int,
> + OPM_LiteralInt,
> OPM_LLT,
> OPM_RegBank,
> OPM_MBB,
> @@ -406,13 +410,14 @@ public:
> }
> };
>
> -/// Generates code to check that an operand is a particular int.
> -class IntOperandMatcher : public OperandPredicateMatcher {
> +/// Generates code to check that an operand is a G_CONSTANT with a particular
> +/// int.
> +class ConstantIntOperandMatcher : public OperandPredicateMatcher {
> protected:
> int64_t Value;
>
> public:
> - IntOperandMatcher(int64_t Value)
> + ConstantIntOperandMatcher(int64_t Value)
> : OperandPredicateMatcher(OPM_Int), Value(Value) {}
>
> static bool classof(const OperandPredicateMatcher *P) {
> @@ -425,6 +430,27 @@ public:
> }
> };
>
> +/// Generates code to check that an operand is a raw int (where MO.isImm() or
> +/// MO.isCImm() is true).
> +class LiteralIntOperandMatcher : public OperandPredicateMatcher {
> +protected:
> + int64_t Value;
> +
> +public:
> + LiteralIntOperandMatcher(int64_t Value)
> + : OperandPredicateMatcher(OPM_LiteralInt), Value(Value) {}
> +
> + static bool classof(const OperandPredicateMatcher *P) {
> + return P->getKind() == OPM_LiteralInt;
> + }
> +
> + void emitCxxPredicateExpr(raw_ostream &OS, RuleMatcher &Rule,
> + StringRef OperandExpr) const override {
> + OS << OperandExpr << ".isCImm() && " << OperandExpr
> + << ".getCImm()->equalsInt(" << Value << ")";
> + }
> +};
> +
> /// Generates code to check that a set of predicates match for a particular
> /// operand.
> class OperandMatcher : public PredicateListMatcher<OperandPredicateMatcher> {
> @@ -1236,7 +1262,7 @@ private:
> createAndImportSelDAGMatcher(InstructionMatcher &InsnMatcher,
> const TreePatternNode *Src) const;
> Error importChildMatcher(InstructionMatcher &InsnMatcher,
> - TreePatternNode *SrcChild, unsigned OpIdx,
> + const TreePatternNode *SrcChild, unsigned OpIdx,
> unsigned &TempOpIdx) const;
> Expected<BuildMIAction &> createAndImportInstructionRenderer(
> RuleMatcher &M, const TreePatternNode *Dst,
> @@ -1299,14 +1325,23 @@ Expected<InstructionMatcher &> GlobalISe
> if (Src->getExtTypes().size() > 1)
> return failedImport("Src pattern has multiple results");
>
> - auto SrcGIOrNull = findNodeEquiv(Src->getOperator());
> - if (!SrcGIOrNull)
> - return failedImport("Pattern operator lacks an equivalent Instruction" +
> - explainOperator(Src->getOperator()));
> - auto &SrcGI = *SrcGIOrNull;
> + if (Src->isLeaf()) {
> + Init *SrcInit = Src->getLeafValue();
> + if (IntInit *SrcIntInit = dyn_cast<IntInit>(SrcInit)) {
> + InsnMatcher.addPredicate<InstructionOpcodeMatcher>(
> + &Target.getInstruction(RK.getDef("G_CONSTANT")));
> + } else
> + return failedImport("Unable to deduce gMIR opcode to handle Src (which is a leaf)");
> + } else {
> + auto SrcGIOrNull = findNodeEquiv(Src->getOperator());
> + if (!SrcGIOrNull)
> + return failedImport("Pattern operator lacks an equivalent Instruction" +
> + explainOperator(Src->getOperator()));
> + auto &SrcGI = *SrcGIOrNull;
>
> - // The operators look good: match the opcode and mutate it to the new one.
> - InsnMatcher.addPredicate<InstructionOpcodeMatcher>(&SrcGI);
> + // The operators look good: match the opcode
> + InsnMatcher.addPredicate<InstructionOpcodeMatcher>(&SrcGI);
> + }
>
> unsigned OpIdx = 0;
> unsigned TempOpIdx = 0;
> @@ -1323,18 +1358,27 @@ Expected<InstructionMatcher &> GlobalISe
> OM.addPredicate<LLTOperandMatcher>(*OpTyOrNone);
> }
>
> - // Match the used operands (i.e. the children of the operator).
> - for (unsigned i = 0, e = Src->getNumChildren(); i != e; ++i) {
> - if (auto Error = importChildMatcher(InsnMatcher, Src->getChild(i), OpIdx++,
> - TempOpIdx))
> - return std::move(Error);
> + if (Src->isLeaf()) {
> + Init *SrcInit = Src->getLeafValue();
> + if (IntInit *SrcIntInit = dyn_cast<IntInit>(SrcInit)) {
> + OperandMatcher &OM = InsnMatcher.addOperand(OpIdx++, "", TempOpIdx);
> + OM.addPredicate<LiteralIntOperandMatcher>(SrcIntInit->getValue());
> + } else
> + return failedImport("Unable to deduce gMIR opcode to handle Src (which is a leaf)");
> + } else {
> + // Match the used operands (i.e. the children of the operator).
> + for (unsigned i = 0, e = Src->getNumChildren(); i != e; ++i) {
> + if (auto Error = importChildMatcher(InsnMatcher, Src->getChild(i),
> + OpIdx++, TempOpIdx))
> + return std::move(Error);
> + }
> }
>
> return InsnMatcher;
> }
>
> Error GlobalISelEmitter::importChildMatcher(InstructionMatcher &InsnMatcher,
> - TreePatternNode *SrcChild,
> + const TreePatternNode *SrcChild,
> unsigned OpIdx,
> unsigned &TempOpIdx) const {
> OperandMatcher &OM =
> @@ -1379,7 +1423,7 @@ Error GlobalISelEmitter::importChildMatc
>
> // Check for constant immediates.
> if (auto *ChildInt = dyn_cast<IntInit>(SrcChild->getLeafValue())) {
> - OM.addPredicate<IntOperandMatcher>(ChildInt->getValue());
> + OM.addPredicate<ConstantIntOperandMatcher>(ChildInt->getValue());
> return Error::success();
> }
>
> @@ -1605,6 +1649,9 @@ Expected<RuleMatcher> GlobalISelEmitter:
> return failedImport("Src pattern root isn't a trivial operator (" +
> toString(std::move(Err)) + ")");
>
> + if (Dst->isLeaf())
> + return failedImport("Dst pattern root isn't a known leaf");
> +
> // Start with the defined operands (i.e., the results of the root operator).
> Record *DstOp = Dst->getOperator();
> if (!DstOp->isSubClassOf("Instruction"))
>
>
> _______________________________________________
> 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/20170524/2b091a0b/attachment.html>
More information about the llvm-commits
mailing list