[PATCH] D34592: [IR] Implement commutable matchers without using combineOr

Dirk Steinke via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 24 03:02:02 PDT 2017


Hi Craig,

I'm seeing multiple instances of code in your patch that looks like this
(e.g. AnyBinaryOp_match, or BinaryOp_match with ConstantExpr)
-      return CE->getOpcode() == Opcode && L.match(CE->getOperand(0)) &&
-             R.match(CE->getOperand(1));
+      return CE->getOpcode() == Opcode &&
+             ((L.match(CE->getOperand(0)) && R.match(CE->getOperand(1))) ||
+              (Commutable && L.match(CE->getOperand(0)) &&
+               R.match(CE->getOperand(1))));
that is, if Commutable is set, it still does the same check as non-permutable.
I don't think that this is what you want it to do.

Dirk

On Sat, 24 Jun 2017 05:03:56 +0000
Craig Topper via Phabricator via llvm-commits <llvm-commits at lists.llvm.org> wrote:

> craig.topper created this revision.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> This shows about an ~7-8k reduction in the opt binary size on my local build.
> 
> 
> https://reviews.llvm.org/D34592
> 
> Files:
>   include/llvm/IR/PatternMatch.h
> 


More information about the llvm-commits mailing list