[llvm] r324014 - [InstCombine] allow multi-use values in canEvaluate* if all uses are in 1 inst
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 16:28:14 PST 2018
Copying this from the revert email. I think the issue is that this code
from canEvaluateTrucnated didn't get changed to only call it on one operand
when they are the same. So for Amjad's test case we recursed to the zext by
going through every possible path through the operands. Something like 2^24
different paths in this case.
case Instruction::Add:
case Instruction::Sub:
case Instruction::Mul:
case Instruction::And:
case Instruction::Or:
case Instruction::Xor:
// These operators can all arbitrarily be extended or truncated.
return canEvaluateTruncated(I->getOperand(0), Ty, IC, CxtI) &&
canEvaluateTruncated(I->getOperand(1), Ty, IC, CxtI);
~Craig
On Mon, Feb 5, 2018 at 4:16 PM, Chandler Carruth via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> FWIW, I'm continuing to work on a test case.
>
> I'm trying to bugpoint reduce in a way that will target a massive compile
> time hit that *only* reproduces with this change (I check that a compiler
> with this change commented out is at least 3x faster to compile). In
> practice so far, I'm down to 1242 instructions in the test case and well
> over 10x compile time difference before/after this patch (ms -> 7s). But it
> doesn't in fact inf-loop in the test case I'm reducing.
>
> Waiting to see how small bugpoint can get it before I try to clean it up
> and post it.
>
> On Mon, Feb 5, 2018 at 1:55 PM Sanjay Patel <spatel at rotateright.com>
> wrote:
>
>> Thanks for the example. I've reverted at r324276.
>> But it's not clear to me if the multi-use case alone is the problem. Ie,
>> can't we cause a compile-time explosion with single-use values too given
>> that the canEvaluate* functions don't have any recursion depth limit?
>>
>> On Mon, Feb 5, 2018 at 10:46 AM, Aboud, Amjad <amjad.aboud at intel.com>
>> wrote:
>>
>>> 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>
>>> 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>
>>> wrote:
>>>
>>> On Thu, Feb 1, 2018 at 1:57 PM Sanjay Patel via llvm-commits <
>>> 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
>>> 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.
>>>
>>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://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/20180205/3ca07705/attachment.html>
More information about the llvm-commits
mailing list