On Thu, Sep 13, 2012 at 10:42 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
> 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.
>
It looks OK to me.
Thanks.
--
H.J.
