<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">
Hi Alexey,
<div><br>
</div>
<div>Thanks! It turns out your bot was the quickest to spot it - about 15 mins later I was inundated.</div>
<div><br>
</div>
<div>Thanks,</div>
<div><br>
</div>
<div>James</div>
<div><br>
<div>
<div>On 15 May 2015, at 19:32, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div dir="ltr">Hi James,
<div><br>
</div>
<div>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.</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, May 15, 2015 at 9:50 AM, James Molloy <span dir="ltr">
<<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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" target="_blank">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>
<div class="h5"><br>
<div class="gmail_quote">On Fri, 15 May 2015 at 17:17 James Molloy <<a href="mailto:james.molloy@arm.com" target="_blank">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>
</div>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">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>
<br>
</blockquote>
</div>
<br>
<br clear="all">
<div><br>
</div>
-- <br>
<div class="gmail_signature">
<div dir="ltr">Alexey Samsonov<br>
<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div>
</div>
</div>
<span><old_test_suite-e7b725.tar.gz></span></blockquote>
</div>
<br>
</div>
<br>
<font face="Arial" color="Black" size="2">-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents
to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.<br>
<br>
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590<br>
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782<br>
</font>
</body>
</html>