Re: Replace FMA_EXPR with one internal fn per optab

[Andrew Pinski]

On Fri, May 11, 2018 at 11:04 AM, Richard Sandiford
<richard.sandif...@linaro.org> wrote:
> Andrew Pinski <pins...@gmail.com> writes:
>> On Fri, May 11, 2018 at 10:15 AM, Richard Sandiford
>> <richard.sandif...@linaro.org> wrote:
>>> There are four optabs for various forms of fused multiply-add:
>>> fma, fms, fnma and fnms.  Of these, only fma had a direct gimple
>>> representation.  For the other three we relied on special pattern-
>>> matching during expand, although tree-ssa-math-opts.c did have
>>> some code to try to second-guess what expand would do.
>>>
>>> This patch removes the old FMA_EXPR representation of fma and
>>> introduces four new internal functions, one for each optab.
>>> IFN_FMA is tied to BUILT_IN_FMA* while the other three are
>>> independent directly-mapped internal functions.  It's then
>>> possible to do the pattern-matching in match.pd and
>>> tree-ssa-math-opts.c (via folding) can select the exact
>>> FMA-based operation.
>>>
>>> The patch removes the gimple FE support for __FMA rather than mapping
>>> it to the internal function.  There's no reason now to treat it
>>> differently from other internal functions (although the FE doesn't
>>> handle those yet).
>>>
>>> The BRIG & HSA parts are a best guess, but seem relatively simple.
>>>
>>> The genmatch.c changes are structured to allow ternary ops in which
>>> the second two rather than the first two operands are commutative.
>>> A later patch makes use of this.
>>>
>>> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf,
>>> x86_64-linux-gnu and powerpc64le-linux-gnu.  OK to install?
>>
>>
>> I think there might be an issue with long double and __float128
>> support here (for both PowerPC and x86_64).  Please add testcases for
>> those to show they are not problematic.
>> What about half float on the aarch64 case?  Is that handle correctly?
>> I did not see a testcase for that case either.
>
> What specific kind of problem are you thinking of?  The patch is
> type-generic and the internal functions are only used if the target
> advertises the appropriate optab.  Targets already check (or should
> check) that the optabs are used under the appropriate conditions for
> that target.  E.g. gcc.target/powerpc/float128-fma1.c checks for the
> four cases of __float128 for PowerPC.

It was more of reference to the documentation addition you did:
"+Target supports all four fused multiply-add optabs for both @code{float}
+and @code{double}."

Also a side note, while I was working improving the use of integer
madd instructions on aarch64, I found if I changed "madd<mode>"
pattern name to "fma<mode>" I could get more madd used (and add with
shifts too).  Does the FMA internal function still support integer
types?

Thanks,
Andrew

>
> Thanks,
> Richard<div id="DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2"><br />
<table style="border-top: 1px solid #D3D4DE;">
        <tr>
        <td style="width: 55px; padding-top: 13px;"><a
href="http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail";
target="_blank"><img
src="https://ipmcdn.avast.com/images/icons/icon-envelope-tick-green-avg-v1.png";
alt="" width="46" height="29" style="width: 46px; height: 29px;"
/></a></td>
                <td style="width: 470px; padding-top: 12px; color: #41424e;
font-size: 13px; font-family: Arial, Helvetica, sans-serif;
line-height: 18px;">Virus-free. <a
href="http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail";
target="_blank" style="color: #4453ea;">www.avg.com</a>
                </td>
        </tr>
</table><a href="#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2" width="1"
height="1"></a></div>