[llvm] r237453 - Canonicalize min/max expressions correctly.
Alexey Samsonov
vonosmas at gmail.com
Fri May 15 11:32:04 PDT 2015
Hi James,
Sending you the artifacts from the bot (preprocessed .cpp and .sh). This
doesn't look like a glitch - the bot was red for several revisions after
your commit, and turned back green after you reverted it.
On Fri, May 15, 2015 at 9:50 AM, James Molloy <james at jamesmolloy.co.uk>
wrote:
> Hi Alexey,
>
> My commit appears to have caused a failure in your bot:
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/3827
>
> but the stdio makes it seem like the process got killed (out of memory?)
>
> Could you please let me know if it was indeed my patch or if it's a glitch
> in your bot? And if so how I could reproduce (or more ideally the .cpp/.sh
> files in /tmp)?
>
> Cheers,
>
> James
>
> On Fri, 15 May 2015 at 17:17 James Molloy <james.molloy at arm.com> wrote:
>
>> Author: jamesm
>> Date: Fri May 15 11:10:59 2015
>> New Revision: 237453
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=237453&view=rev
>> Log:
>> Canonicalize min/max expressions correctly.
>>
>> This patch introduces a canonical form for min/max idioms where one
>> operand
>> is extended or truncated. This often happens when the other operand is a
>> constant. For example:
>>
>> %1 = icmp slt i32 %a, i32 0
>> %2 = sext i32 %a to i64
>> %3 = select i1 %1, i64 %2, i64 0
>>
>> Would now be canonicalized into:
>>
>> %1 = icmp slt i32 %a, i32 0
>> %2 = select i1 %1, i32 %a, i32 0
>> %3 = sext i32 %2 to i64
>>
>> This builds upon a patch posted by David Majenemer
>> (https://www.marc.info/?l=llvm-commits&m=143008038714141&w=2). That pass
>> passively stopped instcombine from ruining canonical patterns. This
>> patch additionally actively makes instcombine canonicalize too.
>>
>> Canonicalization of expressions involving a change in type from int->fp
>> or fp->int are not yet implemented.
>>
>> Added:
>> llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll
>> Modified:
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
>> llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=237453&r1=237452&r2=237453&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>> (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Fri May
>> 15 11:10:59 2015
>> @@ -3970,6 +3970,19 @@ Instruction *InstCombiner::visitFCmpInst
>> }
>> }
>>
>> + // Test if the FCmpInst instruction is used exclusively by a select as
>> + // part of a minimum or maximum operation. If so, refrain from doing
>> + // any other folding. This helps out other analyses which understand
>> + // non-obfuscated minimum and maximum idioms, such as ScalarEvolution
>> + // and CodeGen. And in this case, at least one of the comparison
>> + // operands has at least one user besides the compare (the select),
>> + // which would often largely negate the benefit of folding anyway.
>> + if (I.hasOneUse())
>> + if (SelectInst *SI = dyn_cast<SelectInst>(*I.user_begin()))
>> + if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||
>> + (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
>> + return nullptr;
>> +
>> // Handle fcmp with constant RHS
>> if (Constant *RHSC = dyn_cast<Constant>(Op1)) {
>> if (Instruction *LHSI = dyn_cast<Instruction>(Op0))
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=237453&r1=237452&r2=237453&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Fri May
>> 15 11:10:59 2015
>> @@ -1154,18 +1154,30 @@ Instruction *InstCombiner::visitSelectIn
>> }
>>
>> // See if we can fold the select into one of our operands.
>> - if (SI.getType()->isIntegerTy()) {
>> + if (SI.getType()->isIntOrIntVectorTy()) {
>> if (Instruction *FoldI = FoldSelectIntoOp(SI, TrueVal, FalseVal))
>> return FoldI;
>>
>> Value *LHS, *RHS, *LHS2, *RHS2;
>> - SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS);
>> + Instruction::CastOps CastOp;
>> + SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS, &CastOp);
>>
>> - // MAX(MAX(a, b), a) -> MAX(a, b)
>> - // MIN(MIN(a, b), a) -> MIN(a, b)
>> - // MAX(MIN(a, b), a) -> a
>> - // MIN(MAX(a, b), a) -> a
>> if (SPF) {
>> + // Canonicalize so that type casts are outside select patterns.
>> + if (LHS->getType()->getPrimitiveSizeInBits() !=
>> + SI.getType()->getPrimitiveSizeInBits()) {
>> + CmpInst::Predicate Pred = getICmpPredicateForMinMax(SPF);
>> + Value *Cmp = Builder->CreateICmp(Pred, LHS, RHS);
>> + Value *NewSI = Builder->CreateCast(CastOp,
>> + Builder->CreateSelect(Cmp,
>> LHS, RHS),
>> + SI.getType());
>> + return ReplaceInstUsesWith(SI, NewSI);
>> + }
>> +
>> + // MAX(MAX(a, b), a) -> MAX(a, b)
>> + // MIN(MIN(a, b), a) -> MIN(a, b)
>> + // MAX(MIN(a, b), a) -> a
>> + // MIN(MAX(a, b), a) -> a
>> if (SelectPatternFlavor SPF2 = matchSelectPattern(LHS, LHS2, RHS2))
>> if (Instruction *R =
>> FoldSPFofSPF(cast<Instruction>(LHS),SPF2,LHS2,RHS2,
>> SI, SPF, RHS))
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=237453&r1=237452&r2=237453&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>> (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri
>> May 15 11:10:59 2015
>> @@ -714,6 +714,22 @@ Instruction *InstCombiner::FoldOpIntoSel
>> return nullptr;
>> }
>>
>> + // Test if a CmpInst instruction is used exclusively by a select as
>> + // part of a minimum or maximum operation. If so, refrain from doing
>> + // any other folding. This helps out other analyses which understand
>> + // non-obfuscated minimum and maximum idioms, such as ScalarEvolution
>> + // and CodeGen. And in this case, at least one of the comparison
>> + // operands has at least one user besides the compare (the select),
>> + // which would often largely negate the benefit of folding anyway.
>> + if (auto *CI = dyn_cast<CmpInst>(SI->getCondition())) {
>> + if (CI->hasOneUse()) {
>> + Value *Op0 = CI->getOperand(0), *Op1 = CI->getOperand(1);
>> + if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||
>> + (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
>> + return nullptr;
>> + }
>> + }
>> +
>> Value *SelectTrueVal = FoldOperationIntoSelectOperand(Op, TV, this);
>> Value *SelectFalseVal = FoldOperationIntoSelectOperand(Op, FV, this);
>>
>> @@ -723,7 +739,6 @@ Instruction *InstCombiner::FoldOpIntoSel
>> return nullptr;
>> }
>>
>> -
>> /// FoldOpIntoPhi - Given a binary operator, cast instruction, or select
>> which
>> /// has a PHI node as operand #0, see if we can fold the instruction
>> into the
>> /// PHI (which is only possible if all operands to the PHI are
>> constants).
>>
>> Added: llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll?rev=237453&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll (added)
>> +++ llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll Fri May 15
>> 11:10:59 2015
>> @@ -0,0 +1,73 @@
>> +; RUN: opt -S -instcombine < %s | FileCheck %s
>> +
>> +; CHECK-LABEL: @t1
>> +; CHECK-NEXT: icmp
>> +; CHECK-NEXT: select
>> +; CHECK-NEXT: sext
>> +define i64 @t1(i32 %a) {
>> + ; This is the canonical form for a type-changing min/max.
>> + %1 = icmp slt i32 %a, 5
>> + %2 = select i1 %1, i32 %a, i32 5
>> + %3 = sext i32 %2 to i64
>> + ret i64 %3
>> +}
>> +
>> +; CHECK-LABEL: @t2
>> +; CHECK-NEXT: icmp
>> +; CHECK-NEXT: select
>> +; CHECK-NEXT: sext
>> +define i64 @t2(i32 %a) {
>> + ; Check this is converted into canonical form, as above.
>> + %1 = icmp slt i32 %a, 5
>> + %2 = sext i32 %a to i64
>> + %3 = select i1 %1, i64 %2, i64 5
>> + ret i64 %3
>> +}
>> +
>> +; CHECK-LABEL: @t3
>> +; CHECK-NEXT: icmp
>> +; CHECK-NEXT: select
>> +; CHECK-NEXT: zext
>> +define i64 @t3(i32 %a) {
>> + ; Same as @t2, with flipped operands and zext instead of sext.
>> + %1 = icmp ult i32 %a, 5
>> + %2 = zext i32 %a to i64
>> + %3 = select i1 %1, i64 5, i64 %2
>> + ret i64 %3
>> +}
>> +
>> +; CHECK-LABEL: @t4
>> +; CHECK-NEXT: icmp
>> +; CHECK-NEXT: select
>> +; CHECK-NEXT: trunc
>> +define i32 @t4(i64 %a) {
>> + ; Same again, with trunc.
>> + %1 = icmp slt i64 %a, 5
>> + %2 = trunc i64 %a to i32
>> + %3 = select i1 %1, i32 %2, i32 5
>> + ret i32 %3
>> +}
>> +
>> +; CHECK-LABEL: @t5
>> +; CHECK-NEXT: icmp
>> +; CHECK-NEXT: zext
>> +; CHECK-NEXT: select
>> +define i64 @t5(i32 %a) {
>> + ; Same as @t3, but with mismatched signedness between icmp and zext.
>> + ; InstCombine should leave this alone.
>> + %1 = icmp slt i32 %a, 5
>> + %2 = zext i32 %a to i64
>> + %3 = select i1 %1, i64 5, i64 %2
>> + ret i64 %3
>> +}
>> +
>> +; CHECK-LABEL: @t6
>> +; CHECK-NEXT: icmp
>> +; CHECK-NEXT: select
>> +; CHECK-NEXT: sitofp
>> +define float @t6(i32 %a) {
>> + %1 = icmp slt i32 %a, 0
>> + %2 = select i1 %1, i32 %a, i32 0
>> + %3 = sitofp i32 %2 to float
>> + ret float %3
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
--
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150515/9e689673/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: old_test_suite-e7b725.tar.gz
Type: application/x-gzip
Size: 634957 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150515/9e689673/attachment.bin>
More information about the llvm-commits
mailing list