[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
Wed May 24 02:24:24 PDT 2017


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 <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 <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 <mailto: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 <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 <mailto: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>
>>> 
>> 
>> _______________________________________________
>> 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
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170524/87f7f5e2/attachment.html>


More information about the llvm-commits mailing list