<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:"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;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
span.EmailStyle19
        {mso-style-type:personal-reply;
        font-family:"Courier New";
        color:#44546A;
        font-weight:normal;
        font-style:normal;
        text-decoration:none none;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@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-family:"Courier New";color:#44546A"><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-family:"Courier New";color:#44546A"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-left:.5in"><b>From:</b> llvm-commits <llvm-commits-bounces@lists.llvm.org>
<b>On Behalf Of </b>Sanjay Patel via llvm-commits<br>
<b>Sent:</b> Friday, November 20, 2020 3:16 PM<br>
<b>To:</b> Eric Christopher <echristo@gmail.com><br>
<b>Cc:</b> Mehdi AMINI <reviews+D90554+public+a451c79bd8cad907@reviews.llvm.org>; Fangrui Song <maskray@google.com>; nikic@php.net; llvm-commits <llvm-commits@lists.llvm.org>; dougpuob@gmail.com; sam.parker@arm.com; 88888yl@gmail.com; Kristof Beyls <kristof.beyls@arm.com>;
 higuoxing@gmail.com<br>
<b>Subject:</b> Re: [PATCH] D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
<p style="margin-left:.5in"><b><i><span style="font-size:12.0pt;font-family:"Arial",sans-serif;color:red">EXTERNAL</span></i></b><span style="font-size:12.0pt;font-family:"Arial",sans-serif;color:red">
</span><o:p></o:p></p>
<div>
<div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal" style="margin-left:.5in">On Fri, Nov 20, 2020 at 2:07 PM Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal" style="margin-left:.5in">On Fri, Nov 20, 2020 at 1:44 PM Mehdi AMINI via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal" style="margin-left:.5in">mehdi_amini added a comment.<br>
<br>
In D90554#2408163 <<a href="https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD90554%232408163&data=04%7C01%7Ckevin.neal%40sas.com%7Ca394536816e94978aa4b08d88d912fd8%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637415001999363800%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=07Rq%2FkLxVxS6bLZdLE966u1nZXwspCWP3pbyu7reyiI%3D&reserved=0" target="_blank">https://reviews.llvm.org/D90554#2408163</a>>,
 @spatel wrote:<br>
<br>
> So to clarify policy, we reverted patches when (1) a test based on an experimental intrinsic was crashing and (2) that crash is easily reproducible independent of this patch as shown here:<br>
> 7ae346434 <<a href="https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FrG7ae346434a5f51b81ebaeeb50bd5d97666ee288b&data=04%7C01%7Ckevin.neal%40sas.com%7Ca394536816e94978aa4b08d88d912fd8%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637415001999363800%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Liq%2BFs0ivTaVQ9qcjeKb0j9mRlb3Q9clzWZrCEARJkk%3D&reserved=0" target="_blank">https://reviews.llvm.org/rG7ae346434a5f51b81ebaeeb50bd5d97666ee288b</a>><br>
><br>
> I understand that we revert first and ask questions later, but should that be the rule for experimental code?<br>
<br>
In that case it seems like there was user-visible impact for some clang user according to @brooksmoses comment above? It isn't like an arbitrary Fuzzer was plugged to the system. IMO a revert is "low cost" and easy to re-land with a fix here, little downside
 to do this?<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">Appreciated for sure. I'm not sure how "experimental" we want to consider the rounding math intrinsics at this point. It's an interesting question and probably should be raised on llvm-dev. If it were something
 not user visible or really experimental I probably wouldn't have reverted, but given it's a fairly common use case for developers to use the option it seemed on the "not really as experimental as it sounds" area. I think the intrinsics themselves might be
 experimental in the "we could change these radically" rather than "we don't really expect these to work".<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">Yeah, I agree. The fixes in this case were easy because I could just about tell from the assert and backtrace where the problem was, but it might be more controversial for some theoretical future problem.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">So maybe the real question is when to remove the "experimental" label. For example, we went years with the reduction intrinsics and recently promoted them because it was clear they had become part of the standard
 world. If the constrained intrinsics are already in mainstream use via clang, then we should treat them as 1st class IR?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"> <o:p></o:p></p>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A">Well, the experimental constrained intrinsics only work on X86, PowerPC, and SystemZ currently for starters. The metadata in the intrinsics is not set correctly by clang in some cases
 if you use any relevant math #pragma. There are rules that are supposed to be followed, but we aren’t enforcing them yet in LLVM because clang isn’t ready. And the inliner isn’t fixed to handle them yet.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A">Also, clang’s versions of isnan(), fpclassify(), and a few others will trap if given an SNaN despite IEEE 754 saying they shouldn’t. I’m not sure how to fix that, and it may mean the
 addition of more constrained intrinsics.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A">So I don’t think they should be moved out of experimental until we have a clang+llvm implementation that is much closer to complete.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New";color:#44546A">--</span><span style="color:#44546A">
<br>
</span><span style="font-size:10.0pt;font-family:"Courier New";color:#44546A">Kevin P. Neal</span><span style="color:#44546A"><br>
</span><span style="font-size:10.0pt;font-family:"Courier New";color:#44546A">SAS/C and SAS/C++ Compiler<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New";color:#44546A">Compute Services<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Courier New";color:#44546A">SAS Institute, Inc.</span><span style="color:#44546A"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546A"><o:p> </o:p></span></p>
</div>
</body>
</html>