<div dir="ltr"><div>I don't think propagating the NoContraction to users is the right thing to do, as you said, it's doesn't really model the spec/intention of NoContraction. The fadd could still possibly be contracted with another operation.</div><div><br></div><div>-Ryan</div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 24, 2018 at 5:50 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 23.08.2018 23:11, Michael Berg wrote:<br>
> Can you do without the use of -fp-contract=fast (Options.AllowFPOpFusion == FPOpFusion::Fast ) and without Unsafe?<br>
> As I SPIR-V’s usage of NoContraction flies in the face of both.<br>
<br>
Yes, of course.<br>
<br>
<br>
> If so, you should be able to get what you want, as then you are down to just IR flags. You will need a model to generate the correct behavior though in your SPIR-V implementation wrt IR flag emissions.<br>
<br>
That's the problem though, isn't it?<br>
<br>
Of course we could start out by saying that NoContraction on an fmul <br>
"poisons" its users, i.e. if the value is used by an fadd, then the fadd <br>
doesn't get `contract` either.<br>
<br>
The problem is that this doesn't really model what the SPIR-V says, and <br>
so it's likely to be unreliable in the face of general transformations.<br>
<br>
I hope you appreciate the difficulty of the situation, because I have to <br>
admit that Sanjay's argument has some merit -- but unfortunately, the <br>
one spec here that's actually explicit about what should happen (and <br>
that we happen to have to implement) also happens to disagree with the <br>
LLVM LangRef.<br>
<br>
Cheers,<br>
Nicolai<br>
<br>
<br>
> <br>
> Regards,<br>
> Michael<br>
> <br>
> <br>
>> On Aug 23, 2018, at 1:35 PM, Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a>> wrote:<br>
>><br>
>> I think what Michael is saying that:<br>
>><br>
>> 1. Don't set the global flag.<br>
>> 2. Set `contract` and `reassoc` on *all* floating point instructions by default.<br>
>> 3. Don't set those flags on instructions with NoContraction in SPIR-V.<br>
>><br>
>> I agree with the general thrust of this approach, except for the significant problem that it currently doesn't work.<br>
>><br>
>> 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).<br>
>><br>
>> Unfortunately, LLVM will contract those to an fma, where SPIR-V very explicitly says that that's forbidden.<br>
>><br>
>> 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.<br>
>><br>
>> If there's an argument against making this change, it'd be good if you could spell it out.<br>
>><br>
>> Cheers,<br>
>> Nicolai<br>
>><br>
>><br>
>> On 23.08.2018 21:47, Ryan Taylor wrote:<br>
>>> 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.<br>
>>> On Thu, Aug 23, 2018, 3:42 PM Michael Berg <<a href="mailto:michael_c_berg@apple.com" target="_blank">michael_c_berg@apple.com</a> <mailto:<a href="mailto:michael_c_berg@apple.com" target="_blank">michael_c_berg@apple.com</a>>> wrote:<br>
>>> Ryan,<br>
>>> Given that the global fast math flag overrides most fp behavior also<br>
>>> mapped to IR FMF flags as a control, I think we should remove it<br>
>>> from the argument. The Global fast math should override behavior<br>
>>> without IR flag opposition, as that is its intent under the<br>
>>> definition of Unsafe. Most implementations are moving away from the<br>
>>> large club of the global flag into fine granularity of control on<br>
>>> the IR without the Unsafe module level environment provided by it. This would then put the responsibility on front ends to mint the<br>
>>> proper expression level flags to control the desired behavior.<br>
>>> Regards,<br>
>>> Michael<br>
>>>> On Aug 23, 2018, at 12:13 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>>> wrote:<br>
>>>><br>
>>>> Michael,<br>
>>>><br>
>>>> From the spec with regards to reassoc:<br>
>>>><br>
>>>> – 15225 Include no re-association as a constraint required by<br>
>>>> the NoContraction Decoration.<br>
>>>><br>
>>>> I don't see a solution given the situation where -fp-contract=fast<br>
>>>> and we want to contract. Furthermore, I think a 'nocontract' flag<br>
>>>> will allow the IR to be more readable in it's intention. The<br>
>>>> problem is you can have 2 fp arith instructions with no contracts<br>
>>>> and no reassoc with global fast math flag set, how can you<br>
>>>> differentiate between the two instructions when you want one to be<br>
>>>> contract and the other to be no contract?<br>
>>>><br>
>>>> -Ryan<br>
>>>><br>
>>>><br>
>>>><br>
>>>> On Thu, Aug 23, 2018 at 2:27 PM Michael Berg<br>
>>>> <<a href="mailto:michael_c_berg@apple.com" target="_blank">michael_c_berg@apple.com</a> <mailto:<a href="mailto:michael_c_berg@apple.com" target="_blank">michael_c_berg@apple.com</a>>> wrote:<br>
>>>><br>
>>>> Ryan:<br>
>>>><br>
>>>> I think this will take more, here is the SPIR-V relevant text<br>
>>>> ( <a href="https://www.khronos.org/registry/spir-v/specs/1.0/SPIRV.pdf" rel="noreferrer" target="_blank">https://www.khronos.org/registry/spir-v/specs/1.0/SPIRV.pdf</a> ):<br>
>>>><br>
>>>> <start><br>
>>>><br>
>>>> NoContraction<br>
>>>><br>
>>>> Apply to an arithmetic instruction to indicate the operation<br>
>>>> cannot be combined with another instruction to form a single<br>
>>>> operation. For example, if applied to an OpFMul, that multiply<br>
>>>> can’t be combined with an addition to yield a fused<br>
>>>> multiply-add operation. Furthermore, such operations are not<br>
>>>> allowed to reassociate; e.g., add(a + add(b+c)) cannot be<br>
>>>> transformed to add(add(a+b) + c).<br>
>>>><br>
>>>> <end><br>
>>>><br>
>>>> The problem is that the spec does not mention the reassoc<br>
>>>> attribute, where an implication is made to tie both together<br>
>>>> as one control without specifying precisely that. I do not<br>
>>>> think we need to add another flag. I think we can work this<br>
>>>> within the definition. The text implies NoContraction is both<br>
>>>> contract=off and reassoc=off, baring global context. Since we<br>
>>>> are testing for both in isContractable in some fmul contexts<br>
>>>> and always in fadd context, it should suffice. This and the<br>
>>>> global context should be manageable in a SPIR env. The above<br>
>>>> text does not specify if the add should control fusing or not,<br>
>>>> but leaves it open to interpretation.<br>
>>>><br>
>>>> Our current definition of contract is:<br>
>>>><br>
>>>> ``contract``<br>
>>>> Allow floating-point contraction (e.g. fusing a multiply<br>
>>>> followed by an addition into a fused multiply-and-add).<br>
>>>><br>
>>>> Is somewhat loose, but then we do have a good deal of internal<br>
>>>> context no made evident that exists in the source.<br>
>>>> Lets put a stronger bound on the problem to solidify the<br>
>>>> goal. I believe front ends can be SPIR-V compliant and use<br>
>>>> the current framework. It seems more a management of the<br>
>>>> environment and IR than adding new feature controls.<br>
>>>><br>
>>>> Regards,<br>
>>>> Michael<br>
>>>><br>
>>>><br>
>>>>> On Aug 23, 2018, at 10:51 AM, 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>>> wrote:<br>
>>>>><br>
>>>>> Maybe there is a cleaner solution but it seems like adding a<br>
>>>>> 'nocontract' flag is close to the intention of spir-v and is<br>
>>>>> an easy check in the DAGCombiner without breaking anything<br>
>>>>> else and its intentions are very clear.<br>
>>>>><br>
>>>>> Right now the DAGCombiner logic doesn't seem to be able to<br>
>>>>> handle the case of having fast math globally with instruction<br>
>>>>> level flags to turn off fast math. Right now, either fast<br>
>>>>> math is global and it's assumed everything can be contracted<br>
>>>>> or fast math is not global and we contract if the contract<br>
>>>>> flag is present on the current instruction. I could be<br>
>>>>> missing something.<br>
>>>>><br>
>>>>> -Ryan<br>
>>>>><br>
>>>>> On Thu, Aug 23, 2018 at 1:36 PM Sanjay Patel<br>
>>>>> <<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a> <mailto:<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>>> wrote:<br>
>>>>><br>
>>>>> If we have this:<br>
>>>>> r = (X * Y) + Z<br>
>>>>><br>
>>>>> And we want that to become an fma op/node, 'contract' is<br>
>>>>> checked on the fadd because it is the fadd that's being<br>
>>>>> altered to form the (possibly non-standard) result. As I<br>
>>>>> think was noted earlier, whether 'contract' is set on the<br>
>>>>> fmul is irrelevant in our current implementation. This<br>
>>>>> allows the scenario where a strict fmul was inlined into<br>
>>>>> code with looser FP semantics, and we're still free to<br>
>>>>> create an fma. If the end value allows non-standard<br>
>>>>> behavior, then we assume that intermediate ops leading up<br>
>>>>> to that end value can use non-standard behavior too.<br>
>>>>> (cc'ing Michael Berg who did a lot of the DAG FMF work<br>
>>>>> recently)<br>
>>>>><br>
>>>>> I'm not familiar with SPIR-V, but it sounds like it has<br>
>>>>> an inverse flag system to what we have in IR and DAG -<br>
>>>>> ops are presumed contract-able unless specified with<br>
>>>>> 'no-contract'? Not sure how to resolve that.<br>
>>>>><br>
>>>>> If we want to change the LLVM FMF semantics, then there<br>
>>>>> will be breakage in the IR optimizer too (at least for<br>
>>>>> 'reassoc'; not sure about 'contract'). Either way, I<br>
>>>>> agree that we should try to clarify the LangRef about<br>
>>>>> this because you can't tell how things are supposed to<br>
>>>>> work from the current description.<br>
>>>>><br>
>>>>><br>
>>>>> On Wed, Aug 22, 2018 at 9:41 AM, Nicolai Hähnle via<br>
>>>>> llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>> wrote:<br>
>>>>><br>
>>>>> On 22.08.2018 13:29, Ryan Taylor wrote:<br>
>>>>><br>
>>>>> The example starts as SPIR-V with the<br>
>>>>> NoContraction decoration flag on the fmul.<br>
>>>>><br>
>>>>> I think what you are saying seems valid in that<br>
>>>>> if the user had put the flag on the fadd instead<br>
>>>>> of the fmul it would not contract and so in this<br>
>>>>> example the user needs to put the NoContraction<br>
>>>>> on the fadd though I'm not sure that's a good<br>
>>>>> expectation of the user. On the surface, I think<br>
>>>>> that if an operation didn't have the contract<br>
>>>>> flag than it wouldn't be contracted, regardless<br>
>>>>> of what flags any other operation has.<br>
>>>>><br>
>>>>><br>
>>>>> Okay, I see that the SPIR-V spec specifically calls<br>
>>>>> out this example.<br>
>>>>><br>
>>>>> Unless there are conflicting requirements with<br>
>>>>> another frontend, I'd say we should make sure LLVM is<br>
>>>>> aligned with SPIR-V here. Something along the lines<br>
>>>>> of (in LangRef):<br>
>>>>><br>
>>>>> ``contract``<br>
>>>>> Allow floating-point contraction (e.g. fusing a<br>
>>>>> multiply followed by<br>
>>>>> an addition into a fused multiply-and-add). This<br>
>>>>> flag must be present<br>
>>>>> on all affected instruction.<br>
>>>>><br>
>>>>> And we should probably say the same about ``reassoc``<br>
>>>>> 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<br>
>>>>> via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>>>> <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<br>
>>>>> 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<br>
>>>>> float %20, -1.000000e+00<br>
>>>>> ><br>
>>>>> > is being contracted in DAG to fmad. Is<br>
>>>>> 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<br>
>>>>> fadd, the frontend is<br>
>>>>> essentially saying "different rounding on the<br>
>>>>> 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<br>
>>>>> 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<br>
>>>>> 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><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><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><br>
>>>>> <mailto:<a href="mailto:ryta1203@gmail.com" target="_blank">ryta1203@gmail.com</a>>>>> wrote:<br>
>>>>> ><br>
>>>>> > I'm curious why the condition to fuse<br>
>>>>> is this:<br>
>>>>> ><br>
>>>>> > // Floating-point multiply-add with<br>
>>>>> 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() ||<br>
>>>>> F.hasAllowReassociation();<br>
>>>>> > }<br>
>>>>> ><br>
>>>>> > bool CanFuse = Options.UnsafeFPMath ||<br>
>>>>> isContractable(N);<br>
>>>>> > bool AllowFusionGlobally =<br>
>>>>> (Options.AllowFPOpFusion ==<br>
>>>>> > FPOpFusion::Fast || CanFuse || HasFMAD);<br>
>>>>> > // If the addition is not<br>
>>>>> contractable, do not combine.<br>
>>>>> > if (!AllowFusionGlobally &&<br>
>>>>> !isContractable(N))<br>
>>>>> > return SDValue();<br>
>>>>> ><br>
>>>>> > Specifically the AllowFusionGlobally,<br>
>>>>> I would have expected<br>
>>>>> > something more like:<br>
>>>>> ><br>
>>>>> > bool AllowFusionGlobally =<br>
>>>>> (Options.AllowFPOpFusion ==<br>
>>>>> > FPOpFusion::Fast && CanFuse && HasFMAD);<br>
>>>>> ><br>
>>>>> > or at the very least:<br>
>>>>> ><br>
>>>>> > bool AllowFusionGlobally =<br>
>>>>> ((Options.AllowFPOpFusion ==<br>
>>>>> > FPOpFusion::Fast || CanFuse) && HasFMAD);<br>
>>>>> ><br>
>>>>> > It seems that as long as the target<br>
>>>>> can do fmad it does do fmad<br>
>>>>> > since HasFMAD is true.<br>
>>>>> ><br>
>>>>> > Thanks.<br>
>>>>> ><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><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>><br>
>>>>> ><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>
>>>>> -- 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><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
>>>>> <mailto:<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>>>>> <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>
>>>>> -- 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>
>><br>
>> -- <br>
>> Lerne, wie die Welt wirklich ist,<br>
>> Aber vergiss niemals, wie sie sein sollte.<br>
> <br>
<br>
<br>
-- <br>
Lerne, wie die Welt wirklich ist,<br>
Aber vergiss niemals, wie sie sein sollte.<br>
</blockquote></div>