[llvm] r324276 - [InstCombine] don't try to evaluate instructions with >1 use (revert r324014)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 15:05:00 PST 2018
Add a check to see if the operands are the same?
if (I->getOperand(0) == I->getOperand(1))
return canEvaluateTruncated(I->getOperand(0), Ty, IC, CxtI);
return canEvaluateTruncated(I->getOperand(0), Ty, IC, CxtI) &&
canEvaluateTruncated(I->getOperand(1), Ty, IC, CxtI);
Yes, I think that would prevent the problem, but I'm now wondering if the
whole canEvaluate() functionality is out-of-place for instcombine. There's
no recursion depth limit here, so it's possible this already takes too long
for some corner cases. If aggressive-instcombine can fold these sequences
faster, then move it all over there?
On Mon, Feb 5, 2018 at 3:01 PM, Craig Topper <craig.topper at gmail.com> wrote:
> Shouldn't this part of canEvaluateTruncated been changed in the original
> patch to avoid the explosion?
>
> 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 1:50 PM, Sanjay Patel via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: spatel
>> Date: Mon Feb 5 13:50:32 2018
>> New Revision: 324276
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=324276&view=rev
>> Log:
>> [InstCombine] don't try to evaluate instructions with >1 use (revert
>> r324014)
>>
>> This example causes a compile-time explosion:
>>
>> define i16 @foo(i16 %in) {
>> %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
>> }
>>
>>
>>
>> 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/Transform
>> s/InstCombine/InstCombineCasts.cpp?rev=324276&r1=324275&r2=3
>> 24276&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Mon Feb
>> 5 13:50:32 2018
>> @@ -185,14 +185,8 @@ Value *InstCombiner::EvaluateInDifferent
>> case Instruction::Shl:
>> case Instruction::UDiv:
>> case Instruction::URem: {
>> - 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);
>> - }
>> + Value *LHS = EvaluateInDifferentType(I->getOperand(0), Ty,
>> isSigned);
>> + Value *RHS = EvaluateInDifferentType(I->getOperand(1), Ty,
>> isSigned);
>> Res = BinaryOperator::Create((Instruction::BinaryOps)Opc, LHS, RHS);
>> break;
>> }
>> @@ -326,15 +320,11 @@ 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 --
>> unless those
>> - // multiple uses are all in the same binop instruction -- doing so
>> would
>> - // require duplicating the instruction which isn't profitable.
>> - if (!V->hasOneUse()) {
>> - if (!match(V->user_back(), m_BinOp()))
>> - return true;
>> - if (any_of(V->users(), [&](User *U) { return U != V->user_back(); }))
>> - return true;
>> - }
>> + // We don't extend or shrink something that has multiple uses --
>> doing so
>> + // would require duplicating the instruction which isn't profitable.
>> + if (!V->hasOneUse())
>> + return true;
>> +
>> return false;
>> }
>>
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transfor
>> ms/InstCombine/cast-mul-select.ll?rev=324276&r1=324275&r2=
>> 324276&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/cast-mul-select.ll Mon Feb 5
>> 13:50:32 2018
>> @@ -47,11 +47,16 @@ define i8 @select2(i1 %cond, i8 %x, i8 %
>> ret i8 %F
>> }
>>
>> +; The next 3 tests could be handled in instcombine, but evaluating values
>> +; with multiple uses may be very slow. Let some other pass deal with it.
>> +
>> define i32 @eval_trunc_multi_use_in_one_inst(i32 %x) {
>> ; CHECK-LABEL: @eval_trunc_multi_use_in_one_inst(
>> -; CHECK-NEXT: [[A:%.*]] = add i32 [[X:%.*]], 15
>> -; CHECK-NEXT: [[M:%.*]] = mul i32 [[A]], [[A]]
>> -; CHECK-NEXT: ret i32 [[M]]
>> +; 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]]
>> ;
>> %z = zext i32 %x to i64
>> %a = add nsw nuw i64 %z, 15
>> @@ -62,9 +67,11 @@ 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: [[A:%.*]] = and i32 [[X:%.*]], 5
>> -; CHECK-NEXT: [[M:%.*]] = mul nuw nsw i32 [[A]], [[A]]
>> -; CHECK-NEXT: ret i32 [[M]]
>> +; 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]]
>> ;
>> %t = trunc i32 %x to i16
>> %a = and i16 %t, 5
>> @@ -75,10 +82,12 @@ 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: [[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]]
>> +; 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]]
>> ;
>> %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/Transfor
>> ms/InstCombine/icmp-mul-zext.ll?rev=324276&r1=324275&r2=324276&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/icmp-mul-zext.ll Mon Feb 5
>> 13:50:32 2018
>> @@ -55,12 +55,13 @@ lor.end:
>>
>> define void @PR33765(i8 %beth) {
>> ; CHECK-LABEL: @PR33765(
>> -; CHECK-NEXT: [[CONV:%.*]] = zext i8 [[BETH:%.*]] to i16
>> +; CHECK-NEXT: [[CONV:%.*]] = zext i8 [[BETH:%.*]] to i32
>> ; CHECK-NEXT: br i1 false, label [[IF_THEN9:%.*]], label [[IF_THEN9]]
>> ; CHECK: if.then9:
>> -; CHECK-NEXT: [[MUL:%.*]] = mul nuw i16 [[CONV]], [[CONV]]
>> +; CHECK-NEXT: [[MUL:%.*]] = mul nuw nsw i32 [[CONV]], [[CONV]]
>> ; CHECK-NEXT: [[TINKY:%.*]] = load i16, i16* @glob, align 2
>> -; CHECK-NEXT: [[CONV14:%.*]] = and i16 [[TINKY]], [[MUL]]
>> +; CHECK-NEXT: [[TMP1:%.*]] = trunc i32 [[MUL]] to i16
>> +; CHECK-NEXT: [[CONV14:%.*]] = and i16 [[TINKY]], [[TMP1]]
>> ; 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
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180205/7eb0b278/attachment.html>
More information about the llvm-commits
mailing list