[PATCH] [Reassociate] Similar to "X + -X" -> "0", added code to handle "X + ~X" -> "-1".

[Rahul Jain]

Hi David,

clang-format-diff.py it is.


Sample usage as mentioned in the file:

git diff -U0 HEAD^ | clang-format-diff.py -i -p1

svn diff --diff-cmd=diff -x-U0 | clang-format-diff.py -i

Thanks for pointing out! Will be helpful.

Rahul


On Sat, May 31, 2014 at 9:13 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Sat, May 31, 2014 at 8:02 AM, Rahul Jain <rahul1.jain at samsung.com>
> wrote:
> >
> > Hi Benjamin,
> >
> > Thanks for the review.
> > No I dont have commit access as of now, would be helpful if
> > you could commit it for me. Will request for one soon!
> >
> > Updated patch removing the whitespace-only changes, those were
> > introduced on running clang-format.
>
> There's a script for running clang-format over a diff (or over a git
> revision range) that should help cleanup your work without introducing
> unrelated changes (I forget the exact name, it's somewhere in the
> clang-format project directory)
>
> > Also patched to an updated revision.
> >
> > Please commit it on my behalf.
> >
> > Thanks again for the help!
> > Rahul
> >
> > http://reviews.llvm.org/D3835
> >
> > Files:
> >   lib/Transforms/Scalar/Reassociate.cpp
> >   test/Transforms/Reassociate/inverses.ll
> >
> > Index: lib/Transforms/Scalar/Reassociate.cpp
> > ===================================================================
> > --- lib/Transforms/Scalar/Reassociate.cpp
> > +++ lib/Transforms/Scalar/Reassociate.cpp
> > @@ -1368,11 +1368,10 @@
> >  Value *Reassociate::OptimizeAdd(Instruction *I,
> >                                  SmallVectorImpl<ValueEntry> &Ops) {
> >    // Scan the operand lists looking for X and -X pairs.  If we find
> any, we
> > -  // can simplify the expression. X+-X == 0.  While we're at it, scan
> for any
> > +  // can simplify expressions like X+-X == 0 and X+~X ==-1.  While
> we're at it,
> > +  // scan for any
> >    // duplicates.  We want to canonicalize Y+Y+Y+Z -> 3*Y+Z.
> > -  //
> > -  // TODO: We could handle "X + ~X" -> "-1" if we wanted, since "-X =
> ~X+1".
> > -  //
> > +
> >    for (unsigned i = 0, e = Ops.size(); i != e; ++i) {
> >      Value *TheOp = Ops[i].Op;
> >      // Check to see if we've seen this operand before.  If so, we
> factor all
> > @@ -1412,19 +1411,28 @@
> >        continue;
> >      }
> >
> > -    // Check for X and -X in the operand list.
> > -    if (!BinaryOperator::isNeg(TheOp))
> > +    // Check for X and -X or X and ~X in the operand list.
> > +    if (!BinaryOperator::isNeg(TheOp) && !BinaryOperator::isNot(TheOp))
> >        continue;
> >
> > -    Value *X = BinaryOperator::getNegArgument(TheOp);
> > +    Value *X = nullptr;
> > +    if (BinaryOperator::isNeg(TheOp))
> > +      X = BinaryOperator::getNegArgument(TheOp);
> > +    else if (BinaryOperator::isNot(TheOp))
> > +      X = BinaryOperator::getNotArgument(TheOp);
> > +
> >      unsigned FoundX = FindInOperandList(Ops, i, X);
> >      if (FoundX == i)
> >        continue;
> >
> >      // Remove X and -X from the operand list.
> > -    if (Ops.size() == 2)
> > +    if (Ops.size() == 2 && BinaryOperator::isNeg(TheOp))
> >        return Constant::getNullValue(X->getType());
> >
> > +    // Remove X and ~X from the operand list.
> > +    if (Ops.size() == 2 && BinaryOperator::isNot(TheOp))
> > +      return Constant::getAllOnesValue(X->getType());
> > +
> >      Ops.erase(Ops.begin()+i);
> >      if (i < FoundX)
> >        --FoundX;
> > @@ -1434,6 +1442,13 @@
> >      ++NumAnnihil;
> >      --i;     // Revisit element.
> >      e -= 2;  // Removed two elements.
> > +
> > +    // if X and ~X we append -1 to the operand list.
> > +    if (BinaryOperator::isNot(TheOp)) {
> > +      Value *V = Constant::getAllOnesValue(X->getType());
> > +      Ops.insert(Ops.end(), ValueEntry(getRank(V), V));
> > +      e += 1;
> > +    }
> >    }
> >
> >    // Scan the operand list, checking to see if there are any common
> factors
> > Index: test/Transforms/Reassociate/inverses.ll
> > ===================================================================
> > --- test/Transforms/Reassociate/inverses.ll
> > +++ test/Transforms/Reassociate/inverses.ll
> > @@ -32,3 +32,15 @@
> >  ; CHECK: %tmp.5 = add i32 %b, 1234
> >  ; CHECK: ret i32 %tmp.5
> >  }
> > +
> > +define i32 @test4(i32 %b, i32 %a) {
> > +        %tmp.1 = add i32 %a, 1234
> > +        %tmp.2 = add i32 %b, %tmp.1
> > +        %tmp.4 = xor i32 %a, -1
> > +        ; (b+(a+1234))+~a -> b+1233
> > +        %tmp.5 = add i32 %tmp.2, %tmp.4
> > +        ret i32 %tmp.5
> > +; CHECK-LABEL: @test4(
> > +; CHECK: %tmp.5 = add i32 %b, 1233
> > +; CHECK: ret i32 %tmp.5
> > +}
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> _______________________________________________
> 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/20140531/c49e3584/attachment.html>