<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";
mso-fareast-language:EN-US;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-GB" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I’ve committed a fix in r334019. The problem was in how instcombine was canonicalizing abs, as<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">it was using the type of the select operand to create an operand to the cmp but should have<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">been (and now is) using the type of the cmp operand.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">John<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Ilya Biryukov [mailto:ibiryukov@google.com]
<br>
<b>Sent:</b> 05 June 2018 15:31<br>
<b>To:</b> John Brawn<br>
<b>Cc:</b> llvm-commits<br>
<b>Subject:</b> Re: [llvm] r333927 - [ValueTracking] Match select abs pattern when there's an sext involved<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">This change seems to break the PGO build of clang for us internally. We get an error from BitcodeReaderBase ("Invalid record"), couldn't chase it down any further yet.<o:p></o:p></p>
<div>
<p class="MsoNormal">I'm also struggling to get the reproducer with an upstream version, but no luck so far<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Any ideas on how could this change could break the codegen?<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Mon, Jun 4, 2018 at 6:58 PM John Brawn via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">Author: john.brawn<br>
Date: Mon Jun 4 09:53:57 2018<br>
New Revision: 333927<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=333927&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=333927&view=rev</a><br>
Log:<br>
[ValueTracking] Match select abs pattern when there's an sext involved<br>
<br>
When checking a select to see if it matches an abs, allow the true/false values<br>
to be a sign-extension of the comparison value instead of requiring that they're<br>
directly the comparison value, as all the comparison cares about is the sign of<br>
the value.<br>
<br>
This fixes a regression due to r333702, where we were no longer generating ctlz<br>
due to isKnownNonNegative failing to match such a pattern.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D47631" target="_blank">
https://reviews.llvm.org/D47631</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Analysis/ValueTracking.cpp<br>
llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll<br>
llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll<br>
<br>
Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=333927&r1=333926&r2=333927&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=333927&r1=333926&r2=333927&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Jun 4 09:53:57 2018<br>
@@ -4604,23 +4604,35 @@ static SelectPatternResult matchSelectPa<br>
<br>
const APInt *C1;<br>
if (match(CmpRHS, m_APInt(C1))) {<br>
- if ((CmpLHS == TrueVal && match(FalseVal, m_Neg(m_Specific(CmpLHS)))) ||<br>
- (CmpLHS == FalseVal && match(TrueVal, m_Neg(m_Specific(CmpLHS))))) {<br>
- // Set RHS to the negate operand. LHS was assigned to CmpLHS earlier.<br>
- RHS = (CmpLHS == TrueVal) ? FalseVal : TrueVal;<br>
+ // Sign-extending LHS does not change its sign, so TrueVal/FalseVal can<br>
+ // match against either LHS or sext(LHS).<br>
+ auto MaybeSExtLHS = m_CombineOr(m_Specific(CmpLHS),<br>
+ m_SExt(m_Specific(CmpLHS)));<br>
+ if ((match(TrueVal, MaybeSExtLHS) &&<br>
+ match(FalseVal, m_Neg(m_Specific(TrueVal)))) ||<br>
+ (match(FalseVal, MaybeSExtLHS) &&<br>
+ match(TrueVal, m_Neg(m_Specific(FalseVal))))) {<br>
+ // Set LHS and RHS so that RHS is the negated operand of the select<br>
+ if (match(TrueVal, MaybeSExtLHS)) {<br>
+ LHS = TrueVal;<br>
+ RHS = FalseVal;<br>
+ } else {<br>
+ LHS = FalseVal;<br>
+ RHS = TrueVal;<br>
+ }<br>
<br>
// ABS(X) ==> (X >s 0) ? X : -X and (X >s -1) ? X : -X<br>
// NABS(X) ==> (X >s 0) ? -X : X and (X >s -1) ? -X : X<br>
if (Pred == ICmpInst::ICMP_SGT &&<br>
(C1->isNullValue() || C1->isAllOnesValue())) {<br>
- return {(CmpLHS == TrueVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};<br>
+ return {(LHS == TrueVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};<br>
}<br>
<br>
// ABS(X) ==> (X <s 0) ? -X : X and (X <s 1) ? -X : X<br>
// NABS(X) ==> (X <s 0) ? X : -X and (X <s 1) ? X : -X<br>
if (Pred == ICmpInst::ICMP_SLT &&<br>
(C1->isNullValue() || C1->isOneValue())) {<br>
- return {(CmpLHS == FalseVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};<br>
+ return {(LHS == FalseVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};<br>
}<br>
}<br>
}<br>
<br>
Modified: llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll?rev=333927&r1=333926&r2=333927&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll?rev=333927&r1=333926&r2=333927&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll (original)<br>
+++ llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll Mon Jun 4 09:53:57 2018<br>
@@ -199,3 +199,48 @@ while.cond:<br>
while.end: ; preds = %while.cond<br>
ret i32 %i.0<br>
}<br>
+<br>
+; Recognize CTLZ builtin pattern.<br>
+; Here it will replace the loop -<br>
+; assume builtin is always profitable.<br>
+;<br>
+; int ctlz_sext(short in)<br>
+; {<br>
+; int n = in;<br>
+; if (in < 0)<br>
+; n = -n;<br>
+; int i = 0;<br>
+; while(n >>= 1) {<br>
+; i++;<br>
+; }<br>
+; return i;<br>
+; }<br>
+;<br>
+; ALL: entry<br>
+; ALL: %0 = ashr i32 %abs_n, 1<br>
+; ALL-NEXT: %1 = call i32 @llvm.ctlz.i32(i32 %0, i1 false)<br>
+; ALL-NEXT: %2 = sub i32 32, %1<br>
+; ALL-NEXT: %3 = add i32 %2, 1<br>
+; ALL: %i.0.lcssa = phi i32 [ %2, %while.cond ]<br>
+; ALL: ret i32 %i.0.lcssa<br>
+<br>
+; Function Attrs: norecurse nounwind readnone uwtable<br>
+define i32 @ctlz_sext(i16 %in) {<br>
+entry:<br>
+ %n = sext i16 %in to i32<br>
+ %c = icmp sgt i16 %in, 0<br>
+ %negn = sub nsw i32 0, %n<br>
+ %abs_n = select i1 %c, i32 %n, i32 %negn<br>
+ br label %while.cond<br>
+<br>
+while.cond: ; preds = %while.cond, %entry<br>
+ %n.addr.0 = phi i32 [ %abs_n, %entry ], [ %shr, %while.cond ]<br>
+ %i.0 = phi i32 [ 0, %entry ], [ %inc, %while.cond ]<br>
+ %shr = ashr i32 %n.addr.0, 1<br>
+ %tobool = icmp eq i32 %shr, 0<br>
+ %inc = add nsw i32 %i.0, 1<br>
+ br i1 %tobool, label %while.end, label %while.cond<br>
+<br>
+while.end: ; preds = %while.cond<br>
+ ret i32 %i.0<br>
+}<br>
<br>
Modified: llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll?rev=333927&r1=333926&r2=333927&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll?rev=333927&r1=333926&r2=333927&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll (original)<br>
+++ llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll Mon Jun 4 09:53:57 2018<br>
@@ -200,6 +200,51 @@ while.end:<br>
ret i32 %i.0<br>
}<br>
<br>
+; Recognize CTLZ builtin pattern.<br>
+; Here it will replace the loop -<br>
+; assume builtin is always profitable.<br>
+;<br>
+; int ctlz_sext(short in)<br>
+; {<br>
+; int n = in;<br>
+; if (in < 0)<br>
+; n = -n;<br>
+; int i = 0;<br>
+; while(n >>= 1) {<br>
+; i++;<br>
+; }<br>
+; return i;<br>
+; }<br>
+;<br>
+; ALL: entry<br>
+; ALL: %0 = ashr i32 %abs_n, 1<br>
+; ALL-NEXT: %1 = call i32 @llvm.ctlz.i32(i32 %0, i1 false)<br>
+; ALL-NEXT: %2 = sub i32 32, %1<br>
+; ALL-NEXT: %3 = add i32 %2, 1<br>
+; ALL: %i.0.lcssa = phi i32 [ %2, %while.cond ]<br>
+; ALL: ret i32 %i.0.lcssa<br>
+<br>
+; Function Attrs: norecurse nounwind readnone uwtable<br>
+define i32 @ctlz_sext(i16 %in) {<br>
+entry:<br>
+ %n = sext i16 %in to i32<br>
+ %c = icmp sgt i16 %in, 0<br>
+ %negn = sub nsw i32 0, %n<br>
+ %abs_n = select i1 %c, i32 %n, i32 %negn<br>
+ br label %while.cond<br>
+<br>
+while.cond: ; preds = %while.cond, %entry<br>
+ %n.addr.0 = phi i32 [ %abs_n, %entry ], [ %shr, %while.cond ]<br>
+ %i.0 = phi i32 [ 0, %entry ], [ %inc, %while.cond ]<br>
+ %shr = ashr i32 %n.addr.0, 1<br>
+ %tobool = icmp eq i32 %shr, 0<br>
+ %inc = add nsw i32 %i.0, 1<br>
+ br i1 %tobool, label %while.end, label %while.cond<br>
+<br>
+while.end: ; preds = %while.cond<br>
+ ret i32 %i.0<br>
+}<br>
+<br>
; This loop contains a volatile store. If x is initially negative,<br>
; the code will be an infinite loop because the ashr will eventually produce<br>
; all ones and continue doing so. This prevents the loop from terminating. If<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><o:p></o:p></p>
</blockquote>
</div>
<p class="MsoNormal"><br clear="all">
<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal">-- <o:p></o:p></p>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">Regards,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Ilya Biryukov<o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
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.
</body>
</html>