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

Ryan Taylor via llvm-dev llvm-dev at lists.llvm.org
Thu Aug 23 12:47:40 PDT 2018


I don't think the global fast math flag should override the NoContraction
decoration as that's mostly the point of that decoration to begin with,  to
have fine granular control while still having a broad sweeping
optimization. Did I miss your point? I feel like I did.

On Thu, Aug 23, 2018, 3:42 PM Michael Berg <michael_c_berg at apple.com> wrote:

> Ryan,
>
> Given that the global fast math flag overrides most fp behavior also
> mapped to IR FMF flags as a control, I think we should remove it from the
> argument.  The Global fast math should override behavior without IR flag
> opposition, as that is its intent under the definition of Unsafe.  Most
> implementations are moving away from the large club of the global flag into
> fine granularity of control on the IR without the Unsafe module level
> environment provided by it.  This would then put the responsibility on
> front ends to mint the proper expression level flags to control the desired
> behavior.
>
> Regards,
> Michael
>
> On Aug 23, 2018, at 12:13 PM, Ryan Taylor <ryta1203 at gmail.com> wrote:
>
> Michael,
>
> From the spec with regards to reassoc:
>
>   – 15225 Include no re-association as a constraint required by the
> NoContraction Decoration.
>
> I don't see a solution given the situation where -fp-contract=fast and we
> want to contract. Furthermore, I think a 'nocontract' flag will allow the
> IR to be more readable in it's intention. The problem is you can have 2 fp
> arith instructions with no contracts and no reassoc with global fast math
> flag set, how can you differentiate between the two instructions when you
> want one to be contract and the other to be no contract?
>
> -Ryan
>
>
>
> On Thu, Aug 23, 2018 at 2:27 PM Michael Berg <michael_c_berg at apple.com>
> wrote:
>
>> Ryan:
>>
>> I think this will take more, here is the SPIR-V relevant text (
>> https://www.khronos.org/registry/spir-v/specs/1.0/SPIRV.pdf ):
>>
>> <start>
>>
>> NoContraction
>>
>> Apply to an arithmetic instruction to indicate the operation cannot be
>> combined with another instruction to form a single operation. For example,
>> if applied to an OpFMul, that multiply can’t be combined with an
>> addition to yield a fused multiply-add operation. Furthermore, such
>> operations are not allowed to reassociate; e.g., add(a + add(b+c)) cannot
>> be transformed to add(add(a+b) + c).
>> <end>
>>
>> The problem is that the spec does not mention the reassoc attribute,
>> where an implication is made to tie both together as one control without
>> specifying precisely that.  I do not think we need to add another flag.  I
>> think we can work this within the definition.  The text implies
>> NoContraction is both contract=off and reassoc=off, baring global context.
>> Since we are testing for both in isContractable in some fmul contexts and
>> always in fadd context, it should suffice. This and the global context
>> should be manageable in a SPIR env.  The above text does not specify if the
>> add should control fusing or not, but leaves it open to interpretation.
>>
>> Our current definition of contract is:
>>
>> ``contract``
>> Allow floating-point contraction (e.g. fusing a multiply followed by an
>> addition into a fused multiply-and-add).
>>
>> Is somewhat loose, but then we do have a good deal of internal context no
>> made evident that exists in the source.
>> Lets put a stronger bound on the problem to solidify the goal.  I believe
>> front ends can be SPIR-V compliant and use the current framework.  It seems
>> more a management of the environment and IR than adding new feature
>> controls.
>>
>> Regards,
>> Michael
>>
>>
>> On Aug 23, 2018, at 10:51 AM, Ryan Taylor <ryta1203 at gmail.com> wrote:
>>
>> Maybe there is a cleaner solution but it seems like adding a 'nocontract'
>> flag is close to the intention of spir-v and is an easy check in the
>> DAGCombiner without breaking anything else and its intentions are very
>> clear.
>>
>> Right now the DAGCombiner logic doesn't seem to be able to handle the
>> case of having fast math globally with instruction level flags to turn off
>> fast math. Right now, either fast math is global and it's assumed
>> everything can be contracted or fast math is not global and we contract if
>> the contract flag is present on the current instruction. I could be missing
>> something.
>>
>> -Ryan
>>
>> On Thu, Aug 23, 2018 at 1:36 PM Sanjay Patel <spatel at rotateright.com>
>> wrote:
>>
>>> If we have this:
>>> r = (X * Y) + Z
>>>
>>> And we want that to become an fma op/node, 'contract' is checked on the
>>> fadd because it is the fadd that's being altered to form the (possibly
>>> non-standard) result. As I think was noted earlier, whether 'contract' is
>>> set on the fmul is irrelevant in our current implementation. This allows
>>> the scenario where a strict fmul was inlined into code with looser FP
>>> semantics, and we're still free to create an fma. If the end value allows
>>> non-standard behavior, then we assume that intermediate ops leading up to
>>> that end value can use non-standard behavior too. (cc'ing Michael Berg who
>>> did a lot of the DAG FMF work recently)
>>>
>>> I'm not familiar with SPIR-V, but it sounds like it has an inverse flag
>>> system to what we have in IR and DAG - ops are presumed contract-able
>>> unless specified with 'no-contract'? Not sure how to resolve that.
>>>
>>> If we want to change the LLVM FMF semantics, then there will be breakage
>>> in the IR optimizer too (at least for 'reassoc'; not sure about
>>> 'contract'). Either way, I agree that we should try to clarify the LangRef
>>> about this because you can't tell how things are supposed to work from the
>>> current description.
>>>
>>>
>>> On Wed, Aug 22, 2018 at 9:41 AM, Nicolai Hähnle via llvm-dev <
>>> llvm-dev at lists.llvm.org> 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.
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180823/42f15acc/attachment-0001.html>


More information about the llvm-dev mailing list