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

[Ryan Taylor via llvm-dev]

This is probably going to effect on other backends and break llvm-lit for
them?

On Wed, Aug 22, 2018 at 11:41 AM Nicolai Hähnle <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>> 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>>> 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>
> >      > 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>
> >     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180822/1b71ed7c/attachment.html>