<div dir="ltr"><div dir="ltr">I have 1 concern about adding an explicit fneg op to IR:</div><div dir="ltr"><br> </div><div dir="ltr">Currently, fneg qualifies as a binop in IR (since there's no other way to represent it), and we have IR transforms that are based on matching that pattern (m_BinOp). With a proper unary fneg instruction, those transforms are not going to fire anymore. So I think we'll need to duplicate code to account for that difference (or hack the matcher to include fneg?). <br></div><div dir="ltr"><br></div><div dir="ltr">The existing regression tests probably aren't going to show optimization failures for those cases*, so I'm not sure how to detect regressions without doing an audit of FP transforms in instcombine and other passes.<br></div><div dir="ltr"><br></div><div>* Are we proposing to canonicalize "fsub -0.0, X --> fneg X"? As shown in the example below, I think that's allowed. If we do that, then we might detect some of the affected transforms, but I suspect that we don't have that test coverage for most transforms.<br></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 11, 2018 at 1:46 PM James Y Knight via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>+1 for an explicit FNEG instruction, since as previously discussed, it has stricter requirements for what value may be returned by the operation. And strengthening the requirement on FSUB is not feasible when the values are variables rather than literals.<br></div><div><br></div><div>That is:</div><div><div>FSUB(-0.0,  NaN) = either NaN *or* -NaN</div><div>FSUB(-0.0, -NaN) = either NaN *or* -NaN</div></div><div><br></div><div>FNEG(NaN) = -NaN</div><div>FNEG(-NaN) = NaN</div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 11, 2018 at 3:35 PM Cameron McInally via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">On Tue, Sep 11, 2018 at 2:45 PM, Kevin Neal via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div link="blue" vlink="purple" lang="EN-US">
<div class="m_7747937933675320482m_-6662446747121914135m_8039835832665117577m_-3339098687060147358m_-6937496685457975450WordSection1">
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">Which exactly was the plan?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">Add a new, regular instruction?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">Add a new constrained math intrinsic?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">Both?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u> </span></p></div></div></blockquote><div><br></div><div>I'd like to add an explicit FNEG IR operator. We would not need a constrained FNEG operator if we go this route.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div link="blue" vlink="purple" lang="EN-US"><div class="m_7747937933675320482m_-6662446747121914135m_8039835832665117577m_-3339098687060147358m_-6937496685457975450WordSection1"><p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">Andrew Kaylor made a good point here:<u></u><u></u></span></p>
<ul style="margin-top:0in" type="disc">
<li class="m_7747937933675320482m_-6662446747121914135m_8039835832665117577m_-3339098687060147358m_-6937496685457975450MsoPlainText">As I said, all LLVM IR FP instructions are //assumed// to have no side effects. I'm not sure we want an instruction that goes beyond this to be //defined// as having no side effects. It adds a complication
 to the language and introduces restrictions on the code generator that aren't needed in the ordinary case of non-constrained FP.  The target code generators are free to do anything they want with the other FP instructions, including things that introduce new
 FP status flags being set that otherwise wouldn't be, and for the normal case the back ends should be free to do that with fneg as well.<u></u><u></u></li></ul>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">Personally, I’m not sure I like the idea of having exceptions to the rule that FP instructions also have constrained versions. So I lean towards having both a regular FNEG and a constrained
 version.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-family:"Courier New";color:#44546a">But I think I remember pushback. I can’t put my fingers on the message, though.</span></p></div></div></blockquote><div><br></div><div>We touched on this in the Differential Review and on this thread. To summarize:</div><div><br></div><div>FNEG(X) is not really the same operation as FSUB(-0.0, X), although the differences are admittedly subtle. I even went as far to say that any xforms between the two operations should only occur under FastMath conditions. If we follow those rules, I think emergence guarantees that we don't have to worry about the side effects of FNEG (please correct me if I've missed something).</div><div><br></div><div>Extending on that, I suspect that we should not be canonicalizing FNEG(X) as FSUB(-0.0, X), but rather as XOR(X, 1 << (SIZE_OF_TYPE_IN_BITS - 1)). A utility function could provide us with the sign-bit constant, so it's not that ugly.</div><div><br></div><div>That said, I agree that Andrew's take is compelling. And I would be okay with adding a constrained FNEG to solve the immediate issue, if that is the final decision.</div></div></div></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>