<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=us-ascii"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:Helvetica;
panose-1:2 11 6 4 2 2 2 2 2 4;}
@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:"Book Antiqua";
panose-1:2 4 6 2 5 3 5 3 3 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 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;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Book Antiqua",serif;
color:#943634;
font-weight:normal;
font-style:normal;
text-decoration:none none;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@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-IE link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-family:"Book Antiqua",serif;color:#943634;mso-fareast-language:EN-US'>I like this suggestion<o:p></o:p></span></p><p class=MsoNormal><span style='font-family:"Book Antiqua",serif;color:#943634;mso-fareast-language:EN-US'><o:p> </o:p></span></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri",sans-serif'>From:</span></b><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri",sans-serif'> llvm-dev [mailto:llvm-dev-bounces@lists.llvm.org] <b>On Behalf Of </b>Mehdi Amini via llvm-dev<br><b>Sent:</b> 22 April 2016 18:41<br><b>To:</b> Sanjay Patel <spatel@rotateright.com><br><b>Cc:</b> llvm-dev <llvm-dev@lists.llvm.org>; cfe-dev@lists.llvm.org<br><b>Subject:</b> Re: [llvm-dev] [RFC] remove the llvm.expect intrinsic<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal><o:p> </o:p></p><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On Apr 22, 2016, at 10:39 AM, Mehdi Amini <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt;font-variant-caps: normal;orphans: auto;text-align:start;widows: auto;-webkit-text-stroke-width: 0px;word-spacing:0px'><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br>On Apr 22, 2016, at 10:27 AM, Sanjay Patel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></span></p></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p><div><div><div><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>On Fri, Apr 22, 2016 at 10:39 AM, Philip Reames<span class=apple-converted-space> </span><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>><span class=apple-converted-space> </span>wrote:<o:p></o:p></span></p><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><div><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>On 04/22/2016 09:20 AM, Sanjay Patel via llvm-dev wrote:<o:p></o:p></span></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><div><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>I've proposed removing the llvm.expect intrinsic:<br><a href="http://reviews.llvm.org/D19300" target="_blank">http://reviews.llvm.org/D19300</a><o:p></o:p></span></p></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>The motivation for this change is in:<br><a href="http://reviews.llvm.org/D19299" target="_blank">http://reviews.llvm.org/D19299</a><br><br>For reference:<br>1. We created an intrinsic that's only reason for existing is to improve perf, but the intrinsic can harm optimization by interfering with transforms in other passes.<o:p></o:p></span></p></div></div></blockquote><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>I don't follow this at all. Given expects are eagerly lowered to metadata, where's the interaction? In particular, the expect lowering overrules any metadata on the associated branch/switch. This is exactly what you'd expect for a user annotation interacting with PGO data.<o:p></o:p></span></p></div></blockquote><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>I agree that a user annotation should override PGO data.<o:p></o:p></span></p></div></div></div></div></div></blockquote><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>PGO is also a user input: the user is basically saying "I want the code to be optimized for *this* use case".<o:p></o:p></span></p></div><div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>So interestingly I would have thought the opposite: PGO overrides the source code annotation.<o:p></o:p></span></p></div></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>Here are a couple of reasons why:<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'> - libraries can be used by different client and what is common in one case might not for another.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'> - code evolves, and user can fail to revisit assumption about the common case<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'> - the user can be wrong, PGO should not (?).<o:p></o:p></span></p></div></div></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>For this last point, whatever information prevails in the end, it may be valuable to report to the user some optimization hints about the mismatch between the PGO measurement and the annotation.<o:p></o:p></p></div><p class=MsoNormal><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>-- <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>Mehdi<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br style='font-variant-caps: normal;orphans: auto;text-align:start;widows: auto;-webkit-text-stroke-width: 0px;word-spacing:0px'><br></span><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><div><div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>This is why I initially proposed that builtin_expect() would extend on '[un]predictable' metadata rather than 'prof' metadata in D19299.<span class=apple-converted-space> </span><br><br>But I think that's a separate discussion now; we use 'prof' metadata for this purpose today, and I'm not trying to change that in these patches. If the user-specified representation of builtin_expect() is overwritten by PGO data, I think that's a bug independent of the current proposal/patches.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br> <o:p></o:p></span></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'><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br><br><o:p></o:p></span></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p><div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>2. To solve that, we created a pass to always transform the intrinsic into metadata at a very early stage in LLVM. But now every program is paying a compile-time tax (running the LowerExpectIntrinsic pass) for a feature that is rarely used.<o:p></o:p></span></p></div></div></blockquote><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>Er, what cost? Given this is a single linear pass over the IR - and could actually be made essentially free by checking to see if the module has any uses of expect - I'm suspicious of this compile time argument. Have you actually seen this in profiles? <span class=apple-converted-space> </span><o:p></o:p></span></p></div></blockquote><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div><div><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>No - I expect the actual overhead is in the noise. The real objection to the LowerExpectIntrinsic pass is that it is completely unnecessary. This was raised in the original review. We shouldn't have two mechanisms to represent exactly the same thing. This is also why my initial implementation for builtin_unpredictable() was rejected:<br><a href="http://reviews.llvm.org/D12341">http://reviews.llvm.org/D12341</a><o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>I assumed there was some good reason for LowerExpectIntrinsic to exist, so I copied that design. As noted in that review, my assumption was wrong. And it was suggested then that we should remove LowerExpectIntrinsic but nobody had gotten around to it. With these patches, I'd like to finally fix this.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br><br><br> <o:p></o:p></span></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'><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br>A possible front-end replacement transformation for a source-level "builtin_expect()" is in D19299: I think a front-end can always directly create metadata for this kind of programmer hint rather than using an intermediate representation (the intrinsic). Likewise, if there's some out-of-tree IR pass that is creating an llvm.expect, it should be able to create branch weight metadata directly instead.<o:p></o:p></span></p></div></div></blockquote><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>This seems like a reasonable proposal. The expect intrinsic does give us a mechanism to express value profiling predictions, but we don't appear to actually use that today. My bias would be to leave it in place, but I'm not going to object strongly if the consensus goes the other way. <span class=apple-converted-space> </span><br><br><o:p></o:p></span></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br>Please let me know if you see any problems with this proposal or the patches.<o:p></o:p></span></p></div><div><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>For reference, here's the original post-commit review thread for llvm.expect:<br><a href="https://marc.info/?l=llvm-commits&m=130997676129580&w=4" target="_blank">https://marc.info/?l=llvm-commits&m=130997676129580&w=4</a><o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br><br><o:p></o:p></span></p><pre>_______________________________________________<o:p></o:p></pre><pre>LLVM Developers mailing list<o:p></o:p></pre><pre><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><o:p></o:p></pre><pre><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><o:p></o:p></pre></blockquote><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div></blockquote></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><o:p> </o:p></span></p></div></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><o:p></o:p></span></p></div></blockquote></div></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></body></html>