[llvm-dev] Condition code in DAGCombiner::visitFADDForFMACombine?

[Nicolai Hähnle via llvm-dev]

On 22.08.2018 17:52, Ryan Taylor wrote:
> This is probably going to effect on other backends and break llvm-lit 
> for them?

Very likely, yes. Can you take a look at how big the fallout is? This 
might give us a hint about what other frontends might expect, and who 
needs to be involved in the discussion (if one is needed).

Cheers,
Nicolai


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


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.