[llvm] 8aeb2fe - [InstCombine] Negator: -(X << C) --> X * (-1 << C)
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 5 04:16:09 PDT 2020
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
More information about the llvm-commits
mailing list