From 6959b8e76f18f63aacaaf24dd74b11d733b57314 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Fri, 27 Apr 2018 21:23:20 +0000 Subject: [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 --- llvm/unittests/IR/PatternMatch.cpp | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'llvm/unittests/IR/PatternMatch.cpp') diff --git a/llvm/unittests/IR/PatternMatch.cpp b/llvm/unittests/IR/PatternMatch.cpp index 6bdcf4de7e2..2c6da22298f 100644 --- a/llvm/unittests/IR/PatternMatch.cpp +++ b/llvm/unittests/IR/PatternMatch.cpp @@ -65,6 +65,56 @@ TEST_F(PatternMatchTest, OneUse) { 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); -- cgit v1.2.3