<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 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
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-size:10.0pt;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
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-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">That’s an interesting case
</span><span style="font-size:11.0pt;font-family:Wingdings;color:#1F497D">J</span><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"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Sorry, that should be sdiv/srem -> udiv/urem.
<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">Also I was wrong with the loop-rotate I metioned. Loop-rotate is actually in the pipeline before indvars. In D18867, adding loop-rotate before indvars just happened
to work for those 2 test cases, but it doesn’t solve the fundamental problem. The fundamental problem is that SCEV’s isKnownPredicate lacks the ability to recognize some NSW additions as non-negative. The initial change in D18867 used isKnownNonNegative
of ValueTracking to assist isKnownPredicate, and fixed the FE test. But this is sort of hacky, and Sanjoy suggested we should teach isKnownPredicate to recognize NSW additions. Unfortunately, I haven’t found a good solution. There are some discussions/analysis
in the comments of D18867.<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">In D21773, I changed all indices to “i + 1” because isKnownPredicate could recognize “i +nsw 1” as non-negative, but not “i +nsw 2” or “i +nsw 3”, then this test
passes with the change in D18867. We don’t need to change the test if we can solve the “trunc/zext” problem, though.<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"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Craig Topper [mailto:craig.topper@gmail.com]
<br>
<b>Sent:</b> Wednesday, June 29, 2016 3:42 PM<br>
<b>To:</b> Huang, Li1 <li1.huang@intel.com><br>
<b>Cc:</b> Sanjoy Das <sanjoy@playingwithpointers.com>; Adam Nemet <anemet@apple.com>; llvm-commits <llvm-commits@lists.llvm.org><br>
<b>Subject:</b> Re: [llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">sdiv also prevents converting divide by power of 2 into a shift. That's what was happening in the loop I was looking at. For x86 we ended up with a multiple instruction abs idiom, a shift for the divide, and then undoing the abs.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Wed, Jun 29, 2016 at 12:20 PM, Huang, Li1 <<a href="mailto:li1.huang@intel.com" target="_blank">li1.huang@intel.com</a>> wrote:<o:p></o:p></p>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal">Actually, D18777 is doing the same thing as Craig's change, and is based on the same motivation: udiv/urem -> sdiv/srem. Udiv/urem is generally faster than sdiv/srem on most archs.<br>
<br>
The reason it breaks the FE test is that it converts some sexts to zexts before indvars (during instcombine), but indvars cannot recognize that the zext is unnecessary and should be removed. Instead, indvars inserts truncs before zexts after widening the IV.<br>
<br>
D18867 was proposed to fix this problem, this change teaches indvars that zext of a signed value which is known non-negative is unnecessary. However, this change itself is not enough, a loop-rotate pass is needed before indvars for this change to work.<br>
<br>
The test uses O1 but O1 pipeline doesn't have a loop-rotate before indvars, neither does O2 or O3. So I proposed to change the test in D21773.<br>
<br>
Thanks,<br>
Li Huang<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><br>
-----Original Message-----<br>
From: Sanjoy Das [mailto:<a href="mailto:sanjoy@playingwithpointers.com">sanjoy@playingwithpointers.com</a>]<br>
Sent: Wednesday, June 29, 2016 12:35 AM<br>
To: Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>><br>
Cc: Adam Nemet <<a href="mailto:anemet@apple.com">anemet@apple.com</a>>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>; Huang, Li1 <<a href="mailto:li1.huang@intel.com">li1.huang@intel.com</a>><br>
Subject: Re: [llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.<br>
<br>
I think I have to redact my "I suspect this isn't anything complicated" tone. :)<br>
<br>
To do the right thing in cases like the one you pastebinned, we'll have to track `IsSigned` on individual `NarrowIVDefUse` instances.<br>
<br>
Then for instance, in<br>
<br>
%i.050 = phi i32 [ %add15, %for.body ], [ 0, %for.body.preheader ]<br>
%idxprom45 = zext i32 %i.050 to i64<br>
%arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom45<br>
%0 = load i32, i32* %arrayidx, align 4<br>
%sub = add nsw i32 %i.050, -1<br>
%idxprom1 = sext i32 %sub to i64<br>
%arrayidx2 = getelementptr inbounds i32, i32* %B, i64 %idxprom1<br>
%1 = load i32, i32* %arrayidx2, align 4<br>
<br>
we'll first replace `%idxprom45` with the zero extended wide IV, and then when we get to `%sub`, we'll replace it with `add %wide.zexted.iv, -1` but the corresponding `NarrowIVDefUse` we push to the worklist will have to remember that this new wide add instruction
is really a `sext` of `%sub` (and accordingly handle its users).<br>
<br>
I'll be on vacation from 30th June to 10th of July, but I should be able to get to this once I'm back (unfortunately, I probably won't have time before I leave). If you want to fix this and have someone else or me (once I'm back) review it that's fine too.<br>
<br>
Thanks,<br>
-- Sanjoy<o:p></o:p></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><br>
<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>
<p class="MsoNormal">~Craig<o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>