<div dir="ltr"><div class="gmail_quote"><div class="gmail_attr">This looks a bit dubious to me. Shouldn't we prefer a shift over a multiply? Especially in cases where we don't actually eliminate a sub...</div><div class="gmail_attr"><br></div><div class="gmail_attr">Nikita<br></div><div dir="ltr" class="gmail_attr"><br></div><div dir="ltr" class="gmail_attr">On Wed, Aug 5, 2020 at 2:13 AM Roman Lebedev via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Roman Lebedev<br>
Date: 2020-08-05T03:13:14+03:00<br>
New Revision: 8aeb2fe13a4100b4c2e78d6ef75119304100cb1f<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/8aeb2fe13a4100b4c2e78d6ef75119304100cb1f" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/8aeb2fe13a4100b4c2e78d6ef75119304100cb1f</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/8aeb2fe13a4100b4c2e78d6ef75119304100cb1f.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/8aeb2fe13a4100b4c2e78d6ef75119304100cb1f.diff</a><br>
<br>
LOG: [InstCombine] Negator: -(X << C) --> X * (-1 << C)<br>
<br>
This shows some regressions in tests, but they are all around GEP's,<br>
so i'm not really sure how important those are.<br>
<br>
<a href="https://rise4fun.com/Alive/1Gn" rel="noreferrer" target="_blank">https://rise4fun.com/Alive/1Gn</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp<br>
llvm/test/Transforms/InstCombine/icmp.ll<br>
llvm/test/Transforms/InstCombine/sub-gep.ll<br>
llvm/test/Transforms/InstCombine/sub-of-negatible.ll<br>
llvm/test/Transforms/InstCombine/sub.ll<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp<br>
index 1c7f00b0edee..b684016b6a29 100644<br>
--- a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp<br>
+++ b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp<br>
@@ -324,10 +324,16 @@ LLVM_NODISCARD Value *Negator::visitImpl(Value *V, unsigned Depth) {<br>
}<br>
case Instruction::Shl: {<br>
// `shl` is negatible if the first operand is negatible.<br>
- Value *NegOp0 = negate(I->getOperand(0), Depth + 1);<br>
- if (!NegOp0) // Early return.<br>
+ if (Value *NegOp0 = negate(I->getOperand(0), Depth + 1))<br>
+ return Builder.CreateShl(NegOp0, I->getOperand(1), I->getName() + ".neg");<br>
+ // Otherwise, `shl %x, C` can be interpreted as `mul %x, 1<<C`.<br>
+ auto *Op1C = dyn_cast<Constant>(I->getOperand(1));<br>
+ if (!Op1C) // Early return.<br>
return nullptr;<br>
- return Builder.CreateShl(NegOp0, I->getOperand(1), I->getName() + ".neg");<br>
+ return Builder.CreateMul(<br>
+ I->getOperand(0),<br>
+ ConstantExpr::getShl(Constant::getAllOnesValue(Op1C->getType()), Op1C),<br>
+ I->getName() + ".neg");<br>
}<br>
case Instruction::Or:<br>
if (!haveNoCommonBitsSet(I->getOperand(0), I->getOperand(1), DL, &AC, I,<br>
<br>
diff --git a/llvm/test/Transforms/InstCombine/icmp.ll b/llvm/test/Transforms/InstCombine/icmp.ll<br>
index e3050aa1bac2..7addc85c4c8b 100644<br>
--- a/llvm/test/Transforms/InstCombine/icmp.ll<br>
+++ b/llvm/test/Transforms/InstCombine/icmp.ll<br>
@@ -512,7 +512,8 @@ define i1 @test24(i64 %i) {<br>
; unsigned overflow does not happen during offset computation<br>
define i1 @test24_neg_offs(i32* %p, i64 %offs) {<br>
; CHECK-LABEL: @test24_neg_offs(<br>
-; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[OFFS:%.*]], -2<br>
+; CHECK-NEXT: [[P1_IDX_NEG:%.*]] = mul i64 [[OFFS:%.*]], -4<br>
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[P1_IDX_NEG]], 8<br>
; CHECK-NEXT: ret i1 [[CMP]]<br>
;<br>
%p1 = getelementptr inbounds i32, i32* %p, i64 %offs<br>
<br>
diff --git a/llvm/test/Transforms/InstCombine/sub-gep.ll b/llvm/test/Transforms/InstCombine/sub-gep.ll<br>
index 51eb994bf345..cf9604223f6c 100644<br>
--- a/llvm/test/Transforms/InstCombine/sub-gep.ll<br>
+++ b/llvm/test/Transforms/InstCombine/sub-gep.ll<br>
@@ -58,9 +58,8 @@ define i32 @test_inbounds_nuw_trunc([0 x i32]* %base, i64 %idx) {<br>
<br>
define i64 @test_inbounds_nuw_swapped([0 x i32]* %base, i64 %idx) {<br>
; CHECK-LABEL: @test_inbounds_nuw_swapped(<br>
-; CHECK-NEXT: [[P2_IDX:%.*]] = shl nsw i64 [[IDX:%.*]], 2<br>
-; CHECK-NEXT: [[DIFF_NEG:%.*]] = sub i64 0, [[P2_IDX]]<br>
-; CHECK-NEXT: ret i64 [[DIFF_NEG]]<br>
+; CHECK-NEXT: [[P2_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4<br>
+; CHECK-NEXT: ret i64 [[P2_IDX_NEG]]<br>
;<br>
%p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64 0<br>
%p2 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64 %idx<br>
@@ -73,8 +72,9 @@ define i64 @test_inbounds_nuw_swapped([0 x i32]* %base, i64 %idx) {<br>
; The sub and shl here could be nuw, but this is harder to handle.<br>
define i64 @test_inbounds_nuw_two_gep([0 x i32]* %base, i64 %idx, i64 %idx2) {<br>
; CHECK-LABEL: @test_inbounds_nuw_two_gep(<br>
-; CHECK-NEXT: [[P1_IDX1_NEG:%.*]] = sub i64 [[IDX2:%.*]], [[IDX:%.*]]<br>
-; CHECK-NEXT: [[DOTNEG:%.*]] = shl i64 [[P1_IDX1_NEG]], 2<br>
+; CHECK-NEXT: [[P1_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4<br>
+; CHECK-NEXT: [[P2_IDX_NEG_NEG:%.*]] = shl i64 [[IDX2:%.*]], 2<br>
+; CHECK-NEXT: [[DOTNEG:%.*]] = add i64 [[P2_IDX_NEG_NEG]], [[P1_IDX_NEG]]<br>
; CHECK-NEXT: ret i64 [[DOTNEG]]<br>
;<br>
%p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64 %idx<br>
<br>
diff --git a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll<br>
index caa6e25ccf69..4a3c56337c22 100644<br>
--- a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll<br>
+++ b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll<br>
@@ -1076,8 +1076,8 @@ define i8 @negate_left_shift_by_constant(i8 %x, i8 %y, i8 %z, i8 %k) {<br>
; CHECK-LABEL: @negate_left_shift_by_constant(<br>
; CHECK-NEXT: [[T0:%.*]] = sub i8 [[K:%.*]], [[Z:%.*]]<br>
; CHECK-NEXT: call void @use8(i8 [[T0]])<br>
-; CHECK-NEXT: [[T1:%.*]] = shl i8 [[T0]], 4<br>
-; CHECK-NEXT: [[T2:%.*]] = sub i8 [[X:%.*]], [[T1]]<br>
+; CHECK-NEXT: [[T1_NEG:%.*]] = mul i8 [[T0]], -16<br>
+; CHECK-NEXT: [[T2:%.*]] = add i8 [[T1_NEG]], [[X:%.*]]<br>
; CHECK-NEXT: ret i8 [[T2]]<br>
;<br>
%t0 = sub i8 %k, %z<br>
<br>
diff --git a/llvm/test/Transforms/InstCombine/sub.ll b/llvm/test/Transforms/InstCombine/sub.ll<br>
index 3bc8b67649fc..4116a79d66d9 100644<br>
--- a/llvm/test/Transforms/InstCombine/sub.ll<br>
+++ b/llvm/test/Transforms/InstCombine/sub.ll<br>
@@ -506,8 +506,8 @@ define i64 @test24b(i8* %P, i64 %A){<br>
<br>
define i64 @test25(i8* %P, i64 %A){<br>
; CHECK-LABEL: @test25(<br>
-; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i64 [[A:%.*]], 1<br>
-; CHECK-NEXT: [[DOTNEG:%.*]] = add i64 [[B_IDX]], -84<br>
+; CHECK-NEXT: [[B_IDX_NEG_NEG:%.*]] = shl i64 [[A:%.*]], 1<br>
+; CHECK-NEXT: [[DOTNEG:%.*]] = add i64 [[B_IDX_NEG_NEG]], -84<br>
; CHECK-NEXT: ret i64 [[DOTNEG]]<br>
;<br>
%B = getelementptr inbounds [42 x i16], [42 x i16]* @Arr, i64 0, i64 %A<br>
@@ -521,8 +521,8 @@ define i64 @test25(i8* %P, i64 %A){<br>
define i16 @test25_as1(i8 addrspace(1)* %P, i64 %A) {<br>
; CHECK-LABEL: @test25_as1(<br>
; CHECK-NEXT: [[TMP1:%.*]] = trunc i64 [[A:%.*]] to i16<br>
-; CHECK-NEXT: [[B_IDX:%.*]] = shl nsw i16 [[TMP1]], 1<br>
-; CHECK-NEXT: [[DOTNEG:%.*]] = add i16 [[B_IDX]], -84<br>
+; CHECK-NEXT: [[B_IDX_NEG_NEG:%.*]] = shl i16 [[TMP1]], 1<br>
+; CHECK-NEXT: [[DOTNEG:%.*]] = add i16 [[B_IDX_NEG_NEG]], -84<br>
; CHECK-NEXT: ret i16 [[DOTNEG]]<br>
;<br>
%B = getelementptr inbounds [42 x i16], [42 x i16] addrspace(1)* @Arr_as1, i64 0, i64 %A<br>
@@ -557,9 +557,8 @@ define i64 @test_neg_shl_sub_extra_use1(i64 %a, i64 %b, i64* %p) {<br>
; CHECK-LABEL: @test_neg_shl_sub_extra_use1(<br>
; CHECK-NEXT: [[SUB:%.*]] = sub i64 [[A:%.*]], [[B:%.*]]<br>
; CHECK-NEXT: store i64 [[SUB]], i64* [[P:%.*]], align 8<br>
-; CHECK-NEXT: [[MUL:%.*]] = shl i64 [[SUB]], 2<br>
-; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[MUL]]<br>
-; CHECK-NEXT: ret i64 [[NEG]]<br>
+; CHECK-NEXT: [[MUL_NEG:%.*]] = mul i64 [[SUB]], -4<br>
+; CHECK-NEXT: ret i64 [[MUL_NEG]]<br>
;<br>
%sub = sub i64 %a, %b<br>
store i64 %sub, i64* %p<br>
@@ -840,9 +839,10 @@ define i64 @test29(i8* %foo, i64 %i, i64 %j) {<br>
<br>
define i64 @test30(i8* %foo, i64 %i, i64 %j) {<br>
; CHECK-LABEL: @test30(<br>
-; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i64 [[I:%.*]], 2<br>
-; CHECK-NEXT: [[DOTNEG:%.*]] = sub i64 [[GEP1_IDX]], [[J:%.*]]<br>
-; CHECK-NEXT: ret i64 [[DOTNEG]]<br>
+; CHECK-NEXT: [[GEP1_IDX_NEG:%.*]] = mul i64 [[I:%.*]], -4<br>
+; CHECK-NEXT: [[TMP1:%.*]] = add i64 [[GEP1_IDX_NEG]], [[J:%.*]]<br>
+; CHECK-NEXT: [[DIFF_NEG:%.*]] = sub i64 0, [[TMP1]]<br>
+; CHECK-NEXT: ret i64 [[DIFF_NEG]]<br>
;<br>
%bit = bitcast i8* %foo to i32*<br>
%gep1 = getelementptr inbounds i32, i32* %bit, i64 %i<br>
@@ -855,9 +855,10 @@ define i64 @test30(i8* %foo, i64 %i, i64 %j) {<br>
<br>
define i16 @test30_as1(i8 addrspace(1)* %foo, i16 %i, i16 %j) {<br>
; CHECK-LABEL: @test30_as1(<br>
-; CHECK-NEXT: [[GEP1_IDX:%.*]] = shl nsw i16 [[I:%.*]], 2<br>
-; CHECK-NEXT: [[DOTNEG:%.*]] = sub i16 [[GEP1_IDX]], [[J:%.*]]<br>
-; CHECK-NEXT: ret i16 [[DOTNEG]]<br>
+; CHECK-NEXT: [[GEP1_IDX_NEG:%.*]] = mul i16 [[I:%.*]], -4<br>
+; CHECK-NEXT: [[TMP1:%.*]] = add i16 [[GEP1_IDX_NEG]], [[J:%.*]]<br>
+; CHECK-NEXT: [[DIFF_NEG:%.*]] = sub i16 0, [[TMP1]]<br>
+; CHECK-NEXT: ret i16 [[DIFF_NEG]]<br>
;<br>
%bit = bitcast i8 addrspace(1)* %foo to i32 addrspace(1)*<br>
%gep1 = getelementptr inbounds i32, i32 addrspace(1)* %bit, i16 %i<br>
@@ -1310,8 +1311,8 @@ define i64 @test61([100 x [100 x i8]]* %foo, i64 %i, i64 %j) {<br>
<br>
define i32 @test62(i32 %A) {<br>
; CHECK-LABEL: @test62(<br>
-; CHECK-NEXT: [[B:%.*]] = shl i32 [[A:%.*]], 1<br>
-; CHECK-NEXT: [[C:%.*]] = sub i32 2, [[B]]<br>
+; CHECK-NEXT: [[B_NEG:%.*]] = mul i32 [[A:%.*]], -2<br>
+; CHECK-NEXT: [[C:%.*]] = add i32 [[B_NEG]], 2<br>
; CHECK-NEXT: ret i32 [[C]]<br>
;<br>
%B = sub i32 1, %A<br>
@@ -1321,8 +1322,8 @@ define i32 @test62(i32 %A) {<br>
<br>
define <2 x i32> @test62vec(<2 x i32> %A) {<br>
; CHECK-LABEL: @test62vec(<br>
-; CHECK-NEXT: [[B:%.*]] = shl <2 x i32> [[A:%.*]], <i32 1, i32 1><br>
-; CHECK-NEXT: [[C:%.*]] = sub <2 x i32> <i32 2, i32 2>, [[B]]<br>
+; CHECK-NEXT: [[B_NEG:%.*]] = mul <2 x i32> [[A:%.*]], <i32 -2, i32 -2><br>
+; CHECK-NEXT: [[C:%.*]] = add <2 x i32> [[B_NEG]], <i32 2, i32 2><br>
; CHECK-NEXT: ret <2 x i32> [[C]]<br>
;<br>
%B = sub <2 x i32> <i32 1, i32 1>, %A<br>
@@ -1332,8 +1333,8 @@ define <2 x i32> @test62vec(<2 x i32> %A) {<br>
<br>
define i32 @test63(i32 %A) {<br>
; CHECK-LABEL: @test63(<br>
-; CHECK-NEXT: [[B:%.*]] = shl i32 [[A:%.*]], 1<br>
-; CHECK-NEXT: ret i32 [[B]]<br>
+; CHECK-NEXT: [[B_NEG_NEG:%.*]] = shl i32 [[A:%.*]], 1<br>
+; CHECK-NEXT: ret i32 [[B_NEG_NEG]]<br>
;<br>
%B = sub i32 1, %A<br>
%C = shl i32 %B, 1<br>
@@ -1343,8 +1344,8 @@ define i32 @test63(i32 %A) {<br>
<br>
define <2 x i32> @test63vec(<2 x i32> %A) {<br>
; CHECK-LABEL: @test63vec(<br>
-; CHECK-NEXT: [[B:%.*]] = shl <2 x i32> [[A:%.*]], <i32 1, i32 1><br>
-; CHECK-NEXT: ret <2 x i32> [[B]]<br>
+; CHECK-NEXT: [[B_NEG_NEG:%.*]] = shl <2 x i32> [[A:%.*]], <i32 1, i32 1><br>
+; CHECK-NEXT: ret <2 x i32> [[B_NEG_NEG]]<br>
;<br>
%B = sub <2 x i32> <i32 1, i32 1>, %A<br>
%C = shl <2 x i32> %B, <i32 1, i32 1><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>