<div dir="ltr">Hi <span style="font-size:13.63636302948px">Michael and David,</span><div><span style="font-size:13.63636302948px"><br></span></div><div><span style="font-size:13.63636302948px">I also see some performance regressions on aarch64 target introduced by this commit. No further invesitgation on what's the root cause, but I guess the one Michael shared may cause regression on many targets, including aarch64.</span></div><div><span style="font-size:13.63636302948px"><br></span></div><div><span style="font-size:13.63636302948px">I argee with </span><span style="font-size:13.63636302948px">Michael's suggestion that</span><span style="font-size:13.63636302948px"> revert this commit firstly, and make more performance test over it.</span></div><div><span style="font-size:13.63636302948px"><br></span></div><div><span style="font-size:13.63636302948px">Thanks,</span></div><div><span style="font-size:13.63636302948px">Kevin</span></div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-01-14 21:25 GMT+08:00 Kuperstein, Michael M <span dir="ltr"><<a href="mailto:michael.m.kuperstein@intel.com" target="_blank">michael.m.kuperstein@intel.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi David,<br>
<br>
It turns out this is causing quite a lot of performance regressions.<br>
<br>
One of the issues is that this messes up IndVar simplification.<br>
<br>
For this code:<br>
int foo (int *p, int M, int N, int K)<br>
{<br>
int ret = 0;<br>
for (int i = 0; i < M-N; i++)<br>
ret += p[i * K];<br>
<br>
return ret;<br>
}<br>
<br>
What we get coming into IndVar Simplify before the patch is:<br>
define i32 @foo(i32* nocapture readonly %p, i32 %M, i32 %N, i32 %K) #0 {<br>
entry:<br>
%sub = sub nsw i32 %M, %N<br>
%cmp4 = icmp sgt i32 %sub, 0<br>
br i1 %cmp4, label %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>, label %for.end<br>
<br>
<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>: ; preds = %entry<br>
br label %for.body<br>
<br>
for.body: ; preds = %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>, %for.body<br>
%i.06 = phi i32 [ 0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %inc, %for.body ]<br>
%ret.05 = phi i32 [ 0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %add, %for.body ]<br>
%mul = mul nsw i32 %i.06, %K<br>
%arrayidx = getelementptr inbounds i32* %p, i32 %mul<br>
%0 = load i32* %arrayidx, align 4, !tbaa !1<br>
%add = add nsw i32 %0, %ret.05<br>
%inc = add nsw i32 %i.06, 1<br>
%cmp = icmp slt i32 %inc, %sub<br>
br i1 %cmp, label %for.body, label %for.cond.for.end_crit_edge<br>
<br>
for.cond.for.end_crit_edge: ; preds = %for.body<br>
%add.lcssa = phi i32 [ %add, %for.body ]<br>
br label %for.end<br>
<br>
for.end: ; preds = %for.cond.for.end_crit_edge, %entry<br>
%ret.0.lcssa = phi i32 [ %add.lcssa, %for.cond.for.end_crit_edge ], [ 0, %entry ]<br>
ret i32 %ret.0.lcssa<br>
}<br>
<br>
IndVar Simplify manages to compute the SCEV for the induction variable:<br>
INDVARS: Rewriting loop exit condition to:<br>
LHS: %i.06 = phi i32 [ 0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %inc, %for.body ]<br>
op: !=<br>
RHS: %1 = sub i32 %0, %N<br>
IVCount: (-1 + (-1 * %N) + %M)<br>
<br>
The condition on the backedge gets replaced with an "icmp ne", and the final assembly for the loop body is:<br>
.LBB0_2:<br>
addl (%esi), %eax<br>
addl %edx, %esi<br>
decl %ecx<br>
jne .LBB0_2<br>
<br>
With r225034, IndVar Simplify doesn't fire, we drag the "icmp slt" until the end, and end up with:<br>
.LBB0_2:<br>
addl (%esi), %eax<br>
incl %edi<br>
addl %edx, %esi<br>
cmpl %ecx, %edi<br>
jl .LBB0_2<br>
<br>
Note that this is not the only issue we're seeing, some of the regressions don't appear to be IndVar-related.<br>
It may be a good idea to revert r225034 and do more extensive performance testing before re-committing.<br>
<br>
Thanks,<br>
Michael<br>
<span class=""><br>
-----Original Message-----<br>
From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of David Majnemer<br>
Sent: Wednesday, December 31, 2014 06:22<br>
To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
Subject: [llvm] r225034 - InstCombine: try to transform A-B < 0 into A < B<br>
<br>
Author: majnemer<br>
Date: Tue Dec 30 22:21:41 2014<br>
New Revision: 225034<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225034&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225034&view=rev</a><br>
Log:<br>
InstCombine: try to transform A-B < 0 into A < B<br>
<br>
We are allowed to move the 'B' to the right hand side if we an prove there is no signed overflow and if the comparison itself is signed.<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
llvm/trunk/test/Transforms/InstCombine/icmp.ll<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=225034&r1=225033&r2=225034&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=225034&r1=225033&r2=225034&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Tue<br>
</span>+++ Dec 30 22:21:41 2014<br>
<div><div class="h5">@@ -2692,6 +2692,26 @@ Instruction *InstCombiner::visitICmpInst<br>
return new ICmpInst(I.getPredicate(), A, B);<br>
}<br>
<br>
+ // (icmp sgt (sub nsw A B), -1) -> (icmp sge A, B)<br>
+ if (I.getPredicate() == ICmpInst::ICMP_SGT && CI->isAllOnesValue() &&<br>
+ match(Op0, m_NSWSub(m_Value(A), m_Value(B))))<br>
+ return new ICmpInst(ICmpInst::ICMP_SGE, A, B);<br>
+<br>
+ // (icmp sgt (sub nsw A B), 0) -> (icmp sgt A, B)<br>
+ if (I.getPredicate() == ICmpInst::ICMP_SGT && CI->isZero() &&<br>
+ match(Op0, m_NSWSub(m_Value(A), m_Value(B))))<br>
+ return new ICmpInst(ICmpInst::ICMP_SGT, A, B);<br>
+<br>
+ // (icmp slt (sub nsw A B), 0) -> (icmp slt A, B)<br>
+ if (I.getPredicate() == ICmpInst::ICMP_SLT && CI->isZero() &&<br>
+ match(Op0, m_NSWSub(m_Value(A), m_Value(B))))<br>
+ return new ICmpInst(ICmpInst::ICMP_SLT, A, B);<br>
+<br>
+ // (icmp slt (sub nsw A B), 1) -> (icmp sle A, B)<br>
+ if (I.getPredicate() == ICmpInst::ICMP_SLT && CI->isOne() &&<br>
+ match(Op0, m_NSWSub(m_Value(A), m_Value(B))))<br>
+ return new ICmpInst(ICmpInst::ICMP_SLE, A, B);<br>
+<br>
// If we have an icmp le or icmp ge instruction, turn it into the<br>
// appropriate icmp lt or icmp gt instruction. This allows us to rely on<br>
// them being folded in the code below. The SimplifyICmpInst code has<br>
<br>
Modified: llvm/trunk/test/Transforms/InstCombine/icmp.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp.ll?rev=225034&r1=225033&r2=225034&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp.ll?rev=225034&r1=225033&r2=225034&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/icmp.ll (original)<br>
+++ llvm/trunk/test/Transforms/InstCombine/icmp.ll Tue Dec 30 22:21:41<br>
</div></div>+++ 2014<br>
<span class="">@@ -1522,3 +1522,39 @@ define zeroext i1 @icmp_cmpxchg_strong(i<br>
%icmp = icmp eq i32 %xtrc, %old_val<br>
ret i1 %icmp<br>
}<br>
+<br>
+; CHECK-LABEL: @f1<br>
</span>+; CHECK-NEXT: %[[cmp:.*]] = icmp sge i64 %a, %b ; CHECK-NEXT: ret i1<br>
+%[[cmp]] define i1 @f1(i64 %a, i64 %b) {<br>
<span class="">+ %t = sub nsw i64 %a, %b<br>
+ %v = icmp sge i64 %t, 0<br>
+ ret i1 %v<br>
+}<br>
+<br>
+; CHECK-LABEL: @f2<br>
</span>+; CHECK-NEXT: %[[cmp:.*]] = icmp sgt i64 %a, %b ; CHECK-NEXT: ret i1<br>
+%[[cmp]] define i1 @f2(i64 %a, i64 %b) {<br>
<span class="">+ %t = sub nsw i64 %a, %b<br>
+ %v = icmp sgt i64 %t, 0<br>
+ ret i1 %v<br>
+}<br>
+<br>
+; CHECK-LABEL: @f3<br>
</span>+; CHECK-NEXT: %[[cmp:.*]] = icmp slt i64 %a, %b ; CHECK-NEXT: ret i1<br>
+%[[cmp]] define i1 @f3(i64 %a, i64 %b) {<br>
<span class="">+ %t = sub nsw i64 %a, %b<br>
+ %v = icmp slt i64 %t, 0<br>
+ ret i1 %v<br>
+}<br>
+<br>
+; CHECK-LABEL: @f4<br>
</span>+; CHECK-NEXT: %[[cmp:.*]] = icmp sle i64 %a, %b ; CHECK-NEXT: ret i1<br>
+%[[cmp]] define i1 @f4(i64 %a, i64 %b) {<br>
<span class="">+ %t = sub nsw i64 %a, %b<br>
+ %v = icmp sle i64 %t, 0<br>
+ ret i1 %v<br>
+}<br>
<br>
<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>
</span>---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<br>
<div class="HOEnZb"><div class="h5"><br>
<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>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Best Regards,<div><br></div><div>Kevin Qin</div></div></div>
</div>