[llvm] r303678 - [globalisel][tablegen] Add support for (set $dst, 1) and test X86's OptForSize predicate.

Galina Kistanova via llvm-commits llvm-commits at lists.llvm.org
Wed May 24 09:00:38 PDT 2017


Thank you!

On Wed, May 24, 2017 at 2:24 AM, Daniel Sanders <daniel_l_sanders at apple.com>
wrote:

> The clean build did the trick.
>
>
> On 24 May 2017, at 08:59, Daniel Sanders <daniel_l_sanders at apple.com>
> wrote:
>
> Unfortunately that didn't fix it. I notice that the test is passing on the
> other builder that is running on that buildslave (
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_
> 64-scei-ps4-windows10pro-fast/builds/9844) so it's unlikely to be
> something to do with the machine.
>
> I don't see any mention of generating X86GenGlobalISel.inc in
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-
> expensive-checks-win/builds/2552 so I've started a clean build.
>
> On 24 May 2017, at 07:13, Daniel Sanders via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> 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 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
>
> 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> 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
>> 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
>>
>> 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
>> ============================================================
>> ==================
>> --- 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
>> ============================================================
>> ==================
>> --- 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/TableGe
>> n/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
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> 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/9f66c03f/attachment-0001.html>


More information about the llvm-commits mailing list