<div dir="ltr">You're right. i'll fix before I commit.</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br><div class="gmail_quote">On Sat, Jun 24, 2017 at 3:02 AM, Dirk Steinke <span dir="ltr"><<a href="mailto:steinke.dirk.ml@googlemail.com" target="_blank">steinke.dirk.ml@googlemail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Craig,<br>
<br>
I'm seeing multiple instances of code in your patch that looks like this<br>
(e.g. AnyBinaryOp_match, or BinaryOp_match with ConstantExpr)<br>
-      return CE->getOpcode() == Opcode && L.match(CE->getOperand(0)) &&<br>
-             R.match(CE->getOperand(1));<br>
+      return CE->getOpcode() == Opcode &&<br>
+             ((L.match(CE->getOperand(0)) && R.match(CE->getOperand(1))) ||<br>
+              (Commutable && L.match(CE->getOperand(0)) &&<br>
+               R.match(CE->getOperand(1))));<br>
that is, if Commutable is set, it still does the same check as non-permutable.<br>
I don't think that this is what you want it to do.<br>
<br>
Dirk<br>
<br>
On Sat, 24 Jun 2017 05:03:56 +0000<br>
Craig Topper via Phabricator via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
<br>
> craig.topper created this revision.<br>
><br>
> Turns out creating matchers with combineOr isn't very efficient as we have to build matcher objects for both sides of the OR. Those objects aren't free, the trees usually contain several objects that contain a reference to a Value *, ConstantInt *, APInt * or some such thing. The compiler isn't always willing to inline all the matcher code to get rid of these member variables. Thus we end up loads and stores of these variables.<br>
><br>
> Using combineOR ends up creating two complete copies of the tree and the associated stores. I believe we're also paying for the opcode check twice.<br>
><br>
> This patch adds a commutable mode to several of the matcher objects as a bool template parameter that can be used to enable  commutable support directly in the match functions of the corresponding objects. This avoids the duplicate object creation and the opcode checks.<br>
><br>
> This shows about an ~7-8k reduction in the opt binary size on my local build.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D34592" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D34592</a><br>
><br>
> Files:<br>
>   include/llvm/IR/PatternMatch.h<br>
><br>
</blockquote></div><br></div>