[llvm] r211881 - Added instruction combine to transform few more negative values addition to subtraction (Part 3)

Dinesh Dwivedi dinesh.d at samsung.com
Tue Jul 8 00:45:16 PDT 2014


Hi Numo,

I agree and that's why I changed code submitted in Part 2 to accommodate this. The other part, which 
checks if C is odd and does transform actually take cares of cases where 1 is absorbed in XOR.

I have found few cases while playing with Z3 to prove my transformations where 1 is getting absorbed 
in XOR and has added code to check for them too. Comments might be confusing which I will revisit and 
update once I return back from my travel.

Regards
Dinesh Dwivedi

------- Original Message -------
Sender : Nuno Lopes<nuno.lopes at ist.utl.pt> 
Date   : Jul 01, 2014 14:51 (GMT+05:30)
Title  : Re: [llvm] r211881 - Added instruction combine to transform few more
 negative values addition to subtraction (Part 3)

On Friday 27 June 2014 07:47:35 Dinesh Dwivedi wrote:
> Author: dinesh
> Date: Fri Jun 27 02:47:35 2014
> New Revision: 211881
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=211881&view=rev
> Log:
> Added instruction combine to transform few more negative values addition to
> subtraction (Part 3) This patch enables transforms for
> 
> (x + (~(y | c) + 1) --> x - (y | c) if c is odd

c doesn't need to be odd for this transformation to be correct. I've proved it 
correct for c even as well.

Nuno


> Differential Revision: http://reviews.llvm.org/D4210
> 
> 
> Modified:
>     llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>     llvm/trunk/test/Transforms/InstCombine/add2.ll
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/I
> nstCombineAddSub.cpp?rev=211881&r1=211880&r2=211881&view=diff
> ===========================================================================
> === --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> (original) +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> Fri Jun 27 02:47:35 2014 @@ -954,56 +954,70 @@ bool
> InstCombiner::WillNotOverflowUnsign
>      return true;
> 
>    return false;
> - }
> +}
> 
>  // Checks if any operand is negative and we can convert add to sub.
> - // This function checks for following negative patterns
> - //   ADD(XOR(OR(Z, NOT(C)), C)), 1) == NEG(AND(Z, C))
> - //   ADD(XOR(AND(Z, C), C), 1) == NEG(OR(Z, ~C)) if C is odd
> - //   TODO: XOR(AND(Z, C), (C + 1)) == NEG(OR(Z, ~C)) if C is even
> - Value *checkForNegativeOperand(BinaryOperator &I,
> -                                InstCombiner::BuilderTy *Builder) {
> -   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
> +// This function checks for following negative patterns
> +//   ADD(XOR(OR(Z, NOT(C)), C)), 1) == NEG(AND(Z, C))
> +//   ADD(XOR(AND(Z, C), C), 1) == NEG(OR(Z, ~C))
> +//   XOR(AND(Z, C), (C + 1)) == NEG(OR(Z, ~C)) if C is even
> +Value *checkForNegativeOperand(BinaryOperator &I,
> +                               InstCombiner::BuilderTy *Builder) {
> +  Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
> 
> -   // This function creates 2 instructions to replace ADD, we need at least
> one -   // of LHS or RHS to have one use to ensure benefit in transform. - 
>  if (!LHS->hasOneUse() && !RHS->hasOneUse())
> -     return nullptr;
> -
> -   Value *X = nullptr, *Y = nullptr, *Z = nullptr;
> -   const APInt *C1 = nullptr, *C2 = nullptr;
> -
> -   // if ONE is on other side, swap
> -   if (match(RHS, m_Add(m_Value(X), m_One())))
> -     std::swap(LHS, RHS);
> -
> -   if (match(LHS, m_Add(m_Value(X), m_One()))) {
> -     // if XOR on other side, swap
> -     if (match(RHS, m_Xor(m_Value(Y), m_APInt(C1))))
> -       std::swap(X, RHS);
> -
> -     if (match(X, m_Xor(m_Value(Y), m_APInt(C1)))) {
> -       // X = XOR(Y, C1), Y = OR(Z, C2), C2 = NOT(C1) ==> X == NOT(AND(Z,
> C1)) -       // ADD(ADD(X, 1), RHS) == ADD(X, ADD(RHS, 1)) == SUB(RHS,
> AND(Z, C1)) -       if (match(Y, m_Or(m_Value(Z), m_APInt(C2))) && (*C2 ==
> ~(*C1))) { -         Value *NewAnd = Builder->CreateAnd(Z, *C1);
> -         return Builder->CreateSub(RHS, NewAnd, "sub");
> -       } else if (C1->countTrailingZeros() == 0) {
> -         // if C1 is ODD and
> -         // X = XOR(Y, C1), Y = AND(Z, C2), C2 == C1 ==> X == NOT(OR(Z,
> ~C1)) -         // ADD(ADD(X, 1), RHS) == ADD(X, ADD(RHS, 1)) == SUB(RHS,
> OR(Z, ~C1)) -         if (match(Y, m_And(m_Value(Z), m_APInt(C2))) && (*C1
> == *C2)) { -           Value *NewOr = Builder->CreateOr(Z, ~(*C1));
> -           return Builder->CreateSub(RHS, NewOr, "sub");
> -         }
> -       }
> -     }
> -   }
> +  // This function creates 2 instructions to replace ADD, we need at least
> one +  // of LHS or RHS to have one use to ensure benefit in transform. + 
> if (!LHS->hasOneUse() && !RHS->hasOneUse())
> +    return nullptr;
> +
> +  Value *X = nullptr, *Y = nullptr, *Z = nullptr;
> +  const APInt *C1 = nullptr, *C2 = nullptr;
> +
> +  // if ONE is on other side, swap
> +  if (match(RHS, m_Add(m_Value(X), m_One())))
> +    std::swap(LHS, RHS);
> +
> +  if (match(LHS, m_Add(m_Value(X), m_One()))) {
> +    // if XOR on other side, swap
> +    if (match(RHS, m_Xor(m_Value(Y), m_APInt(C1))))
> +      std::swap(X, RHS);
> +
> +    if (match(X, m_Xor(m_Value(Y), m_APInt(C1)))) {
> +      // X = XOR(Y, C1), Y = OR(Z, C2), C2 = NOT(C1) ==> X == NOT(AND(Z,
> C1)) +      // ADD(ADD(X, 1), RHS) == ADD(X, ADD(RHS, 1)) == SUB(RHS,
> AND(Z, C1)) +      if (match(Y, m_Or(m_Value(Z), m_APInt(C2))) && (*C2 ==
> ~(*C1))) { +        Value *NewAnd = Builder->CreateAnd(Z, *C1);
> +        return Builder->CreateSub(RHS, NewAnd, "sub");
> +      } else if (match(Y, m_And(m_Value(Z), m_APInt(C2))) && (*C1 == *C2))
> { +        // X = XOR(Y, C1), Y = AND(Z, C2), C2 == C1 ==> X == NOT(OR(Z,
> ~C1)) +        // ADD(ADD(X, 1), RHS) == ADD(X, ADD(RHS, 1)) == SUB(RHS,
> OR(Z, ~C1)) +        Value *NewOr = Builder->CreateOr(Z, ~(*C1));
> +        return Builder->CreateSub(RHS, NewOr, "sub");
> +      }
> +    }
> +  }
> 
> -   return nullptr;
> - }
> +  // Restore LHS and RHS
> +  LHS = I.getOperand(0);
> +  RHS = I.getOperand(1);
> +
> +  // if XOR is on other side, swap
> +  if (match(RHS, m_Xor(m_Value(Y), m_APInt(C1))))
> +    std::swap(LHS, RHS);
> +
> +  // C2 is ODD
> +  // LHS = XOR(Y, C1), Y = AND(Z, C2), C1 == (C2 + 1) => LHS == NEG(OR(Z,
> ~C2)) +  // ADD(LHS, RHS) == SUB(RHS, OR(Z, ~C2))
> +  if (match(LHS, m_Xor(m_Value(Y), m_APInt(C1))))
> +    if (C1->countTrailingZeros() == 0)
> +      if (match(Y, m_And(m_Value(Z), m_APInt(C2))) && *C1 == (*C2 + 1)) {
> +        Value *NewOr = Builder->CreateOr(Z, ~(*C2));
> +        return Builder->CreateSub(RHS, NewOr, "sub");
> +      }
> +  return nullptr;
> +}
> 
> - Instruction *InstCombiner::visitAdd(BinaryOperator &I) {
> +Instruction *InstCombiner::visitAdd(BinaryOperator &I) {
>     bool Changed = SimplifyAssociativeOrCommutative(I);
>     Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
> 
> @@ -1579,7 +1593,7 @@ Instruction *InstCombiner::visitSub(Bina
>          match(Op1, m_Trunc(m_PtrToInt(m_Value(RHSOp)))))
>        if (Value *Res = OptimizePointerDifference(LHSOp, RHSOp,
> I.getType())) return ReplaceInstUsesWith(I, Res);
> -  }
> +      }
> 
>    return nullptr;
>  }
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/add2.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/
> add2.ll?rev=211881&r1=211880&r2=211881&view=diff
> ===========================================================================
> === --- llvm/trunk/test/Transforms/InstCombine/add2.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/add2.ll Fri Jun 27 02:47:35 2014
> @@ -87,17 +87,22 @@ define i16 @test9(i16 %a) {
>  ; CHECK-NEXT:  ret i16 %d
>  }
> 
> -define i32 @test10(i32 %x) {
> -  %x.not = or i32 %x, -1431655766
> -  %neg = xor i32 %x.not, 1431655765
> -  %add = add i32 %x, 1
> +; y + (~((x >> 3) & 0x55555555) + 1) -> y - ((x >> 3) & 0x55555555)
> +define i32 @test10(i32 %x, i32 %y) {
> +  %shr = ashr i32 %x, 3
> +  %shr.not = or i32 %shr, -1431655766
> +  %neg = xor i32 %shr.not, 1431655765
> +  %add = add i32 %y, 1
>    %add1 = add i32 %add, %neg
>    ret i32 %add1
>  ; CHECK-LABEL: @test10(
> -; CHECK-NEXT: [[AND:%[a-z0-9]+]] = and i32 %x, -1431655766
> -; CHECK-NEXT: ret i32 [[AND]]
> +; CHECK-NEXT: [[SHR:%[a-z0-9]+]] = ashr i32 %x, 3
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = and i32 [[SHR]], 1431655765
> +; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
> +; CHECK-NEXT: ret i32 [[SUB]]
>  }
> 
> +; y + (~(x & 0x55555555) + 1) -> y - (x & 0x55555555)
>  define i32 @test11(i32 %x, i32 %y) {
>    %x.not = or i32 %x, -1431655766
>    %neg = xor i32 %x.not, 1431655765
> @@ -110,20 +115,20 @@ define i32 @test11(i32 %x, i32 %y) {
>  ; CHECK-NEXT: ret i32 [[SUB]]
>  }
> 
> +; (y + 1) + ~(x & 0x55555555) -> y - (x & 0x55555555)
>  define i32 @test12(i32 %x, i32 %y) {
> -  %shr = ashr i32 %x, 3
> -  %shr.not = or i32 %shr, -1431655766
> -  %neg = xor i32 %shr.not, 1431655765
> -  %add = add i32 %y, 1
> -  %add1 = add i32 %add, %neg
> +  %add = add nsw i32 %y, 1
> +  %x.not = or i32 %x, -1431655766
> +  %neg = xor i32 %x.not, 1431655765
> +  %add1 = add nsw i32 %add, %neg
>    ret i32 %add1
>  ; CHECK-LABEL: @test12(
> -; CHECK-NEXT: [[SHR:%[a-z0-9]+]] = ashr i32 %x, 3
> -; CHECK-NEXT: [[AND:%[a-z0-9]+]] = and i32 [[SHR]], 1431655765
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = and i32 %x, 1431655765
>  ; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
>  ; CHECK-NEXT: ret i32 [[SUB]]
>  }
> 
> +; y + (~(x & 0x55555556) + 1) -> y - (x & 0x55555556)
>  define i32 @test13(i32 %x, i32 %y) {
>    %x.not = or i32 %x, -1431655767
>    %neg = xor i32 %x.not, 1431655766
> @@ -136,43 +141,67 @@ define i32 @test13(i32 %x, i32 %y) {
>  ; CHECK-NEXT: ret i32 [[SUB]]
>  }
> 
> +; (y + 1) + ~(x & 0x55555556) -> y - (x & 0x55555556)
>  define i32 @test14(i32 %x, i32 %y) {
> -  %shr = ashr i32 %x, 3
> -  %shr.not = or i32 %shr, -1431655767
> -  %neg = xor i32 %shr.not, 1431655766
> -  %add = add i32 %y, 1
> -  %add1 = add i32 %add, %neg
> +  %add = add nsw i32 %y, 1
> +  %x.not = or i32 %x, -1431655767
> +  %neg = xor i32 %x.not, 1431655766
> +  %add1 = add nsw i32 %add, %neg
>    ret i32 %add1
>  ; CHECK-LABEL: @test14(
> -; CHECK-NEXT: [[SHR:%[a-z0-9]+]] = ashr i32 %x, 3
> -; CHECK-NEXT: [[AND:%[a-z0-9]+]] = and i32 [[SHR]], 1431655766
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = and i32 %x, 1431655766
>  ; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
>  ; CHECK-NEXT: ret i32 [[SUB]]
>  }
> 
> +; y + (~(x | 0x55555556) + 1) -> y - (x | 0x55555556)
>  define i32 @test15(i32 %x, i32 %y) {
> - %x.not = and i32 %x, -1431655767
> - %neg = xor i32 %x.not, -1431655767
> - %add = add i32 %y, 1
> - %add1 = add i32 %add, %neg
> - ret i32 %add1
> +  %x.not = and i32 %x, -1431655767
> +  %neg = xor i32 %x.not, -1431655767
> +  %add = add i32 %y, 1
> +  %add1 = add i32 %add, %neg
> +  ret i32 %add1
>  ; CHECK-LABEL: @test15(
> -; CHECK-NEXT: [[OR:%[a-z0-9]+]] = or i32 %x, 1431655766
> -; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[OR]]
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = or i32 %x, 1431655766
> +; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
>  ; CHECK-NEXT: ret i32 [[SUB]]
>  }
> 
> +; (y + 1) + ~(x | 0x55555556) -> y - (x | 0x555555556)
>  define i32 @test16(i32 %x, i32 %y) {
> - %shr = ashr i32 %x, 3
> - %shr.not = and i32 %shr, -1431655767
> - %neg = xor i32 %shr.not, -1431655767
> - %add = add i32 %y, 1
> - %add1 = add i32 %add, %neg
> - ret i32 %add1
> +  %add = add nsw i32 %y, 1
> +  %x.not = and i32 %x, -1431655767
> +  %neg = xor i32 %x.not, -1431655767
> +  %add1 = add nsw i32 %add, %neg
> +  ret i32 %add1
>  ; CHECK-LABEL: @test16(
> -; CHECK-NEXT: [[SHR:%[a-z0-9]+]] = ashr i32 %x, 3
> -; CHECK-NEXT: [[OR:%[a-z0-9]+]] = or i32 [[SHR]], 1431655766
> -; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[OR]]
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = or i32 %x, 1431655766
> +; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
> +; CHECK-NEXT: ret i32 [[SUB]]
> +}
> +
> +; y + (~(x | 0x55555555) + 1) -> y - (x | 0x55555555)
> +define i32 @test17(i32 %x, i32 %y) {
> +  %x.not = and i32 %x, -1431655766
> +  %add2 = xor i32 %x.not, -1431655765
> +  %add1 = add nsw i32 %add2, %y
> +  ret i32 %add1
> +; CHECK-LABEL: @test17(
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = or i32 %x, 1431655765
> +; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
> +; CHECK-NEXT: ret i32 [[SUB]]
> +}
> +
> +; (y + 1) + ~(x | 0x55555555) -> y - (x | 0x55555555)
> +define i32 @test18(i32 %x, i32 %y) {
> +  %add = add nsw i32 %y, 1
> +  %x.not = and i32 %x, -1431655766
> +  %neg = xor i32 %x.not, -1431655766
> +  %add1 = add nsw i32 %add, %neg
> +  ret i32 %add1
> +; CHECK-LABEL: @test18(
> +; CHECK-NEXT: [[AND:%[a-z0-9]+]] = or i32 %x, 1431655765
> +; CHECK-NEXT: [[SUB:%[a-z0-9]+]] = sub i32 %y, [[AND]]
>  ; CHECK-NEXT: ret i32 [[SUB]]
>  }




More information about the llvm-commits mailing list