[llvm-dev] Condition code in DAGCombiner::visitFADDForFMACombine?
Nicolai Hähnle via llvm-dev
llvm-dev at lists.llvm.org
Thu Aug 23 13:35:41 PDT 2018
I think what Michael is saying that:
1. Don't set the global flag.
2. Set `contract` and `reassoc` on *all* floating point instructions by
default.
3. Don't set those flags on instructions with NoContraction in SPIR-V.
I agree with the general thrust of this approach, except for the
significant problem that it currently doesn't work.
If we follow 1-3 above, then SPIR-V fmul+fadd with NoContraction set on
the fmul will be translated into LLVM IR fmul+fadd with contract +
reassoc set on the fadd (but *not* on the fmul).
Unfortunately, LLVM will contract those to an fma, where SPIR-V very
explicitly says that that's forbidden.
Sanjay & Michael, preferably we'd just align LLVM IR semantics here with
SPIR-V. LangRef is very vague about what `contract` means *exactly*, and
it does make sense to say that if you want to contract N instructions
into a single instruction, then *all* those instructions must have
`contract` set.
If there's an argument against making this change, it'd be good if you
could spell it out.
Cheers,
Nicolai
On 23.08.2018 21:47, Ryan Taylor wrote:
> 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
> <mailto: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
>> <mailto: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 <mailto: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
>>> <mailto: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 <mailto: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
>>> <mailto: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>
>>> <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.
>>> _______________________________________________
>>> 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.
More information about the llvm-dev
mailing list