<div dir="ltr">This is probably going to effect on other backends and break llvm-lit for them?</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 22, 2018 at 11:41 AM Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com">nhaehnle@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 22.08.2018 13:29, Ryan Taylor wrote:<br>
> The example starts as SPIR-V with the NoContraction decoration flag on <br>
> the fmul.<br>
> <br>
> I think what you are saying seems valid in that if the user had put the <br>
> flag on the fadd instead of the fmul it would not contract and so in <br>
> this example the user needs to put the NoContraction on the fadd though <br>
> I'm not sure that's a good expectation of the user. On the surface, I <br>
> think that if an operation didn't have the contract flag than it <br>
> wouldn't be contracted, regardless of what flags any other operation has.<br>
<br>
Okay, I see that the SPIR-V spec specifically calls out this example.<br>
<br>
Unless there are conflicting requirements with another frontend, I'd say <br>
we should make sure LLVM is aligned with SPIR-V here. Something along <br>
the lines of (in LangRef):<br>
<br>
``contract``<br>
    Allow floating-point contraction (e.g. fusing a multiply followed by<br>
    an addition into a fused multiply-and-add). This flag must be present<br>
    on all affected instruction.<br>
<br>
And we should probably say the same about ``reassoc`` as well.<br>
<br>
Cheers,<br>
Nicolai<br>
<br>
<br>
<br>
<br>
> <br>
> On Wed, Aug 22, 2018 at 3:55 AM Nicolai Hähnle via llvm-dev <br>
> <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>> wrote:<br>
> <br>
>     On 21.08.2018 16:08, Ryan Taylor via llvm-dev wrote:<br>
>      > So I have a test case where:<br>
>      ><br>
>      > %20 = fmul nnan arcp float %15, %19<br>
>      > %21 = fadd reassoc nnan arcp contract float %20, -1.000000e+00<br>
>      ><br>
>      > is being contracted in DAG to fmad. Is this correct since the<br>
>     fmul has<br>
>      > no reassoc or contract fast math flag?<br>
> <br>
>     By having the reassoc and contract flags on fadd, the frontend is<br>
>     essentially saying "different rounding on the value produced by the<br>
>     fadd<br>
>     is okay".<br>
> <br>
>     So I'd say contracting this to fma is correct.<br>
> <br>
>     Where does this code come from, and why do you think contracting to fma<br>
>     is wrong?<br>
> <br>
>     Cheers,<br>
>     Nicolai<br>
> <br>
> <br>
> <br>
> <br>
>      ><br>
>      > Thanks.<br>
>      ><br>
>      > On Mon, Aug 20, 2018 at 12:56 PM Ryan Taylor <<a href="mailto:ryta1203@gmail.com" target="_blank">ryta1203@gmail.com</a><br>
>     <mailto:<a href="mailto:ryta1203@gmail.com" target="_blank">ryta1203@gmail.com</a>><br>
>      > <mailto:<a href="mailto:ryta1203@gmail.com" target="_blank">ryta1203@gmail.com</a> <mailto:<a href="mailto:ryta1203@gmail.com" target="_blank">ryta1203@gmail.com</a>>>> wrote:<br>
>      ><br>
>      >     I'm curious why the condition to fuse is this:<br>
>      ><br>
>      >     // Floating-point multiply-add with intermediate rounding.<br>
>      >        bool HasFMAD = (LegalOperations &&<br>
>      >     TLI.isOperationLegal(ISD::FMAD, VT));<br>
>      ><br>
>      >     static bool isContractable(SDNode *N) {<br>
>      >        SDNodeFlags F = N->getFlags();<br>
>      >        return F.hasAllowContract() || F.hasAllowReassociation();<br>
>      >     }<br>
>      ><br>
>      >     bool CanFuse = Options.UnsafeFPMath || isContractable(N);<br>
>      >     bool AllowFusionGlobally = (Options.AllowFPOpFusion ==<br>
>      >     FPOpFusion::Fast || CanFuse || HasFMAD);<br>
>      >     // If the addition is not contractable, do not combine.<br>
>      >     if (!AllowFusionGlobally && !isContractable(N))<br>
>      >          return SDValue();<br>
>      ><br>
>      >     Specifically the AllowFusionGlobally, I would have expected<br>
>      >     something more like:<br>
>      ><br>
>      >     bool AllowFusionGlobally = (Options.AllowFPOpFusion ==<br>
>      >     FPOpFusion::Fast && CanFuse && HasFMAD);<br>
>      ><br>
>      >     or at the very least:<br>
>      ><br>
>      >     bool AllowFusionGlobally = ((Options.AllowFPOpFusion ==<br>
>      >     FPOpFusion::Fast || CanFuse) && HasFMAD);<br>
>      ><br>
>      >     It seems that as long as the target can do fmad it does do fmad<br>
>      >     since HasFMAD is true.<br>
>      ><br>
>      >     Thanks.<br>
>      ><br>
>      ><br>
>      ><br>
>      ><br>
>      ><br>
>      > _______________________________________________<br>
>      > LLVM Developers mailing list<br>
>      > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<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>
>      ><br>
> <br>
> <br>
>     -- <br>
>     Lerne, wie die Welt wirklich ist,<br>
>     Aber vergiss niemals, wie sie sein sollte.<br>
>     _______________________________________________<br>
>     LLVM Developers mailing list<br>
>     <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a> <mailto:<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>
> <br>
<br>
<br>
-- <br>
Lerne, wie die Welt wirklich ist,<br>
Aber vergiss niemals, wie sie sein sollte.<br>
</blockquote></div>