<div dir="ltr"><div><div><div><div><div><div><div>Thanks for letting me know about this problem!<br><br>There's no 'shl nsw' visible in the earlier (r292487) code, so it would be better to see exactly what the IR looks like before that added transform fires.<br></div><br>But I see a red flag:<br>
%smax = select i1 %11, i64 %10, i64 8193<br><br></div>The new icmp transform allowed us to create an smax, but we have this hack in InstCombiner::visitICmpInst():<br><br> // Test if the ICmpInst 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><br></div>...so that prevented folding the icmp into the earlier math.<br><br></div>I am actively working on trying to get rid of that bail-out by improving min/max value tracking and icmp/select folding. In fact, we might be able to remove it right now, but I don't know the history of that code or what cases it was supposed to help.<br></div></div></div><div><div><div><div><br></div><div>If it's possible, can you remove that check locally, rebuild, and try the benchmark again on your system? I'd love to know if that change alone would solve the problem.<br></div><div><br><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 20, 2017 at 10:11 AM, Evgeny Astigeevich <span dir="ltr"><<a href="mailto:Evgeny.Astigeevich@arm.com" target="_blank">Evgeny.Astigeevich@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
We found that today's 17.30%/11.37% performance regressions in LNT SingleSource/Benchmarks/Shooto<wbr>ut/sieve on LNT-AArch64-A53-O3__clang_DEV_<wbr>_aarch64 and LNT-Thumb2v7-A15-O3__clang_DEV<wbr>__thumbv7 (<a href="http://llvm.org/perf/db_default/v4/nts/daily_report/2017/1/20?filter-machine-regex=aarch64%7Carm%7Cthumb%7Cgreen" rel="noreferrer" target="_blank">http://llvm.org/perf/db_defau<wbr>lt/v4/nts/daily_report/2017/1/<wbr>20?filter-machine-regex=aarch6<wbr>4%7Carm%7Cthumb%7Cgreen</a>) are caused by changes [rL292492] in InstCombine:<br>
<br>
<a href="https://reviews.llvm.org/D28406" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2840<wbr>6</a> "[InstCombine] icmp sgt (shl nsw X, C1), C0 --> icmp sgt X, C0 >> C1"<br>
<br>
The Loop Vectorizer generates code with more instructions:<br>
<br>
==== Loop Vectorizer from rL292492 ====<br>
for.body5: ; preds = %for.inc16.for.body5_crit_edge<wbr>, %for.cond.preheader<br>
%indvar = phi i64 [ %indvar.next, %for.inc16.for.body5_crit_edge ], [ 0, %for.cond.preheader ]<br>
%1 = phi i8 [ %.pre, %for.inc16.for.body5_crit_edge ], [ 1, %for.cond.preheader ]<br>
%count.122 = phi i32 [ %count.2, %for.inc16.for.body5_crit_edge ], [ 0, %for.cond.preheader ]<br>
%i.119 = phi i64 [ %inc17, %for.inc16.for.body5_crit_edge ], [ 2, %for.cond.preheader ]<br>
%2 = add i64 %indvar, 2<br>
%3 = shl i64 %indvar, 1<br>
%4 = add i64 %3, 4<br>
%5 = add i64 %indvar, 2<br>
%6 = shl i64 %indvar, 1<br>
%7 = add i64 %6, 4<br>
%8 = add i64 %indvar, 2<br>
%9 = mul i64 %indvar, 3<br>
%10 = add i64 %9, 6<br>
%11 = icmp sgt i64 %10, 8193<br>
%smax = select i1 %11, i64 %10, i64 8193<br>
%12 = mul i64 %indvar, -2<br>
%13 = add i64 %12, -5<br>
%14 = add i64 %smax, %13<br>
%15 = add i64 %indvar, 2<br>
%16 = udiv i64 %14, %15<br>
%17 = add i64 %16, 1<br>
%tobool7 = icmp eq i8 %1, 0<br>
br i1 %tobool7, label %for.inc16, label %if.then<br>
==============================<wbr>==<br>
<br>
The code generated by the Loop Vectorizer before the changes:<br>
<br>
==== Loop Vectorizer from rL292487 ====<br>
for.body5: ; preds = %for.inc16.for.body5_crit_edge<wbr>, %for.cond.preheader<br>
%indvar = phi i64 [ %indvar.next, %for.inc16.for.body5_crit_edge ], [ 0, %for.cond.preheader ]<br>
%1 = phi i8 [ %.pre, %for.inc16.for.body5_crit_edge ], [ 1, %for.cond.preheader ]<br>
%count.122 = phi i32 [ %count.2, %for.inc16.for.body5_crit_edge ], [ 0, %for.cond.preheader ]<br>
%i.119 = phi i64 [ %inc17, %for.inc16.for.body5_crit_edge ], [ 2, %for.cond.preheader ]<br>
%2 = add i64 %indvar, 2<br>
%3 = shl i64 %indvar, 1<br>
%4 = add i64 %3, 4<br>
%5 = add i64 %indvar, 2<br>
%6 = shl i64 %indvar, 1<br>
%7 = add i64 %6, 4<br>
%8 = add i64 %indvar, 2<br>
%9 = mul i64 %indvar, -2<br>
%10 = add i64 %9, 8188<br>
%11 = add i64 %indvar, 2<br>
%12 = udiv i64 %10, %11<br>
%13 = add i64 %12, 1<br>
%tobool7 = icmp eq i8 %1, 0<br>
br i1 %tobool7, label %for.inc16, label %if.then<br>
==============================<wbr>==<br>
<br>
I have not investigated yet why the behaviour of the Vectorizer is changed.<br>
<br>
Kind regards,<br>
Evgeny Astigeevich<br>
Senior Compiler Engineer<br>
Compilation Tools<br>
ARM<br>
<br>
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>
</blockquote></div><br></div></div></div></div></div></div></div></div></div></div>