[Bug target/54564] New: [4.8 Regression

[Bug target/54564] New: [4.8 Regression

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

             Bug #: 54564
           Summary: [4.8 Regression
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jakub@gcc.gnu.org

[Bug target/54564] [4.8 Regression

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-09-13 11:04:40 UTC ---
Unintended, I suppose ;)

[Bug target/54564] [4.8 Regression

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-13 11:12:40 UTC ---
Seems that starting with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190492
some fma tests fail:
FAIL: gcc.target/i386/fma-fmaddXX.c execution test
FAIL: gcc.target/i386/fma-fmsubXX.c execution test
FAIL: gcc.target/i386/fma-fnmaddXX.c execution test
FAIL: gcc.target/i386/fma-fnmsubXX.c execution test

The bug seems to be in the fmai_vmfmadd_<mode> expander and following patterns.
(match_dup 0) in the expander for the input operand that provides the vector
elts other than lowest is definitely wrong, the first operand to the expander
is the destination register, very likely uninitialized pseudo that is only
going to be initialized by that vfma*s{s,d} insn.  That means that the high
elements of the result of _mm_f{,n}m{add,sub}_s{s,d} are pretty much garbage.
The question is what the high elements of the result are supposed to be, the
intrinsics aren't very well documented.  Are the high elements supposed to
contain the high elements of the first argument of the intrinsics?  I'd assume
so.  But then it must be (match_dup 1) in the expander, and we need to rule out
the last alternative from the patterns.  Also the question is about
_mm_fnm{add_sub}_s{s,d} - those currently negate the first operand, thus it
would be pretty random what gets in the high elements.  Shouldn't we negate the
second argument instead?

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
   Last reconfirmed|                            |2012-09-13
         Resolution|INVALID                     |
   Target Milestone|---                         |4.7.2
            Summary|[4.8 Regression             |[4.8 Regression] Broken
                   |                            |__builtin_ia32_vfmadds[sd]3
     Ever Confirmed|0                           |1

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #3 from Yukhin Kirill <kirill.yukhin at intel dot com> 2012-09-13 11:57:35 UTC ---
Fails also occur on real HW.

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-13 12:07:28 UTC ---
Created attachment 28187
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28187
gcc48-pr54564.patch

Very lightly tested fix (just
make check-gcc RUNTESTFLAGS='--target_board=valgrind-sim/-m64 i386.exp=\*fma\*'
with valgrind that supports fma).

It is unfortunate that we don't generate the vf{,n}madd231s{s,d} insns any
longer, but I don't see how that would be possible, because the intrinsic (as
the testcases for it show) really wants to put the high elements of the first
argument into the destination, merged with the fma result.  The insns are
barely useful for anything else than the intrinsic, normal code would either
use the scalar insn that ignores the higher elements, or full vector version,
so IMHO this isn't a big deal.

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-14 12:16:59 UTC ---
H.J., could you please review the fmaintrin.h changes of
http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00921.html
(Uros wanted your feedback on that).  Thanks.

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-14 12:36:41 UTC ---
Author: jakub
Date: Fri Sep 14 12:36:33 2012
New Revision: 191297

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191297
Log:
    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.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-14 12:37:32 UTC ---
Author: jakub
Date: Fri Sep 14 12:37:16 2012
New Revision: 191298

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191298
Log:
    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.

Modified:
    trunk/gcc/config/i386/fmaintrin.h
    trunk/gcc/config/i386/sse.md

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-14 12:36:41 UTC ---
Author: jakub
Date: Fri Sep 14 12:36:33 2012
New Revision: 191297

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191297
Log:
    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.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-14 12:37:32 UTC ---
Author: jakub
Date: Fri Sep 14 12:37:16 2012
New Revision: 191298

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191298
Log:
    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.

Modified:
    trunk/gcc/config/i386/fmaintrin.h
    trunk/gcc/config/i386/sse.md

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-14 12:40:37 UTC ---
Fixed on trunk, backport also tested on the 4.7 branch, but as 4.7.2-rc1 is in
the works, I'll defer it for after 4.7.2 is released.

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-17 09:16:47 UTC ---
Author: jakub
Date: Mon Sep 17 09:16:39 2012
New Revision: 191377

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191377
Log:
    PR target/54564
    ChangeLog entry fixup

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.2                       |4.7.3

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-20 10:19:45 UTC ---
GCC 4.7.2 has been released.

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-09-20 15:52:49 UTC ---
Author: jakub
Date: Thu Sep 20 15:52:41 2012
New Revision: 191570

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191570
Log:
    Backported from mainline
    2012-09-14  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.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/i386/fmaintrin.h
    branches/gcc-4_7-branch/gcc/config/i386/sse.md

[Bug target/54564] [4.8 Regression] Broken __builtin_ia32_vfmadds[sd]3

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54564

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-08 12:35:55 UTC ---
Fixed.