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