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

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 24 09:10:01 PDT 2017


You're right. i'll fix before I commit.

~Craig

On Sat, Jun 24, 2017 at 3:02 AM, Dirk Steinke <
steinke.dirk.ml at googlemail.com> wrote:

> 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170624/a32ad8fb/attachment.html>


More information about the llvm-commits mailing list