[PATCH] Added instruction combine to transform few more negative values addition to subtraction

[Nick Lewycky]

On 19 June 2014 02:56, Dinesh Dwivedi <dinesh.d at samsung.com> wrote:

> Hi Nick,
>
> m_Not checks for pattern XOR(X, -1) and will not work in current scenario.
> i.e. it can check if we have XOR(X, -1) pattern but can not identify that
> 0x55555555 and ~(0xAAAAAAAA) are NOT of each other.
>
> Am I missing something?
>

Sounds like m_Not is missing something. Could I interest you in teaching it
to handle m_Not(ConstantInt) by looking for ~CI?


> Regards
> Dinesh Dwivedi
>
> ------- Original Message -------
> Sender : Nick Lewycky<nicholas at mxc.ca>
> Date   : Jun 19, 2014 11:50 (GMT+05:30)
> Title  : Re: [PATCH] Added instruction combine to transform few more
> negative
>  values addition to subtraction
>
> Nick Lewycky wrote:
> > Sadness. Did you know that gcc gets this even at -O0? Not this pattern
> mind you, but the original testcase. It's that simple, at least before we
> transform to xor(or) form. GCC doesn't get it in xor(or) form either.
> >
> > I looked for a better way to address this optimizer deficiency and
> didn't find one. The one thing I didn't check is whether we could get it by
> reordering our optimizations inside instcombine and do something else
> before it gets into xor(or) form. Even if that did fix the testcase in
> PR14365 it would still leave the case where someone actually does write
> "((a|~c) ^ c) + (a+1)".
>
> Actually, I was just thinking about this some more. There's no reason
> 'C' needs to be a constant at all, is there? As long as it's X and ~X
> (use m_Value(X) then m_Not(m_Specific(X))) the transform should be
> correct, right?
>
> Nick
>
> >
> > ================
> > Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:928
> > @@ +927,3 @@
> > +
> > +  // This function creates 2 instructions to replace ADD, we need
> atleast one of
> > +  // LHS or RHS to have one use to ensure benefit in transform.
> > ----------------
> > "atleast" -->  "at least"
> >
> > ================
> > Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:951
> > @@ +950,3 @@
> > +    if (match(X, m_Xor(m_Value(Y), m_APInt(C1)))) {
> > +      if (match(Y, m_Or(m_Value(Z), m_APInt(C2)))&&  (*C2 == ~(*C1))) {
> > +        Value *NewAnd = Builder->CreateAnd(Z, *C1);
> > ----------------
> > Optional: *C2 == ~(*C1) might be more efficient as !C2->intersects(*C1).
> It isn't, but in theory we could improve intersects to not compute an
> intermediate value (ie., it doesn't need to look at all the bits once it
> finds a pair are set in both).
> >
> > ================
> > Comment at: test/Transforms/InstCombine/add2.ll:142
> > @@ +141,1 @@
> > +}
> >  No newline at end of file
> >
> > ----------------
> > Please make sure there's a newline at the end of the file.
> >
> > http://reviews.llvm.org/D3733
> >
> >
> >
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140619/01407d2c/attachment.html>