[PatternMatch] Stabilize the matching order of commutative matchers
Summary:
Currently, we
1. match `LHS` matcher to the `first` operand of binary operator,
2. and then match `RHS` matcher to the `second` operand of binary operator.
If that does not match, we swap the `LHS` and `RHS` matchers:
1. match `RHS` matcher to the `first` operand of binary operator,
2. and then match `LHS` matcher to the `second` operand of binary operator.
This works ok.
But it complicates writing of commutative matchers, where one would like to match
(`m_Value()`) the value on one side, and use (`m_Specific()`) it on the other side.
This is additionally complicated by the fact that `m_Specific()` stores the `Value *`,
not `Value **`, so it won't work at all out of the box.
The last problem is trivially solved by adding a new `m_c_Specific()` that stores the
`Value **`, not `Value *`. I'm choosing to add a new matcher, not change the existing
one because i guess all the current users are ok with existing behavior,
and this additional pointer indirection may have performance drawbacks.
Also, i'm storing pointer, not reference, because for some mysterious-to-me reason
it did not work with the reference.
The first one appears trivial, too.
Currently, we
1. match `LHS` matcher to the `first` operand of binary operator,
2. and then match `RHS` matcher to the `second` operand of binary operator.
If that does not match, we swap the ~~`LHS` and `RHS` matchers~~ **operands**:
1. match ~~`RHS`~~ **`LHS`** matcher to the ~~`first`~~ **`second`** operand of binary operator,
2. and then match ~~`LHS`~~ **`RHS`** matcher to the ~~`second`~ **`first`** operand of binary operator.
Surprisingly, `$ ninja check-llvm` still passes with this.
But i expect the bots will disagree..
The motivational unittest is included.
I'd like to use this in D45664.
Reviewers: spatel, craig.topper, arsenm, RKSimon
Reviewed By: craig.topper
Subscribers: xbolva00, wdng, llvm-commits
Differential Revision: https://reviews.llvm.org/D45828
llvm-svn: 331085
diff --git a/llvm/unittests/IR/PatternMatch.cpp b/llvm/unittests/IR/PatternMatch.cpp
index 6bdcf4d..2c6da22 100644
--- a/llvm/unittests/IR/PatternMatch.cpp
+++ b/llvm/unittests/IR/PatternMatch.cpp
@@ -65,6 +65,56 @@
EXPECT_FALSE(m_OneUse(m_Value()).match(Leaf));
}
+TEST_F(PatternMatchTest, CommutativeDeferredValue) {
+ Value *X = IRB.getInt32(1);
+ Value *Y = IRB.getInt32(2);
+
+ {
+ Value *tX = X;
+ EXPECT_TRUE(match(X, m_Deferred(tX)));
+ EXPECT_FALSE(match(Y, m_Deferred(tX)));
+ }
+ {
+ const Value *tX = X;
+ EXPECT_TRUE(match(X, m_Deferred(tX)));
+ EXPECT_FALSE(match(Y, m_Deferred(tX)));
+ }
+ {
+ Value *const tX = X;
+ EXPECT_TRUE(match(X, m_Deferred(tX)));
+ EXPECT_FALSE(match(Y, m_Deferred(tX)));
+ }
+ {
+ const Value *const tX = X;
+ EXPECT_TRUE(match(X, m_Deferred(tX)));
+ EXPECT_FALSE(match(Y, m_Deferred(tX)));
+ }
+
+ {
+ Value *tX = nullptr;
+ EXPECT_TRUE(match(IRB.CreateAnd(X, X), m_And(m_Value(tX), m_Deferred(tX))));
+ EXPECT_EQ(tX, X);
+ }
+ {
+ Value *tX = nullptr;
+ EXPECT_FALSE(
+ match(IRB.CreateAnd(X, Y), m_c_And(m_Value(tX), m_Deferred(tX))));
+ }
+
+ auto checkMatch = [X, Y](Value *Pattern) {
+ Value *tX = nullptr, *tY = nullptr;
+ EXPECT_TRUE(match(
+ Pattern, m_c_And(m_Value(tX), m_c_And(m_Deferred(tX), m_Value(tY)))));
+ EXPECT_EQ(tX, X);
+ EXPECT_EQ(tY, Y);
+ };
+
+ checkMatch(IRB.CreateAnd(X, IRB.CreateAnd(X, Y)));
+ checkMatch(IRB.CreateAnd(X, IRB.CreateAnd(Y, X)));
+ checkMatch(IRB.CreateAnd(IRB.CreateAnd(X, Y), X));
+ checkMatch(IRB.CreateAnd(IRB.CreateAnd(Y, X), X));
+}
+
TEST_F(PatternMatchTest, FloatingPointOrderedMin) {
Type *FltTy = IRB.getFloatTy();
Value *L = ConstantFP::get(FltTy, 1.0);