<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: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;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
p.gmail-m-6952061586146863985msolistparagraph, li.gmail-m-6952061586146863985msolistparagraph, div.gmail-m-6952061586146863985msolistparagraph
{mso-style-name:gmail-m_-6952061586146863985msolistparagraph;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;
font-weight:normal;
font-style:normal;
text-decoration:none none;}
.MsoChpDefault
{mso-style-type:export-only;
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;}
/* List Definitions */
@list l0
{mso-list-id:606501847;
mso-list-template-ids:-1840604776;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></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"><span style="color:#1F497D">I think the use of target option simply owe to the fact that DAG and other backend passes always lose the FMFs during transformation in the past.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Thanks<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Phoebe (Pengfei)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <llvm-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Renato Golin via llvm-dev<br>
<b>Sent:</b> Saturday, October 30, 2021 9:30 PM<br>
<b>To:</b> Kaylor, Andrew <andrew.kaylor@intel.com><br>
<b>Cc:</b> Yaxun Liu <yaxun.liu@amd.com>; Sebastian Pop <sebpop@gmail.com>; llvm-dev@lists.llvm.org; Ammarguellat, Zahira <zahira.ammarguellat@intel.com>; Ulrich Weigand <Ulrich.Weigand@de.ibm.com>; Ballman, Aaron <aaron.ballman@intel.com><br>
<b>Subject:</b> Re: [llvm-dev] [RFC] Eliminating non-IR floating-point controls in the selection DAG<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Fri, 29 Oct 2021 at 21:51, Kaylor, Andrew <<a href="mailto:andrew.kaylor@intel.com">andrew.kaylor@intel.com</a>> wrote:<o:p></o:p></p>
</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">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">> Finally, I think we need to separate the IR from DAG/MIR behaviour. It seems to me that the target option is what overrides the behaviour, not function/module options, so we should
worry about the targets' behaviour, not at which level the flag is<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
</div>
</blockquote>
<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="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I don’t know what you’re saying here. What do you mean by “we should worry about the targets' behaviour”? If the IR says fp-contract is not allowed for a given instruction, why
should the target options be allowed to overrule that?<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">They shouldn't. Unless it's a hardware-specific design issue (ex. there is no fused op, or there is no un-fused op), but that should be done at the DAG level, I think, not IR level.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><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="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Two notes, however:<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<ol start="1" type="1">
<li class="gmail-m-6952061586146863985msolistparagraph" style="mso-list:l0 level1 lfo1">
resetTargetOptions doesn’t handle the AllowFPOpFusion setting (as far as I can tell, there is no corresponding function attribute)<o:p></o:p></li><li class="gmail-m-6952061586146863985msolistparagraph" style="mso-list:l0 level1 lfo1">
There is a comment for resetTargetOptions which says this:<o:p></o:p></li></ol>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;margin-left:.5in">
<span style="font-size:10.0pt;font-family:"Courier New"">// FIXME: This function needs to go away for a number of reasons:</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;margin-left:.5in">
<span style="font-size:10.0pt;font-family:"Courier New"">// a) global state on the TargetMachine is terrible in general,</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;margin-left:.5in">
<span style="font-size:10.0pt;font-family:"Courier New"">// b) these target options should be passed only on the function</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;margin-left:.5in">
<span style="font-size:10.0pt;font-family:"Courier New"">// and not on the TargetMachine (via TargetOptions) at all.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">As you may guess, I completely agree with the comment. FWIW, this comment was added by Eric Christopher in 2015 (<a href="https://github.com/llvm/llvm-project/commit/35a8a62125ccfa0ef804584738a295ed5c17750f" target="_blank">https://github.com/llvm/llvm-project/commit/35a8a62125ccfa0ef804584738a295ed5c17750f</a>).<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I think the biggest problem there is that this is a target flag not an IR flag and that's not a good design. I completely agree with the comment also.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I also agree the dance of inst/func/mod level flags isn't trivial and it a source of at least confusion, but potentially bugs.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I also want to clarify what I meant for "perfectly valid", and that was "it solves the problem and is technically doable". I didn't want to imply it was good or there wasn't a better way of doing it.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><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="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The core of my complaint about the current state of things is that the backend is treating the fast-math flags as if they are “on or undecided” rather than “on or off.” IMO, the
absence of a fast-math flag in the IR means that the behavior controlled by that flag is not permitted. That’s the way the IR is treated in IR-level optimization passes, but the backend codegen (at least in places) behaves as though the absence of a fast-math
flag means “not permitted unless enabled by TargetOptions.” That’s bad as a starting point, but it’s particularly bad when you start linking together IR from multiple modules that may have been created from different compilation units with different options.<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I absolutely agree with you. That's why I've added Sebastian, who worked on a very similar problem not long ago. I myself have fixed similar problems in the Arm backend a long time ago.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I think we really should treat the lack of fast-math-like flags to mean their behaviour is *forbidden*, and force front-ends and middle-end passes to add them if the user requested / allowed.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><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="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I’d also like to make one thing explicit. While the change I’m proposing would change the behavior of the backend for all front ends, it wouldn’t “break” them in the sense of producing
incorrect results. When TargetOptions::AllowFPOpFusion==FPOpFusionMode::Fast the backend is *<b>allowed</b>* to fuse operations, but it isn’t required or guaranteed to fuse them. With the change I am proposing, the backend wouldn’t form fuse FP options unless
the ‘contract’ flag were set, but that wouldn’t lead to incorrect results. It would lead to lost performance in some cases, but I think that can be recovered by generating IR with the semantics that were intended. Similarly, for the other fast-math flags.
However, I will admit that it could be an uncomfortable transition for some front ends.<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">It's not that simple. We want to keep the performance for existing code with existing compilation infrastructure as much as possible.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">When an existing project with its existing compilation flags use a new version of clang and suddenly the performance is considerably lower, they may start to look for things to change or may just blame on "new compiler" and try other compilers,
or worse, get stuck with an old version of clang.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">But this is also not a huge deal, and we can fix that with more clear communication...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><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="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Regarding bloating the IR, I’m not sure if this is a theoretical problem or real one. The bitcode writer will emit a byte for the fast-math flags if any are set for each call or
FP operation, and if none are set it will not emit that byte. For clang, this is a non-issue because clang always sets the fast-math flags on the individual operations. It’s at least theoretically possible that there is some front end that never sets the fast-math
flags and relies on the global setting. In that case, adding the fast-math flags would bloat the IR considerably. For most of the FMF, omitting the flags would cause considerable missed optimization opportunities in the middle end, but the ‘contract’ flag
is a bit of a special case because it isn’t typically used at all before codegen.<o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">My comment on the increase in size was meant as a check, not as a blocker. It could be well within noise levels and totally worth the change.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<div>
<p class="MsoNormal">cheers,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">--renato<o:p></o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>