<div dir="ltr">Hi Alexey,<div><br></div><div>My commit appears to have caused a failure in your bot: <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/3827">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/3827</a></div><div><br></div><div>but the stdio makes it seem like the process got killed (out of memory?)</div><div><br></div><div>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)?</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div><div><br><div class="gmail_quote">On Fri, 15 May 2015 at 17:17 James Molloy <<a href="mailto:james.molloy@arm.com">james.molloy@arm.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jamesm<br>
Date: Fri May 15 11:10:59 2015<br>
New Revision: 237453<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=237453&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=237453&view=rev</a><br>
Log:<br>
Canonicalize min/max expressions correctly.<br>
<br>
This patch introduces a canonical form for min/max idioms where one operand<br>
is extended or truncated. This often happens when the other operand is a<br>
constant. For example:<br>
<br>
%1 = icmp slt i32 %a, i32 0<br>
%2 = sext i32 %a to i64<br>
%3 = select i1 %1, i64 %2, i64 0<br>
<br>
Would now be canonicalized into:<br>
<br>
%1 = icmp slt i32 %a, i32 0<br>
%2 = select i1 %1, i32 %a, i32 0<br>
%3 = sext i32 %2 to i64<br>
<br>
This builds upon a patch posted by David Majenemer<br>
(<a href="https://www.marc.info/?l=llvm-commits&m=143008038714141&w=2" target="_blank">https://www.marc.info/?l=llvm-commits&m=143008038714141&w=2</a>). That pass<br>
passively stopped instcombine from ruining canonical patterns. This<br>
patch additionally actively makes instcombine canonicalize too.<br>
<br>
Canonicalization of expressions involving a change in type from int->fp<br>
or fp->int are not yet implemented.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp<br>
llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=237453&r1=237452&r2=237453&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=237453&r1=237452&r2=237453&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Fri May 15 11:10:59 2015<br>
@@ -3970,6 +3970,19 @@ Instruction *InstCombiner::visitFCmpInst<br>
}<br>
}<br>
<br>
+ // Test if the FCmpInst instruction is used exclusively by a select as<br>
+ // part of a minimum or maximum operation. If so, refrain from doing<br>
+ // any other folding. This helps out other analyses which understand<br>
+ // non-obfuscated minimum and maximum idioms, such as ScalarEvolution<br>
+ // and CodeGen. And in this case, at least one of the comparison<br>
+ // operands has at least one user besides the compare (the select),<br>
+ // which would often largely negate the benefit of folding anyway.<br>
+ if (I.hasOneUse())<br>
+ if (SelectInst *SI = dyn_cast<SelectInst>(*I.user_begin()))<br>
+ if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||<br>
+ (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))<br>
+ return nullptr;<br>
+<br>
// Handle fcmp with constant RHS<br>
if (Constant *RHSC = dyn_cast<Constant>(Op1)) {<br>
if (Instruction *LHSI = dyn_cast<Instruction>(Op0))<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=237453&r1=237452&r2=237453&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=237453&r1=237452&r2=237453&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Fri May 15 11:10:59 2015<br>
@@ -1154,18 +1154,30 @@ Instruction *InstCombiner::visitSelectIn<br>
}<br>
<br>
// See if we can fold the select into one of our operands.<br>
- if (SI.getType()->isIntegerTy()) {<br>
+ if (SI.getType()->isIntOrIntVectorTy()) {<br>
if (Instruction *FoldI = FoldSelectIntoOp(SI, TrueVal, FalseVal))<br>
return FoldI;<br>
<br>
Value *LHS, *RHS, *LHS2, *RHS2;<br>
- SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS);<br>
+ Instruction::CastOps CastOp;<br>
+ SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS, &CastOp);<br>
<br>
- // MAX(MAX(a, b), a) -> MAX(a, b)<br>
- // MIN(MIN(a, b), a) -> MIN(a, b)<br>
- // MAX(MIN(a, b), a) -> a<br>
- // MIN(MAX(a, b), a) -> a<br>
if (SPF) {<br>
+ // Canonicalize so that type casts are outside select patterns.<br>
+ if (LHS->getType()->getPrimitiveSizeInBits() !=<br>
+ SI.getType()->getPrimitiveSizeInBits()) {<br>
+ CmpInst::Predicate Pred = getICmpPredicateForMinMax(SPF);<br>
+ Value *Cmp = Builder->CreateICmp(Pred, LHS, RHS);<br>
+ Value *NewSI = Builder->CreateCast(CastOp,<br>
+ Builder->CreateSelect(Cmp, LHS, RHS),<br>
+ SI.getType());<br>
+ return ReplaceInstUsesWith(SI, NewSI);<br>
+ }<br>
+<br>
+ // MAX(MAX(a, b), a) -> MAX(a, b)<br>
+ // MIN(MIN(a, b), a) -> MIN(a, b)<br>
+ // MAX(MIN(a, b), a) -> a<br>
+ // MIN(MAX(a, b), a) -> a<br>
if (SelectPatternFlavor SPF2 = matchSelectPattern(LHS, LHS2, RHS2))<br>
if (Instruction *R = FoldSPFofSPF(cast<Instruction>(LHS),SPF2,LHS2,RHS2,<br>
SI, SPF, RHS))<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=237453&r1=237452&r2=237453&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=237453&r1=237452&r2=237453&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri May 15 11:10:59 2015<br>
@@ -714,6 +714,22 @@ Instruction *InstCombiner::FoldOpIntoSel<br>
return nullptr;<br>
}<br>
<br>
+ // Test if a CmpInst instruction is used exclusively by a select as<br>
+ // part of a minimum or maximum operation. If so, refrain from doing<br>
+ // any other folding. This helps out other analyses which understand<br>
+ // non-obfuscated minimum and maximum idioms, such as ScalarEvolution<br>
+ // and CodeGen. And in this case, at least one of the comparison<br>
+ // operands has at least one user besides the compare (the select),<br>
+ // which would often largely negate the benefit of folding anyway.<br>
+ if (auto *CI = dyn_cast<CmpInst>(SI->getCondition())) {<br>
+ if (CI->hasOneUse()) {<br>
+ Value *Op0 = CI->getOperand(0), *Op1 = CI->getOperand(1);<br>
+ if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||<br>
+ (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))<br>
+ return nullptr;<br>
+ }<br>
+ }<br>
+<br>
Value *SelectTrueVal = FoldOperationIntoSelectOperand(Op, TV, this);<br>
Value *SelectFalseVal = FoldOperationIntoSelectOperand(Op, FV, this);<br>
<br>
@@ -723,7 +739,6 @@ Instruction *InstCombiner::FoldOpIntoSel<br>
return nullptr;<br>
}<br>
<br>
-<br>
/// FoldOpIntoPhi - Given a binary operator, cast instruction, or select which<br>
/// has a PHI node as operand #0, see if we can fold the instruction into the<br>
/// PHI (which is only possible if all operands to the PHI are constants).<br>
<br>
Added: llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll?rev=237453&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll?rev=237453&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll (added)<br>
+++ llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll Fri May 15 11:10:59 2015<br>
@@ -0,0 +1,73 @@<br>
+; RUN: opt -S -instcombine < %s | FileCheck %s<br>
+<br>
+; CHECK-LABEL: @t1<br>
+; CHECK-NEXT: icmp<br>
+; CHECK-NEXT: select<br>
+; CHECK-NEXT: sext<br>
+define i64 @t1(i32 %a) {<br>
+ ; This is the canonical form for a type-changing min/max.<br>
+ %1 = icmp slt i32 %a, 5<br>
+ %2 = select i1 %1, i32 %a, i32 5<br>
+ %3 = sext i32 %2 to i64<br>
+ ret i64 %3<br>
+}<br>
+<br>
+; CHECK-LABEL: @t2<br>
+; CHECK-NEXT: icmp<br>
+; CHECK-NEXT: select<br>
+; CHECK-NEXT: sext<br>
+define i64 @t2(i32 %a) {<br>
+ ; Check this is converted into canonical form, as above.<br>
+ %1 = icmp slt i32 %a, 5<br>
+ %2 = sext i32 %a to i64<br>
+ %3 = select i1 %1, i64 %2, i64 5<br>
+ ret i64 %3<br>
+}<br>
+<br>
+; CHECK-LABEL: @t3<br>
+; CHECK-NEXT: icmp<br>
+; CHECK-NEXT: select<br>
+; CHECK-NEXT: zext<br>
+define i64 @t3(i32 %a) {<br>
+ ; Same as @t2, with flipped operands and zext instead of sext.<br>
+ %1 = icmp ult i32 %a, 5<br>
+ %2 = zext i32 %a to i64<br>
+ %3 = select i1 %1, i64 5, i64 %2<br>
+ ret i64 %3<br>
+}<br>
+<br>
+; CHECK-LABEL: @t4<br>
+; CHECK-NEXT: icmp<br>
+; CHECK-NEXT: select<br>
+; CHECK-NEXT: trunc<br>
+define i32 @t4(i64 %a) {<br>
+ ; Same again, with trunc.<br>
+ %1 = icmp slt i64 %a, 5<br>
+ %2 = trunc i64 %a to i32<br>
+ %3 = select i1 %1, i32 %2, i32 5<br>
+ ret i32 %3<br>
+}<br>
+<br>
+; CHECK-LABEL: @t5<br>
+; CHECK-NEXT: icmp<br>
+; CHECK-NEXT: zext<br>
+; CHECK-NEXT: select<br>
+define i64 @t5(i32 %a) {<br>
+ ; Same as @t3, but with mismatched signedness between icmp and zext.<br>
+ ; InstCombine should leave this alone.<br>
+ %1 = icmp slt i32 %a, 5<br>
+ %2 = zext i32 %a to i64<br>
+ %3 = select i1 %1, i64 5, i64 %2<br>
+ ret i64 %3<br>
+}<br>
+<br>
+; CHECK-LABEL: @t6<br>
+; CHECK-NEXT: icmp<br>
+; CHECK-NEXT: select<br>
+; CHECK-NEXT: sitofp<br>
+define float @t6(i32 %a) {<br>
+ %1 = icmp slt i32 %a, 0<br>
+ %2 = select i1 %1, i32 %a, i32 0<br>
+ %3 = sitofp i32 %2 to float<br>
+ ret float %3<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div>