[Bug middle-end/59990] New: [4.7/4.8/4.9 regression] incorrect memcpy optimization

[Bug middle-end/59990] New: [4.7/4.8/4.9 regression] incorrect memcpy optimization

[ebotcazou at gcc dot gnu.org]

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

            Bug ID: 59990
           Summary: [4.7/4.8/4.9 regression] incorrect memcpy optimization
           Product: gcc
           Version: 4.7.3
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ebotcazou at gcc dot gnu.org
            Target: i?86-*-*

This is a regression introduced in the 4.7.x series of compilers, visible on
x86 but probably reproducible on other platforms.  If you compile:

extern void abort (void);

unsigned char value[4] = { 66, 9, 160, 255 };

int main (void)
{
  volatile float f;
  unsigned char a[4];

  __builtin_memcpy ((void *)&f, value, 4);
  __builtin_memcpy (a, (void *)&f, 4);
  if (a[2] != 160)
    abort ();

  return 0;
}

with optimization enabled, it aborts at run time because FP instructions are
incorrectly used to implement the memcpy operations.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[joseph at codesourcery dot com]

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

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
Seems rather like bug 58416 to me, if I understand it correctly (SFmode 
and DFmode x87 floating-point loads wrongly used when the result is used 
other than for arithmetic).  Should only affect platforms where loads and 
stores extend signaling NaNs to a wider format and so convert them to 
quiet NaNs with an exception raised.

[Bug target/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |target

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Each target has its own code to do expansion of memcpy.  The middle-end code
for memcpy uses the integer modes to do the copying and if the target gets that
incorrect then it is a bug in the target code.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |middle-end

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Each target has its own code to do expansion of memcpy.  The middle-end code
> for memcpy uses the integer modes to do the copying and if the target gets
> that incorrect then it is a bug in the target code.

No, and that's precisely the bug...

[Bug target/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-01-30
                 CC|                            |hjl.tools at gmail dot com,
                   |                            |kirill.yukhin at intel dot com,
                   |                            |ubizjak at gmail dot com
          Component|middle-end                  |target
   Target Milestone|---                         |4.7.4
     Ever confirmed|0                           |1

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> ---
It is caused by r174489.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |middle-end

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
This is really annoying, please conduct at least a minimal analysis before
recategorizing a PR, it's a MIDDLE END issue, namely fold_builtin_memory_op
wrongly choosing a floating-point type to implement the memcpy.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess the important thing is if it is allowed to raise exceptions on a simple
load and store of a floating point value.  So:
extern void abort (void);
union U
{
  unsigned char value[4];
  float f;
} u = { { 66, 9, 160, 255} };

int
main (void)
{
  volatile union U v;

  v.f = u.f;
  if (v.value[2] != 160)
    abort ();

  return 0;
}

If it is ok that thiscase aborts and the #c0 should not, then the bug would be
in fold-const.c and we should use integral types instead of floating point
types to fold the copy.  If this testcase shouldn't abort too, then it is a
target issue, where it should avoid the i387 loads if the result isn't used in
some arithmetic instruction.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[ebotcazou at gcc dot gnu.org]

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

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> If it is ok that thiscase aborts and the #c0 should not, then the bug would
> be in fold-const.c and we should use integral types instead of floating
> point types to fold the copy.  If this testcase shouldn't abort too, then it
> is a target issue, where it should avoid the i387 loads if the result isn't
> used in some arithmetic instruction.

I disagree, using FP types to implement memcpy in the middle-end is a no-no,
i387 isn't the only architecture which can fiddle with FP values.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #7)
> > If it is ok that thiscase aborts and the #c0 should not, then the bug would
> > be in fold-const.c and we should use integral types instead of floating
> > point types to fold the copy.  If this testcase shouldn't abort too, then it
> > is a target issue, where it should avoid the i387 loads if the result isn't
> > used in some arithmetic instruction.
> 
> I disagree, using FP types to implement memcpy in the middle-end is a no-no,
> i387 isn't the only architecture which can fiddle with FP values.

I agree and I have a fix.  Fortunately it's very easy and cleans up the code
even ;)

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 31991
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31991&action=edit
patch

Must resist turning this fn upside down ...

Anyway, untested patch attached.  We can surely do better selecting a
type, but IIRC most targets have an integer mode that covers all other
modes (even vector modes).

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That will likely break -fsanitize=enum as you are removing the handling of the
enums.  Can you please handle it the same if -fsanitize=enum (or always?).

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[joseph at codesourcery dot com]

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

--- Comment #11 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
On Thu, 30 Jan 2014, jakub at gcc dot gnu.org wrote:

> I guess the important thing is if it is allowed to raise exceptions on a simple
> load and store of a floating point value.  So:

At C level, if we follow draft TS 18661-1 (WG14 N1778) for specification 
of signaling NaNs, "Whether C assignment (6.5.16) (and conversion as if by 
assignment) to the same format is an IEC 60559 convertFormat or copy 
operation is implementation-defined, even if <fenv.h> defines the macro 
FE_SNANS_ALWAYS_SIGNAL".  I think it's most useful for such assignments, 
function calling etc. to be implemented not to raise exceptions - doing 
otherwise prevents properly implementing issignaling.  But when actual 
floating-point assignments are involved (as in bug 56831) it's only really 
a bug with -fsignaling-nans (although if there isn't any significant 
performance cost it's probably best not to depend on that).

When, as in bug 58416 and here, there are no floating-point assignments 
involved, it's entirely unambiguously a bug to raise exceptions.

> If it is ok that thiscase aborts and the #c0 should not, then the bug would be
> in fold-const.c and we should use integral types instead of floating point
> types to fold the copy.  If this testcase shouldn't abort too, then it is a
> target issue, where it should avoid the i387 loads if the result isn't used in
> some arithmetic instruction.

And if the value is both used in arithmetic and copied with memcpy, the 
copy can't reuse the value that was loaded into a floating-point register 
for the arithmetic (because the copy needs to copy the original signaling 
NaN to the destination, not the quiet NaN that resulted from the 
floating-point load).

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #9)
> Created attachment 31991 [details]
> patch
> 
> Must resist turning this fn upside down ...
> 
> Anyway, untested patch attached.  We can surely do better selecting a
> type, but IIRC most targets have an integer mode that covers all other
> modes (even vector modes).

Besides the GET_MODE_SIZE -> GET_MODE_BITSIZE fix and handling ENUMERAL_TYPEs
similarly for -fsanitize=enum, do you think you could add the
build_aligned_type call too?  I mean, if say the floating
point/boolean/complex/enum type has certain alignment and the integral type we
are going to use instead has different, it would be better to preserve the
alignment info.

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/59990] [4.7/4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P3
      Known to work|                            |4.9.0
            Summary|[4.7/4.8/4.9 regression]    |[4.7/4.8 regression]
                   |incorrect memcpy            |incorrect memcpy
                   |optimization                |optimization

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for trunk sofar.

[Bug middle-end/59990] [4.7/4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/59990] [4.7/4.8/4.9 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Fri Jan 31 11:54:26 2014
New Revision: 207338

URL: http://gcc.gnu.org/viewcvs?rev=207338&root=gcc&view=rev
Log:
2014-01-31  Richard Biener  <rguenther@suse.de>

    PR middle-end/59990
    * builtins.c (fold_builtin_memory_op): Make sure to not
    use a floating-point mode or a boolean or enumeral type for
    the copy operation.

    * gcc.dg/torture/pr59990.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr59990.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/59990] [4.7/4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
Backporting this to 4.8 regresses

//-m32
FAIL: gcc.target/i386/pr49168-1.c scan-assembler movdqu[\t ]*%xmm[0-9]+,[^,]*

flt128_va:
.LFB0:
        .cfi_startproc
        movl    4(%esp), %eax
        movdqa  20(%esp), %xmm0
        movlps  %xmm0, (%eax)
        movhps  %xmm0, 8(%eax)
        ret

expanding from

flt128_va (void * mem, __float128 d)
{
  uint128_t _2;

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  _2 = VIEW_CONVERT_EXPR<uint128_t>(d_5(D));
  MEM[(char * {ref-all})mem_3(D)] = _2;
  return;

which doesn't end up using a UNSPEC_STOREU insn as on trunk but

(insn 8 7 0 2 (set (mem:V2SF (plus:SI (reg/v/f:SI 60 [ mem ])
                (const_int 8 [0x8])) [0 MEM[(char * {ref-all})mem_3(D)]+8 S8
A8])
        (vec_select:V2SF (subreg:V4SF (reg/v:TF 61 [ d ]) 0)
            (parallel [
                    (const_int 2 [0x2])
                    (const_int 3 [0x3])
                ])))
/space/rguenther/src/svn/gcc-4_8-branch/gcc/testsuite/gcc.target/i386/pr49168-1.c:10
-1
     (nil))

[Bug middle-end/59990] [4.7/4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
seems to be TARGET_SSE_TYPELESS_STORES tuning dependent (and that was changed
on trunk for generic).

[Bug middle-end/59990] [4.7/4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |4.8.4

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
The 4.7 branch is being closed, moving target milestone to 4.8.4.

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[anders.blomdell at control dot lth.se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

anders.blomdell at control dot lth.se changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anders.blomdell at control dot lth
                   |                            |.se

--- Comment #19 from anders.blomdell at control dot lth.se ---
Created attachment 34775
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34775&action=edit
preprocessed program

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[anders.blomdell at control dot lth.se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

--- Comment #20 from anders.blomdell at control dot lth.se ---
Created attachment 34776
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34776&action=edit
Shell-script for testing flag variations

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[anders.blomdell at control dot lth.se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

--- Comment #21 from anders.blomdell at control dot lth.se ---
Sorry attachments belongs to bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65077

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[anders.blomdell at control dot lth.se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

--- Comment #22 from anders.blomdell at control dot lth.se ---
Sorry attachments belongs to bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65077

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[anders.blomdell at control dot lth.se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

--- Comment #23 from anders.blomdell at control dot lth.se ---
Sorry attachments belongs to bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65077

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Wed Jun 10 12:53:09 2015
New Revision: 224327

URL: https://gcc.gnu.org/viewcvs?rev=224327&root=gcc&view=rev
Log:
2015-06-10  Richard Biener  <rguenther@suse.de>

        Backport from mainline
        2014-04-04  Cong Hou  <congh@google.com>

        PR tree-optimization/60656
        * tree-vect-stmts.c (supportable_widening_operation):
        Fix a bug that elements in a vector with vect_used_by_reduction
        property are incorrectly reordered when the operation on it is not
        consistant with the one in reduction operation.

        * gcc.dg/vect/pr60656.c: New test.

        2014-01-31  Richard Biener  <rguenther@suse.de>

        PR middle-end/59990
        * builtins.c (fold_builtin_memory_op): Make sure to not
        use a floating-point mode or a boolean or enumeral type for
        the copy operation.

        * gcc.dg/torture/pr59990.c: New testcase.
        * gcc.target/i386/pr49168-1.c: Adjust.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr59990.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/vect/pr60656.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/builtins.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/i386/pr49168-1.c
    branches/gcc-4_8-branch/gcc/tree-vect-stmts.c

[Bug middle-end/59990] [4.8 regression] incorrect memcpy optimization

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59990

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

--- Comment #25 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.