<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Dec 9, 2014 at 5:36 PM, Juergen Ributzka <span dir="ltr"><<a href="mailto:juergen@apple.com" target="_blank">juergen@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ributzka<br>
Date: Tue Dec 9 10:36:10 2014<br>
New Revision: 223785<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=223785&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=223785&view=rev</a><br>
Log:<br>
Add more pattern matchers for compares, instructions, and BinaryOperators. NFC.<br>
<br>
Add a few more matchers to make the code in the next commit more compact.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/IR/PatternMatch.h<br>
<br>
Modified: llvm/trunk/include/llvm/IR/PatternMatch.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/PatternMatch.h?rev=223785&r1=223784&r2=223785&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/PatternMatch.h?rev=223785&r1=223784&r2=223785&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/IR/PatternMatch.h (original)<br>
+++ llvm/trunk/include/llvm/IR/PatternMatch.h Tue Dec 9 10:36:10 2014<br>
@@ -68,6 +68,18 @@ struct class_match {<br>
<br>
/// m_Value() - Match an arbitrary value and ignore it.<br>
inline class_match<Value> m_Value() { return class_match<Value>(); }<br></blockquote><div><br></div><div>Please keep a blank line between top-level constructs, especially those preceded by comments, especially doxygen comments. It makes reading *much* easier.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+/// m_Instruction() - Match an arbitrary instruction and ignore it.<br></blockquote><div><br></div><div>Please don't repeat function names in doxygen comments. I know lots of code does this, but new code shouldn't.</div><div><br></div><div>I'm also surprised at the instruction matcher. Why would Value not work? Almost all of pattern match works very hard to match generically across both instructions and constants, and it seems strange to start diverging now.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+inline class_match<Instruction> m_Instruction() {<br>
+ return class_match<Instruction>();<br>
+}<br>
+/// m_BinOp() - Match an arbitrary binary operation and ignore it.<br>
+inline class_match<BinaryOperator> m_BinOp() {<br>
+ return class_match<BinaryOperator>();<br>
+}<br>
+/// m_Cmp() - Matches any compare instruction and ignore it.<br>
+inline class_match<CmpInst> m_Cmp() {<br>
+ return class_match<CmpInst>();<br>
+}<br></blockquote><div><br></div><div>I see you're just replicating other matchers. I think it makes a lot more sense to organize them together. For example, having this here, and then a capturing matcher below, and then a *non* capturing matcher that works totally differently from this still further down doesn't really make sense to me.</div><div><br></div><div>What this really indicates is a fundamental flaw in the matcher interface: it is not well suited to matching instructions when you need to capture them while matching. I tried to use it to do that just the other day as well, and wrote code to extend it in similar ways, but the code piles up and doesn't really make sense. And it doesn't make the call sites that much better either.</div><div><br></div><div>With the current design of the pattern match library, I don't think it makes sense to do non-leaf-matching patterns with it. To do those, you really want to decouple the pattern structure from the binding action (much as we did with the ASTMatchers in Clang) but that's a totally new API.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
/// m_ConstantInt() - Match an arbitrary ConstantInt and ignore it.<br>
inline class_match<ConstantInt> m_ConstantInt() {<br>
return class_match<ConstantInt>();<br>
@@ -299,6 +311,12 @@ struct bind_ty {<br>
/// m_Value - Match a value, capturing it if we match.<br>
inline bind_ty<Value> m_Value(Value *&V) { return V; }<br>
<br>
+/// m_Instruction - Match a instruction, capturing it if we match.<br>
+inline bind_ty<Instruction> m_Instruction(Instruction *&I) { return I; }<br>
+<br>
+/// m_BinOp - Match a instruction, capturing it if we match.<br>
+inline bind_ty<BinaryOperator> m_BinOp(BinaryOperator *&I) { return I; }<br>
+<br>
/// m_ConstantInt - Match a ConstantInt, capturing the value if we match.<br>
inline bind_ty<ConstantInt> m_ConstantInt(ConstantInt *&CI) { return CI; }<br>
<br>
@@ -390,6 +408,30 @@ inline specific_intval m_SpecificInt(uin<br>
inline bind_const_intval_ty m_ConstantInt(uint64_t &V) { return V; }<br>
<br>
//===----------------------------------------------------------------------===//<br>
+// Matcher for any binary operator.<br>
+//<br>
+template<typename LHS_t, typename RHS_t><br>
+struct AnyBinaryOp_match {<br>
+ LHS_t L;<br>
+ RHS_t R;<br>
+<br>
+ AnyBinaryOp_match(const LHS_t &LHS, const RHS_t &RHS) : L(LHS), R(RHS) {}<br>
+<br>
+ template<typename OpTy><br>
+ bool match(OpTy *V) {<br>
+ if (auto *I = dyn_cast<BinaryOperator>(V))<br>
+ return L.match(I->getOperand(0)) && R.match(I->getOperand(1));<br>
+ return false;<br>
+ }<br>
+};<br></blockquote><div><br></div><div>Having completely separate code for matching any binary operators and specific binary operators really doesn't make sense to me. Why not combine them?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+template<typename LHS, typename RHS><br>
+inline AnyBinaryOp_match<LHS, RHS><br>
+m_BinOp(const LHS &L, const RHS &R) {<br>
+ return AnyBinaryOp_match<LHS, RHS>(L, R);<br>
+}<br>
+<br>
+//===----------------------------------------------------------------------===//<br>
// Matchers for specific binary operators.<br>
//<br>
<br>
@@ -701,17 +743,21 @@ struct CmpClass_match {<br>
};<br>
<br>
template<typename LHS, typename RHS><br>
+inline CmpClass_match<LHS, RHS, CmpInst, CmpInst::Predicate><br>
+m_Cmp(CmpInst::Predicate &Pred, const LHS &L, const RHS &R) {<br>
+ return CmpClass_match<LHS, RHS, CmpInst, CmpInst::Predicate>(Pred, L, R);<br>
+}<br>
+<br>
+template<typename LHS, typename RHS><br>
inline CmpClass_match<LHS, RHS, ICmpInst, ICmpInst::Predicate><br>
m_ICmp(ICmpInst::Predicate &Pred, const LHS &L, const RHS &R) {<br>
- return CmpClass_match<LHS, RHS,<br>
- ICmpInst, ICmpInst::Predicate>(Pred, L, R);<br>
+ return CmpClass_match<LHS, RHS, ICmpInst, ICmpInst::Predicate>(Pred, L, R);<br>
}<br>
<br>
template<typename LHS, typename RHS><br>
inline CmpClass_match<LHS, RHS, FCmpInst, FCmpInst::Predicate><br>
m_FCmp(FCmpInst::Predicate &Pred, const LHS &L, const RHS &R) {<br>
- return CmpClass_match<LHS, RHS,<br>
- FCmpInst, FCmpInst::Predicate>(Pred, L, R);<br>
+ return CmpClass_match<LHS, RHS, FCmpInst, FCmpInst::Predicate>(Pred, L, R);<br>
}<br>
<br>
//===----------------------------------------------------------------------===//<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>