Re: [PATCH] Fix up _mm_f{,n}m{add,sub}_s{s,d} (PR target/54564)

[Uros Bizjak <ubizjak at gmail dot com>]

On Thu, Sep 13, 2012 at 5:52 PM, Jakub Jelinek <jakub@redhat.com> wrote:

> The fma-*.c testcase show that these intrinsics probably mean to preserve
> the high elements (other than the lowest) of the first argument of the
> fmaintrin.h *_s{s,d} intrinsics in the destination (the HW insn preserve
> there the destination register, but that varies - for 132 and 213 it is the
> first one (but the negation performed for _mm_fnm*_s[sd] breaks it anyway),
> for 231 it is the last one).  What the expander did was to put there
> an uninitialized pseudo, so we ended up with pretty random content, before
> H.J's http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190492 it happened
> to work by accident, but when things changed slightly and reload chose
> different alternative, this broke.
>
> The following patch fixes it, by tweaking the header so that the first
> argument is not negated (we negate the second one instead), as we don't want
> to negate the high elements if e.g. for whatever reason combiner doesn't
> match it.  It fixes the expander to use a dup of the X operand as the high
> element provider for the pattern, removes the 231 alternatives (because
> those provide different destination high elements) and removes commutative
> marker (again, that would mean different high elements).

Can we introduce additional "*fmai_fmadd_<mode>_1" pattern (and
others) that would cover missing 231 alternative?

> 2012-09-13  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/54564
>         * config/i386/sse.md (fmai_vmfmadd_<mode>): Use (match_dup 1)
>         instead of (match_dup 0) as second argument to vec_merge.
>         (*fmai_fmadd_<mode>, *fmai_fmsub_<mode>): Likewise.
>         Remove third alternative.
>         (*fmai_fnmadd_<mode>, *fmai_fnmsub_<mode>): Likewise.  Negate
>         operand 2 instead of operand 1, but put it as first argument
>         of fma.
>
>         * config/i386/fmaintrin.h (_mm_fnmadd_sd, _mm_fnmadd_ss,
>         _mm_fnmsub_sd, _mm_fnmsub_ss): Negate the second argument instead
>         of the first.

OK, but header change should be also reviewed by H.J.

Thanks,
Uros.