[PATCH] D63993: [InstCombine] (Y + ~X) + 1 --> Y - X fold (PR42459)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

Thank you for the review!



================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1207-1210
   // (A + 1) + ~B --> A - B
   // ~B + (A + 1) --> A - B
   if (match(&I, m_c_BinOp(m_Add(m_Value(A), m_One()), m_Not(m_Value(B)))))
     return BinaryOperator::CreateSub(A, B);
----------------
spatel wrote:
> lebedev.ri wrote:
> > spatel wrote:
> > > Here's the sibling matcher. The -reassociation pass tries to create this form (move constant operands up in a chain of reassociative ops).
> > Yep, found it once i understood that the fold *was* being done by instcombine after all (after adding commutative tests), and moved the fold here.
> > Though i don't understand the `-print-after-all` output, `-reassociation` does not run there..
> The descriptive text for -reassociation is "Reassociate expressions", so that might be obscuring your search, but I see the expected diff:
> ```
> *** IR Dump After Simplify the CFG ***
> ; Function Attrs: norecurse nounwind readnone
> define i32 @t0(i32 %x, i32 %y) local_unnamed_addr #0 {
>   %o0 = xor i32 %x, -1
>   %o1 = add i32 %o0, %y
>   %o2 = add i32 %o1, 1
>   ret i32 %o2
> }
> *** IR Dump After Reassociate expressions ***
> ; Function Attrs: norecurse nounwind readnone
> define i32 @t0(i32 %x, i32 %y) local_unnamed_addr #0 {
>   %o0 = xor i32 %x, -1
>   %o1 = add i32 %o0, 1
>   %o2 = add i32 %o1, %y
>   ret i32 %o2
> }
> 
> ```
Oh yeah, thank you!
I was looking for something else, and kind-of did not notice *that* change.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63993/new/

https://reviews.llvm.org/D63993