[llvm] 70472f3 - [Reassociate] Convert `add`-like `or`'s into an `add`'s to allow reassociation

[Nikita Popov via llvm-commits]

On Tue, Nov 3, 2020 at 8:31 PM Roman Lebedev via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> Author: Roman Lebedev
> Date: 2020-11-03T22:30:51+03:00
> New Revision: 70472f34b289d663a9f70740dfc9ae32fc89e077
>
> URL:
> https://github.com/llvm/llvm-project/commit/70472f34b289d663a9f70740dfc9ae32fc89e077
> DIFF:
> https://github.com/llvm/llvm-project/commit/70472f34b289d663a9f70740dfc9ae32fc89e077.diff
>
> LOG: [Reassociate] Convert `add`-like `or`'s into an `add`'s to allow
> reassociation
>
> InstCombine is quite aggressive in doing the opposite transform,
> folding `add` of operands with no common bits set into an `or`,
> and that not many things support that new pattern..
>
> In this case, teaching Reassociate about it is easy,
> there's preexisting art for `sub`/`shl`:
> just convert such an `or` into an `add`:
> https://rise4fun.com/Alive/Xlyv
>
> Added:
>
>
> Modified:
>     llvm/lib/Transforms/Scalar/Reassociate.cpp
>     llvm/test/Transforms/Reassociate/add-like-or.ll
>
> Removed:
>
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp
> b/llvm/lib/Transforms/Scalar/Reassociate.cpp
> index ba7f367267fe..164971c916d4 100644
> --- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
> +++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
> @@ -920,6 +920,24 @@ static Value *NegateValue(Value *V, Instruction *BI,
>    return NewNeg;
>  }
>
> +/// If we have (X|Y), and iff X and Y have no common bits set,
> +/// transform this into (X+Y) to allow arithmetics reassociation.
> +static BinaryOperator *ConvertOrWithNoCommonBitsToAdd(Instruction *Or) {
> +  // Convert an or into an add.
> +  BinaryOperator *New =
> +      CreateAdd(Or->getOperand(0), Or->getOperand(1), "", Or, Or);
> +  New->setHasNoSignedWrap();
> +  New->setHasNoUnsignedWrap();
> +  New->takeName(Or);
> +
> +  // Everyone now refers to the add instruction.
> +  Or->replaceAllUsesWith(New);
> +  New->setDebugLoc(Or->getDebugLoc());
> +
> +  LLVM_DEBUG(dbgs() << "Converted or into an add: " << *New << '\n');
> +  return New;
> +}
> +
>  /// Return true if we should break up this subtract of X-Y into (X + -Y).
>  static bool ShouldBreakUpSubtract(Instruction *Sub) {
>    // If this is a negation, we can't split it up!
> @@ -2116,6 +2134,18 @@ void ReassociatePass::OptimizeInst(Instruction *I) {
>    if (I->getType()->isIntegerTy(1))
>      return;
>
> +  // If this is a bitwise or instruction of operands
> +  // with no common bits set, convert it to X+Y.
> +  if (I->getOpcode() == Instruction::Or &&
> +      haveNoCommonBitsSet(I->getOperand(0), I->getOperand(1),
> +                          I->getModule()->getDataLayout(),
> /*AC=*/nullptr, I,
> +                          /*DT=*/nullptr)) {
> +    Instruction *NI = ConvertOrWithNoCommonBitsToAdd(I);
> +    RedoInsts.insert(I);
> +    MadeChange = true;
> +    I = NI;
> +  }
> +
>    // If this is a subtract instruction which is not already in negate
> form,
>    // see if we can convert it to X+-Y.
>    if (I->getOpcode() == Instruction::Sub) {
>
> diff  --git a/llvm/test/Transforms/Reassociate/add-like-or.ll
> b/llvm/test/Transforms/Reassociate/add-like-or.ll
> index 25f2c0563da9..13477ad5a491 100644
> --- a/llvm/test/Transforms/Reassociate/add-like-or.ll
> +++ b/llvm/test/Transforms/Reassociate/add-like-or.ll
> @@ -17,7 +17,7 @@ define i32 @test1(i32 %a, i32 %b) {
>  define i32 @test2(i32 %x) {
>  ; CHECK-LABEL: @test2(
>  ; CHECK-NEXT:    [[X_NUMLZ:%.*]] = tail call i32 @llvm.ctlz.i32(i32
> [[X:%.*]], i1 true), [[RNG0:!range !.*]]
> -; CHECK-NEXT:    [[RES:%.*]] = or i32 [[X_NUMLZ]], -32
> +; CHECK-NEXT:    [[RES:%.*]] = add nuw nsw i32 [[X_NUMLZ]], -32
>  ; CHECK-NEXT:    ret i32 [[RES]]
>  ;
>    %x.numlz = tail call i32 @llvm.ctlz.i32(i32 %x, i1 true), !range !0
> @@ -29,9 +29,8 @@ define i32 @test2(i32 %x) {
>  define i32 @test3(i32 %x, i32 %bit) {
>  ; CHECK-LABEL: @test3(
>  ; CHECK-NEXT:    [[X_NUMLZ:%.*]] = tail call i32 @llvm.ctlz.i32(i32
> [[X:%.*]], i1 true), [[RNG0]]
> -; CHECK-NEXT:    [[ZERO_MINUS_X_NUMACTIVEBITS:%.*]] = or i32 [[X_NUMLZ]],
> -32
> -; CHECK-NEXT:    [[BIT_PLUS_ONE:%.*]] = add i32 [[BIT:%.*]], 1
> -; CHECK-NEXT:    [[RES:%.*]] = add i32 [[BIT_PLUS_ONE]],
> [[ZERO_MINUS_X_NUMACTIVEBITS]]
> +; CHECK-NEXT:    [[BIT_PLUS_ONE:%.*]] = add i32 [[BIT:%.*]], -31
> +; CHECK-NEXT:    [[RES:%.*]] = add i32 [[BIT_PLUS_ONE]], [[X_NUMLZ]]
>  ; CHECK-NEXT:    ret i32 [[RES]]
>  ;
>    %x.numlz = tail call i32 @llvm.ctlz.i32(i32 %x, i1 true), !range !0
>

Not familiar with this transform, so possibly stupid question: The way I'm
reading this change, you are unconditionally reverting the InstCombine
canonicalization. The other transform you mention (shl -> mul) first checks
whether doing this will result in reassociation opportunities. I'm
wondering if the same shouldn't be done here as well?

Regards,
Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201103/f209063e/attachment.html>