[llvm] 8aeb2fe - [InstCombine] Negator: -(X << C) --> X * (-1 << C)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 5 06:36:33 PDT 2020
Our rules for tie-breaking for canonical form are created on-the-fly /
self-referential...so whatever we put more effort into becomes canonical.
:)
In this case, analysis for shifts is better in general than multiplies, so
we'll need to look out for regressions if we are now creating more
multiplies.
For example, we seem to be missing these icmp folds (but some constants are
handled, so we probably have over-specific code somewhere):
https://rise4fun.com/Alive/sdM9
I can look into that one.
On Wed, Aug 5, 2020 at 7:16 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:
> On Wed, Aug 5, 2020 at 11:46 AM Nikita Popov <nikita.ppv at gmail.com> wrote:
> > This looks a bit dubious to me.
> Please can you be more specific?
> mul isn't obviously much worse than shl.
>
> FWIW i saw this in real IR, i expected it to be already handled
> by Negator, but due to instruction visitation order the mul that i saw
> in the IR got converted into a shl before we've encountered negation.
>
> > Shouldn't we prefer a shift over a multiply?
> Yep, we should, that is why i try to sink the negation first,
> and only if that fails i convert shl -> mul.
>
> > Especially in cases where we don't actually eliminate a sub...
> We didn't increase instruction count and we changed non-commutable
> instruction (sub) into a commutable instruction (add),
> so as far as instcombine's "cost-model" is concerned this is fine.
>
> > Nikita
>
> Roman
>
> > On Wed, Aug 5, 2020 at 2:13 AM Roman Lebedev via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >>
> >>
> >> Author: Roman Lebedev
> >> Date: 2020-08-05T03:13:14+03:00
> >> New Revision: 8aeb2fe13a4100b4c2e78d6ef75119304100cb1f
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/8aeb2fe13a4100b4c2e78d6ef75119304100cb1f
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/8aeb2fe13a4100b4c2e78d6ef75119304100cb1f.diff
> >>
> >> LOG: [InstCombine] Negator: -(X << C) --> X * (-1 << C)
> >>
> >> This shows some regressions in tests, but they are all around GEP's,
> >> so i'm not really sure how important those are.
> >>
> >> https://rise4fun.com/Alive/1Gn
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >> llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
> >> llvm/test/Transforms/InstCombine/icmp.ll
> >> llvm/test/Transforms/InstCombine/sub-gep.ll
> >> llvm/test/Transforms/InstCombine/sub-of-negatible.ll
> >> llvm/test/Transforms/InstCombine/sub.ll
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> ################################################################################
> >> diff --git a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
> b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
> >> index 1c7f00b0edee..b684016b6a29 100644
> >> --- a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
> >> +++ b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
> >> @@ -324,10 +324,16 @@ LLVM_NODISCARD Value *Negator::visitImpl(Value
> *V, unsigned Depth) {
> >> }
> >> case Instruction::Shl: {
> >> // `shl` is negatible if the first operand is negatible.
> >> - Value *NegOp0 = negate(I->getOperand(0), Depth + 1);
> >> - if (!NegOp0) // Early return.
> >> + if (Value *NegOp0 = negate(I->getOperand(0), Depth + 1))
> >> + return Builder.CreateShl(NegOp0, I->getOperand(1), I->getName()
> + ".neg");
> >> + // Otherwise, `shl %x, C` can be interpreted as `mul %x, 1<<C`.
> >> + auto *Op1C = dyn_cast<Constant>(I->getOperand(1));
> >> + if (!Op1C) // Early return.
> >> return nullptr;
> >> - return Builder.CreateShl(NegOp0, I->getOperand(1), I->getName() +
> ".neg");
> >> + return Builder.CreateMul(
> >> + I->getOperand(0),
> >> +
> ConstantExpr::getShl(Constant::getAllOnesValue(Op1C->getType()), Op1C),
> >> + I->getName() + ".neg");
> >> }
> >> case Instruction::Or:
> >> if (!haveNoCommonBitsSet(I->getOperand(0), I->getOperand(1), DL,
> &AC, I,
> >>
> >> diff --git a/llvm/test/Transforms/InstCombine/icmp.ll
> b/llvm/test/Transforms/InstCombine/icmp.ll
> >> index e3050aa1bac2..7addc85c4c8b 100644
> >> --- a/llvm/test/Transforms/InstCombine/icmp.ll
> >> +++ b/llvm/test/Transforms/InstCombine/icmp.ll
> >> @@ -512,7 +512,8 @@ define i1 @test24(i64 %i) {
> >> ; unsigned overflow does not happen during offset computation
> >> define i1 @test24_neg_offs(i32* %p, i64 %offs) {
> >> ; CHECK-LABEL: @test24_neg_offs(
> >> -; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[OFFS:%.*]], -2
> >> +; CHECK-NEXT: [[P1_IDX_NEG:%.*]] = mul i64 [[OFFS:%.*]], -4
> >> +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[P1_IDX_NEG]], 8
> >> ; CHECK-NEXT: ret i1 [[CMP]]
> >> ;
> >> %p1 = getelementptr inbounds i32, i32* %p, i64 %offs
> >>
> >> diff --git a/llvm/test/Transforms/InstCombine/sub-gep.ll
> b/llvm/test/Transforms/InstCombine/sub-gep.ll
> >> index 51eb994bf345..cf9604223f6c 100644
> >> --- a/llvm/test/Transforms/InstCombine/sub-gep.ll
> >> +++ b/llvm/test/Transforms/InstCombine/sub-gep.ll
> >> @@ -58,9 +58,8 @@ define i32 @test_inbounds_nuw_trunc([0 x i32]* %base,
> i64 %idx) {
> >>
> >> define i64 @test_inbounds_nuw_swapped([0 x i32]* %base, i64 %idx) {
> >> ; CHECK-LABEL: @test_inbounds_nuw_swapped(
> >> -; CHECK-NEXT: [[P2_IDX:%.*]] = shl nsw i64 [[IDX:%.*]], 2
> >> -; CHECK-NEXT: [[DIFF_NEG:%.*]] = sub i64 0, [[P2_IDX]]
> >> -; CHECK-NEXT: ret i64 [[DIFF_NEG]]
> >> +; CHECK-NEXT: [[P2_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4
> >> +; CHECK-NEXT: ret i64 [[P2_IDX_NEG]]
> >> ;
> >> %p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64
> 0
> >> %p2 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64
> %idx
> >> @@ -73,8 +72,9 @@ define i64 @test_inbounds_nuw_swapped([0 x i32]*
> %base, i64 %idx) {
> >> ; The sub and shl here could be nuw, but this is harder to handle.
> >> define i64 @test_inbounds_nuw_two_gep([0 x i32]* %base, i64 %idx, i64
> %idx2) {
> >> ; CHECK-LABEL: @test_inbounds_nuw_two_gep(
> >> -; CHECK-NEXT: [[P1_IDX1_NEG:%.*]] = sub i64 [[IDX2:%.*]],
> [[IDX:%.*]]
> >> -; CHECK-NEXT: [[DOTNEG:%.*]] = shl i64 [[P1_IDX1_NEG]], 2
> >> +; CHECK-NEXT: [[P1_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4
> >> +; CHECK-NEXT: [[P2_IDX_NEG_NEG:%.*]] = shl i64 [[IDX2:%.*]], 2
> >> +; CHECK-NEXT: [[DOTNEG:%.*]] = add i64 [[P2_IDX_NEG_NEG]],
> [[P1_IDX_NEG]]
> >> ; CHECK-NEXT: ret i64 [[DOTNEG]]
> >> ;
> >> %p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64
> %idx
> >>
> >> diff --git a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
> b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
> >> index caa6e25ccf69..4a3c56337c22 100644
> >> --- a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
> >> +++ b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
> >> @@ -1076,8 +1076,8 @@ define i8 @negate_left_shift_by_constant(i8 %x,
> i8 %y, i8 %z, i8 %k) {
> >> ; CHECK-LABEL: @negate_left_shift_by_constant(
> >> ; CHECK-NEXT: [[T0:%.*]] = sub i8 [[K:%.*]], [[Z:%.*]]
> >> ; CHECK-NEXT: call void @use8(i8 [[T0]])
> >> -; CHECK-NEXT: [[T1:%.*]] = shl i8 [[T0]], 4
> >> -; CHECK-NEXT: [[T2:%.*]] = sub i8 [[X:%.*]], [[T1]]
> >> +; CHECK-NEXT: [[T1_NEG:%.*]] = mul i8 [[T0]], -16
> >> +; CHECK-NEXT: [[T2:%.*]] = add i8 [[T1_NEG]], [[X:%.*]]
> >> ; CHECK-NEXT: ret i8 [[T2]]
> >> ;
> >> %t0 = sub i8 %k, %z
> >>
> >> diff --git a/llvm/test/Transforms/InstCombine/sub.ll
> b/llvm/test/Transforms/InstCombine/sub.ll
> >> index 3bc8b67649fc..4116a79d66d9 100644
> >> --- a/llvm/test/Transforms/InstCombine/sub.ll
> >> +++ b/llvm/test/Transforms/InstCombine/sub.ll
> >> @@ -506,8 +506,8 @@ define i64 @test24b(i8* %P, i64 %A){
> >>
> >> define i64 @test25(i8* %P, i64 %A){
> >> ; CHECK-LABEL: @test25(
> >> -; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i64 [[A:%.*]], 1
> >> -; CHECK-NEXT: [[DOTNEG:%.*]] = add i64 [[B_IDX]], -84
> >> +; CHECK-NEXT: [[B_IDX_NEG_NEG:%.*]] = shl i64 [[A:%.*]], 1
> >> +; CHECK-NEXT: [[DOTNEG:%.*]] = add i64 [[B_IDX_NEG_NEG]], -84
> >> ; CHECK-NEXT: ret i64 [[DOTNEG]]
> >> ;
> >> %B = getelementptr inbounds [42 x i16], [42 x i16]* @Arr, i64 0, i64
> %A
> >> @@ -521,8 +521,8 @@ define i64 @test25(i8* %P, i64 %A){
> >> define i16 @test25_as1(i8 addrspace(1)* %P, i64 %A) {
> >> ; CHECK-LABEL: @test25_as1(
> >> ; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[A:%.*]] to i16
> >> -; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i16 [[TMP1]], 1
> >> -; CHECK-NEXT: [[DOTNEG:%.*]] = add i16 [[B_IDX]], -84
> >> +; CHECK-NEXT: [[B_IDX_NEG_NEG:%.*]] = shl i16 [[TMP1]], 1
> >> +; CHECK-NEXT: [[DOTNEG:%.*]] = add i16 [[B_IDX_NEG_NEG]], -84
> >> ; CHECK-NEXT: ret i16 [[DOTNEG]]
> >> ;
> >> %B = getelementptr inbounds [42 x i16], [42 x i16] addrspace(1)*
> @Arr_as1, i64 0, i64 %A
> >> @@ -557,9 +557,8 @@ define i64 @test_neg_shl_sub_extra_use1(i64 %a, i64
> %b, i64* %p) {
> >> ; CHECK-LABEL: @test_neg_shl_sub_extra_use1(
> >> ; CHECK-NEXT: [[SUB:%.*]] = sub i64 [[A:%.*]], [[B:%.*]]
> >> ; CHECK-NEXT: store i64 [[SUB]], i64* [[P:%.*]], align 8
> >> -; CHECK-NEXT: [[MUL:%.*]] = shl i64 [[SUB]], 2
> >> -; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[MUL]]
> >> -; CHECK-NEXT: ret i64 [[NEG]]
> >> +; CHECK-NEXT: [[MUL_NEG:%.*]] = mul i64 [[SUB]], -4
> >> +; CHECK-NEXT: ret i64 [[MUL_NEG]]
> >> ;
> >> %sub = sub i64 %a, %b
> >> store i64 %sub, i64* %p
> >> @@ -840,9 +839,10 @@ define i64 @test29(i8* %foo, i64 %i, i64 %j) {
> >>
> >> define i64 @test30(i8* %foo, i64 %i, i64 %j) {
> >> ; CHECK-LABEL: @test30(
> >> -; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i64 [[I:%.*]], 2
> >> -; CHECK-NEXT: [[DOTNEG:%.*]] = sub i64 [[GEP1_IDX]], [[J:%.*]]
> >> -; CHECK-NEXT: ret i64 [[DOTNEG]]
> >> +; CHECK-NEXT: [[GEP1_IDX_NEG:%.*]] = mul i64 [[I:%.*]], -4
> >> +; CHECK-NEXT: [[TMP1:%.*]] = add i64 [[GEP1_IDX_NEG]], [[J:%.*]]
> >> +; CHECK-NEXT: [[DIFF_NEG:%.*]] = sub i64 0, [[TMP1]]
> >> +; CHECK-NEXT: ret i64 [[DIFF_NEG]]
> >> ;
> >> %bit = bitcast i8* %foo to i32*
> >> %gep1 = getelementptr inbounds i32, i32* %bit, i64 %i
> >> @@ -855,9 +855,10 @@ define i64 @test30(i8* %foo, i64 %i, i64 %j) {
> >>
> >> define i16 @test30_as1(i8 addrspace(1)* %foo, i16 %i, i16 %j) {
> >> ; CHECK-LABEL: @test30_as1(
> >> -; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i16 [[I:%.*]], 2
> >> -; CHECK-NEXT: [[DOTNEG:%.*]] = sub i16 [[GEP1_IDX]], [[J:%.*]]
> >> -; CHECK-NEXT: ret i16 [[DOTNEG]]
> >> +; CHECK-NEXT: [[GEP1_IDX_NEG:%.*]] = mul i16 [[I:%.*]], -4
> >> +; CHECK-NEXT: [[TMP1:%.*]] = add i16 [[GEP1_IDX_NEG]], [[J:%.*]]
> >> +; CHECK-NEXT: [[DIFF_NEG:%.*]] = sub i16 0, [[TMP1]]
> >> +; CHECK-NEXT: ret i16 [[DIFF_NEG]]
> >> ;
> >> %bit = bitcast i8 addrspace(1)* %foo to i32 addrspace(1)*
> >> %gep1 = getelementptr inbounds i32, i32 addrspace(1)* %bit, i16 %i
> >> @@ -1310,8 +1311,8 @@ define i64 @test61([100 x [100 x i8]]* %foo, i64
> %i, i64 %j) {
> >>
> >> define i32 @test62(i32 %A) {
> >> ; CHECK-LABEL: @test62(
> >> -; CHECK-NEXT: [[B:%.*]] = shl i32 [[A:%.*]], 1
> >> -; CHECK-NEXT: [[C:%.*]] = sub i32 2, [[B]]
> >> +; CHECK-NEXT: [[B_NEG:%.*]] = mul i32 [[A:%.*]], -2
> >> +; CHECK-NEXT: [[C:%.*]] = add i32 [[B_NEG]], 2
> >> ; CHECK-NEXT: ret i32 [[C]]
> >> ;
> >> %B = sub i32 1, %A
> >> @@ -1321,8 +1322,8 @@ define i32 @test62(i32 %A) {
> >>
> >> define <2 x i32> @test62vec(<2 x i32> %A) {
> >> ; CHECK-LABEL: @test62vec(
> >> -; CHECK-NEXT: [[B:%.*]] = shl <2 x i32> [[A:%.*]], <i32 1, i32 1>
> >> -; CHECK-NEXT: [[C:%.*]] = sub <2 x i32> <i32 2, i32 2>, [[B]]
> >> +; CHECK-NEXT: [[B_NEG:%.*]] = mul <2 x i32> [[A:%.*]], <i32 -2, i32
> -2>
> >> +; CHECK-NEXT: [[C:%.*]] = add <2 x i32> [[B_NEG]], <i32 2, i32 2>
> >> ; CHECK-NEXT: ret <2 x i32> [[C]]
> >> ;
> >> %B = sub <2 x i32> <i32 1, i32 1>, %A
> >> @@ -1332,8 +1333,8 @@ define <2 x i32> @test62vec(<2 x i32> %A) {
> >>
> >> define i32 @test63(i32 %A) {
> >> ; CHECK-LABEL: @test63(
> >> -; CHECK-NEXT: [[B:%.*]] = shl i32 [[A:%.*]], 1
> >> -; CHECK-NEXT: ret i32 [[B]]
> >> +; CHECK-NEXT: [[B_NEG_NEG:%.*]] = shl i32 [[A:%.*]], 1
> >> +; CHECK-NEXT: ret i32 [[B_NEG_NEG]]
> >> ;
> >> %B = sub i32 1, %A
> >> %C = shl i32 %B, 1
> >> @@ -1343,8 +1344,8 @@ define i32 @test63(i32 %A) {
> >>
> >> define <2 x i32> @test63vec(<2 x i32> %A) {
> >> ; CHECK-LABEL: @test63vec(
> >> -; CHECK-NEXT: [[B:%.*]] = shl <2 x i32> [[A:%.*]], <i32 1, i32 1>
> >> -; CHECK-NEXT: ret <2 x i32> [[B]]
> >> +; CHECK-NEXT: [[B_NEG_NEG:%.*]] = shl <2 x i32> [[A:%.*]], <i32 1,
> i32 1>
> >> +; CHECK-NEXT: ret <2 x i32> [[B_NEG_NEG]]
> >> ;
> >> %B = sub <2 x i32> <i32 1, i32 1>, %A
> >> %C = shl <2 x i32> %B, <i32 1, i32 1>
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200805/ef11b26f/attachment.html>
More information about the llvm-commits
mailing list