[PATCH] D45828: [PatternMatch] Stabilize the matching order of commutative matchers

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 19 10:33:55 PDT 2018


lebedev.ri created this revision.
lebedev.ri added reviewers: spatel, craig.topper, arsenm, RKSimon.
Herald added a subscriber: wdng.

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 https://reviews.llvm.org/D45664.


Repository:
  rL LLVM

https://reviews.llvm.org/D45828

Files:
  include/llvm/IR/PatternMatch.h
  unittests/IR/PatternMatch.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45828.143120.patch
Type: text/x-patch
Size: 4889 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180419/4086d97d/attachment.bin>


More information about the llvm-commits mailing list