On Thu, Sep 13, 2012 at 5:52 PM, Jakub Jelinek <ja...@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 <ja...@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.
