<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;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.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" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><o:p> </o:p></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>Nikita Popov via llvm-commits<br>
<b>Sent:</b> Thursday, May 5, 2022 11:44 AM<br>
<b>To:</b> Philip Reames <listmail@philipreames.com><br>
<b>Cc:</b> Serge Pavlov <llvmlistbot@llvm.org>; llvm-commits <llvm-commits@lists.llvm.org><br>
<b>Subject:</b> Re: [llvm] 83914ee - [InstCombine] Remove side effect of replaced constrained intrinsics<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
<p align="center" style="margin-left:.5in;text-align:center"><strong><span style="font-size:10.5pt;font-family:"Arial",sans-serif;color:black;background:yellow">WARNING:</span></strong><span style="font-size:10.5pt;font-family:"Arial",sans-serif;color:black;background:yellow">
This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.</span><o:p></o:p></p>
<div>
<div>
<div>
<div>
<p class="MsoNormal" style="margin-left:.5in">On Thu, May 5, 2022 at 6:17 PM Philip Reames 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:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<p style="margin-left:.5in"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="margin-left:.5in">On 5/5/22 09:14, Serge Pavlov wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<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">This looks incorrect to me. SimplifyCall returns a Value* for a<br>
simplified expression and you're interpreting that as a boolean meaning<br>
"delete call".<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">In this case SimplifyCall is called for a call, which is not used and was not removed only because it has declared side effect. Non-null replacement returned by SimplifyCall is used as an indicator that the call
may be simplified, so its side effect is absent or may be ignored. Such unused calls appear as a result of other transformations, they are not removed due to the side effect and produce useless instructions in resulting code.
<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<p style="margin-left:.5in">My point is that SimplifyCall is allowed to simplify one side effecting call to another side effecting call. As in, you can't make assumptions about *which* simplifications SimplifyCall has made. If it returns a nonnull result,
you must use it (or leave the instruction unmodified). <o:p></o:p></p>
</div>
</blockquote>
<div>
<p class="MsoNormal" style="margin-left:.5in">Hey Philip,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">This is not quite how InstSimplify works: It returns a refinement of the instruction return value. The simplification result has no relation to modelling of side effects.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">For example, let's say you have "%x = call i32 @foo(i32 returned %y)" then InstSimplify could simplify %x to %y, because the return value of the call is known, regardless of any side-effects it may have. Replacing
uses of %x with %y is legal, but removing the call is not. (InstSimplify doesn't actually perform this fold because of reasons related to CGSSC passes, but it could perform it, and serves as a simple illustration.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">Of course, this also means that generally you can't say that "because SimplifyCall returned something, you can drop the call" -- that would be obviously illegal for the aforementioned fold.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">What this patch is doing is to say that for constrained FP intrinsics in particular, SimplifyCall gains an additional contract that InstSimplify does not generally have: It will only return a value if the constrained
FP intrinsic is side-effect free.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">The reason for this is pragmatic, as determining whether a constrained FP call is side-effect free is essentially the same as determining whether it can be folded. As such, this patch tries to piggy-back on the
existing simplification logic to determine whether the call can also be removed. This somewhat dirties the API contract, but avoids the need to reimplement folding logic in wouldInstructionBeTriviallyDead().<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-left:.5in">At least that is my understanding of the situation here.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Maybe it makes sense to split off “FoldConstrainedFP” into a separate method, just to avoid confusion. It would do the same thing SimplifyCall currently does for constrained FP, just without tying it to any assumption about the way SimplifyInstruction
works in general.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">-Eli<o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</body>
</html>