[llvm] r324014 - [InstCombine] allow multi-use values in canEvaluate* if all uses are in 1 inst
Aboud, Amjad via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 09:46:26 PST 2018
Hi Sanjay,
Please see this example, where aggressive-instcombine manage to reduce the expression “immediately”, while instcombine runs for minutes.
define i16 @foo(i16 %in) {
ENTRY:
%x = zext i16 %in to i32
%a1 = mul i32 %x, %x
%a2 = mul i32 %a1, %a1
%a3 = mul i32 %a2, %a2
%a4 = mul i32 %a3, %a3
%a5 = mul i32 %a4, %a4
%a6 = mul i32 %a5, %a5
%a7 = mul i32 %a6, %a6
%a8 = mul i32 %a7, %a7
%a9 = mul i32 %a8, %a8
%a10 = mul i32 %a9, %a9
%a11 = mul i32 %a10, %a10
%a12 = mul i32 %a11, %a11
%a13 = mul i32 %a12, %a12
%a14 = mul i32 %a13, %a13
%a15 = mul i32 %a14, %a14
%a16 = mul i32 %a15, %a15
%a17 = mul i32 %a16, %a16
%a18 = mul i32 %a17, %a17
%a19 = mul i32 %a18, %a18
%a20 = mul i32 %a19, %a19
%a21 = mul i32 %a20, %a20
%a22 = mul i32 %a21, %a21
%a23 = mul i32 %a22, %a22
%a24 = mul i32 %a23, %a23
%T = trunc i32 %a24 to i16
ret i16 %T
}
The reason is that you did not optimize the function “canEvaluateTruncated”, notice that same issue exists for other canEvaluate* functions.
Regards,
Amjad
From: Sanjay Patel [mailto:spatel at rotateright.com]
Sent: Monday, February 05, 2018 19:32
To: Chandler Carruth <chandlerc at gmail.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>; Aboud, Amjad <amjad.aboud at intel.com>
Subject: Re: [llvm] r324014 - [InstCombine] allow multi-use values in canEvaluate* if all uses are in 1 inst
As Amjad noted in D42739<https://reviews.llvm.org/D42739>, I failed to account for select / PHI ops with multi-uses in this patch. I've attempted to close that gap with:
https://reviews.llvm.org/rL324252
Please let me know if the inf-loop is gone with that change. I'm still trying to create a test that would find that gap, but assuming that was the bug, if we can get a reduction from whatever real program hit it, that would be great. Thanks!
On Mon, Feb 5, 2018 at 7:28 AM, Sanjay Patel <spatel at rotateright.com<mailto:spatel at rotateright.com>> wrote:
Thanks for letting me know. Sounds like an inf-loop.
Since this patch is duplicating a transform that occurs in -aggressive-instcombine, it's possible that the bug already exists apart from this change, but might take the benign form of a transform reversal across passes, or it might be masked in that case.
On Sat, Feb 3, 2018 at 8:02 PM, Chandler Carruth <chandlerc at gmail.com<mailto:chandlerc at gmail.com>> wrote:
On Thu, Feb 1, 2018 at 1:57 PM Sanjay Patel via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Author: spatel
Date: Thu Feb 1 13:55:53 2018
New Revision: 324014
URL: http://llvm.org/viewvc/llvm-project?rev=324014&view=rev
Log:
[InstCombine] allow multi-use values in canEvaluate* if all uses are in 1 inst
This is the enhancement suggested in D42536 to fix a shortcoming in
regular InstCombine's canEvaluate* functionality.
When we have multiple uses of a value, but they're all in one instruction, we can
allow that expression to be narrowed or widened for the same cost as a single-use
value.
AFAICT, this can only matter for multiply: sub/and/or/xor/select would be simplified
away if the operands are the same value; add becomes shl; shifts with a variable shift
amount aren't handled.
FWIW, we have at least one case which is now either inf-looping or taking dramatically longer to compile after this commit (way over 2x from what I can tell).
We're working on building a test case, but just wanted to send a heads-up in case others are hitting slow compiles.
Differential Revision: https://reviews.llvm.org/D42739
Modified:
llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll
llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=324014&r1=324013&r2=324014&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Thu Feb 1 13:55:53 2018
@@ -185,8 +185,14 @@ Value *InstCombiner::EvaluateInDifferent
case Instruction::Shl:
case Instruction::UDiv:
case Instruction::URem: {
- Value *LHS = EvaluateInDifferentType(I->getOperand(0), Ty, isSigned);
- Value *RHS = EvaluateInDifferentType(I->getOperand(1), Ty, isSigned);
+ Value *LHS, *RHS;
+ if (I->getOperand(0) == I->getOperand(1)) {
+ // Don't create an unnecessary value if the operands are repeated.
+ LHS = RHS = EvaluateInDifferentType(I->getOperand(0), Ty, isSigned);
+ } else {
+ LHS = EvaluateInDifferentType(I->getOperand(0), Ty, isSigned);
+ RHS = EvaluateInDifferentType(I->getOperand(1), Ty, isSigned);
+ }
Res = BinaryOperator::Create((Instruction::BinaryOps)Opc, LHS, RHS);
break;
}
@@ -320,10 +326,12 @@ static bool canNotEvaluateInType(Value *
assert(!isa<Constant>(V) && "Constant should already be handled.");
if (!isa<Instruction>(V))
return true;
- // We can't extend or shrink something that has multiple uses: doing so would
- // require duplicating the instruction in general, which isn't profitable.
+ // We can't extend or shrink something that has multiple uses -- unless those
+ // multiple uses are all in the same instruction -- doing so would require
+ // duplicating the instruction which isn't profitable.
if (!V->hasOneUse())
- return true;
+ if (any_of(V->users(), [&](User *U) { return U != V->user_back(); }))
+ return true;
return false;
}
Modified: llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll?rev=324014&r1=324013&r2=324014&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll Thu Feb 1 13:55:53 2018
@@ -49,11 +49,9 @@ define i8 @select2(i1 %cond, i8 %x, i8 %
define i32 @eval_trunc_multi_use_in_one_inst(i32 %x) {
; CHECK-LABEL: @eval_trunc_multi_use_in_one_inst(
-; CHECK-NEXT: [[Z:%.*]] = zext i32 [[X:%.*]] to i64
-; CHECK-NEXT: [[A:%.*]] = add nuw nsw i64 [[Z]], 15
-; CHECK-NEXT: [[M:%.*]] = mul i64 [[A]], [[A]]
-; CHECK-NEXT: [[T:%.*]] = trunc i64 [[M]] to i32
-; CHECK-NEXT: ret i32 [[T]]
+; CHECK-NEXT: [[A:%.*]] = add i32 [[X:%.*]], 15
+; CHECK-NEXT: [[M:%.*]] = mul i32 [[A]], [[A]]
+; CHECK-NEXT: ret i32 [[M]]
;
%z = zext i32 %x to i64
%a = add nsw nuw i64 %z, 15
@@ -64,11 +62,9 @@ define i32 @eval_trunc_multi_use_in_one_
define i32 @eval_zext_multi_use_in_one_inst(i32 %x) {
; CHECK-LABEL: @eval_zext_multi_use_in_one_inst(
-; CHECK-NEXT: [[T:%.*]] = trunc i32 [[X:%.*]] to i16
-; CHECK-NEXT: [[A:%.*]] = and i16 [[T]], 5
-; CHECK-NEXT: [[M:%.*]] = mul nuw nsw i16 [[A]], [[A]]
-; CHECK-NEXT: [[R:%.*]] = zext i16 [[M]] to i32
-; CHECK-NEXT: ret i32 [[R]]
+; CHECK-NEXT: [[A:%.*]] = and i32 [[X:%.*]], 5
+; CHECK-NEXT: [[M:%.*]] = mul nuw nsw i32 [[A]], [[A]]
+; CHECK-NEXT: ret i32 [[M]]
;
%t = trunc i32 %x to i16
%a = and i16 %t, 5
@@ -79,12 +75,10 @@ define i32 @eval_zext_multi_use_in_one_i
define i32 @eval_sext_multi_use_in_one_inst(i32 %x) {
; CHECK-LABEL: @eval_sext_multi_use_in_one_inst(
-; CHECK-NEXT: [[T:%.*]] = trunc i32 [[X:%.*]] to i16
-; CHECK-NEXT: [[A:%.*]] = and i16 [[T]], 14
-; CHECK-NEXT: [[M:%.*]] = mul nuw nsw i16 [[A]], [[A]]
-; CHECK-NEXT: [[O:%.*]] = or i16 [[M]], -32768
-; CHECK-NEXT: [[R:%.*]] = sext i16 [[O]] to i32
-; CHECK-NEXT: ret i32 [[R]]
+; CHECK-NEXT: [[A:%.*]] = and i32 [[X:%.*]], 14
+; CHECK-NEXT: [[M:%.*]] = mul nuw nsw i32 [[A]], [[A]]
+; CHECK-NEXT: [[O:%.*]] = or i32 [[M]], -32768
+; CHECK-NEXT: ret i32 [[O]]
;
%t = trunc i32 %x to i16
%a = and i16 %t, 14
Modified: llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll?rev=324014&r1=324013&r2=324014&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll Thu Feb 1 13:55:53 2018
@@ -55,13 +55,12 @@ lor.end:
define void @PR33765(i8 %beth) {
; CHECK-LABEL: @PR33765(
-; CHECK-NEXT: [[CONV:%.*]] = zext i8 [[BETH:%.*]] to i32
+; CHECK-NEXT: [[CONV:%.*]] = zext i8 [[BETH:%.*]] to i16
; CHECK-NEXT: br i1 false, label [[IF_THEN9:%.*]], label [[IF_THEN9]]
; CHECK: if.then9:
-; CHECK-NEXT: [[MUL:%.*]] = mul nuw nsw i32 [[CONV]], [[CONV]]
+; CHECK-NEXT: [[MUL:%.*]] = mul nuw i16 [[CONV]], [[CONV]]
; CHECK-NEXT: [[TINKY:%.*]] = load i16, i16* @glob, align 2
-; CHECK-NEXT: [[TMP1:%.*]] = trunc i32 [[MUL]] to i16
-; CHECK-NEXT: [[CONV14:%.*]] = and i16 [[TINKY]], [[TMP1]]
+; CHECK-NEXT: [[CONV14:%.*]] = and i16 [[TINKY]], [[MUL]]
; CHECK-NEXT: store i16 [[CONV14]], i16* @glob, align 2
; CHECK-NEXT: ret void
;
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180205/911d98c3/attachment-0001.html>
More information about the llvm-commits
mailing list