[Bug c++/56564] New: [4.7 Regression] movdqa on possibly-8-byte-aligned struct with -O3

[Bug c++/56564] New: [4.7 Regression] movdqa on possibly-8-byte-aligned struct with -O3

[lukeocamden at gmail dot com]

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

             Bug #: 56564
           Summary: [4.7 Regression] movdqa on possibly-8-byte-aligned
                    struct with -O3
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: lukeocamden@gmail.com


#include <boost/exception_ptr.hpp>

struct foo { };

int main()
{
  boost::copy_exception(foo());
}

Compiling the above with -O3 results in the following instruction being
emitted:

movdqa  %xmm0,
_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep(%rip)

But that symbol need not be 16-byte aligned (it's a boost::exception_ptr, which
contains a boost::shared_ptr, which is just a pair of pointers).

This crashes if
_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep
comes from another object file where it is declared with 8-byte alignment.

Possible duplicate of 54167? Works fine with 4.6.2

Preprocessed source is attached.

[Bug c++/56564] [4.7 Regression] movdqa on possibly-8-byte-aligned struct with -O3

[lukeocamden at gmail dot com]

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

--- Comment #1 from lukeocamden at gmail dot com 2013-03-07 16:26:39 UTC ---
Created attachment 29611
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29611
Preprocessed source file

[Bug c++/56564] movdqa on possibly-8-byte-aligned struct with -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-03-08
            Summary|[4.7 Regression] movdqa on  |movdqa on
                   |possibly-8-byte-aligned     |possibly-8-byte-aligned
                   |struct with -O3             |struct with -O3
     Ever Confirmed|0                           |1
      Known to fail|                            |4.6.4, 4.7.2, 4.8.0

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-08 10:00:54 UTC ---
This instruction appears in an EH region of function

_ZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEv

(AFAIK).  It's defined twice, once weak and aligned 8 and once strong
and aligned 16, so AFAIK it _is_ aligned properly.

        .align 8
        .type  
_ZGVZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep,
@gnu_unique_object
        .size  
_ZGVZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep,
8
_ZGVZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep:
        .zero   8
        .weak  
_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep
        .section       
.bss._ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep,"awG",@nobits,_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep,comdat
        .align 16
        .type  
_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep,
@gnu_unique_object
        .size  
_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep,
16
_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep:
        .zero   16

and readelf shows:

  [192] .bss._ZZN5boost16 NOBITS           0000000000000000  00001ca0
       0000000000000010  0000000000000000 WAG       0     0     16

with alignment of 16.

> This crashes if
>_ZZN5boost16exception_detail27get_static_exception_objectINS0_10bad_alloc_EEENS_13exception_ptrEvE2ep
> comes from another object file where it is declared with 8-byte alignment.

so this would be a bug and a violation of ODR(?)

What's this "other object file"?

The code piece in question is:

        template <class Exception>
        exception_ptr
        get_static_exception_object()
            {
            Exception ba;
            exception_detail::clone_impl<Exception> c(ba);
            static exception_ptr ep(shared_ptr<exception_detail::clone_base
const>(new exception_detail::clone_impl<Exception>(c)));
            return ep;
            }

OTOH, not sure what increases the alignment of that object from it's
type-alignmend.

Same alignment is emitted with 4.8 and also 4.6 - so you must be unlucky
with that other object file (compiled with which compiler?)

Please also attach preprocessed source of the "other object file"

[Bug c++/56564] movdqa on possibly-8-byte-aligned struct with -O3

[lukeocamden at gmail dot com]

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

--- Comment #3 from lukeocamden at gmail dot com 2013-03-08 10:32:34 UTC ---
Sorry for my cryptic comments about the "other object file". It's compiled with
icc 13. I will attach the preprocessed source and generated assembly.

[Bug c++/56564] movdqa on possibly-8-byte-aligned struct with -O3

[lukeocamden at gmail dot com]

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

--- Comment #4 from lukeocamden at gmail dot com 2013-03-08 10:47:10 UTC ---
Created attachment 29618
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29618
Preprocessed with ICC

[Bug c++/56564] movdqa on possibly-8-byte-aligned struct with -O3

[lukeocamden at gmail dot com]

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

--- Comment #5 from lukeocamden at gmail dot com 2013-03-08 11:07:45 UTC ---
Created attachment 29619
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29619
Generated by icc 13

[Bug c++/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-08 11:20:19 UTC ---
This is due to ix86_data_alignment, which has:

  /* x86-64 ABI requires arrays greater than 16 bytes to be aligned
     to 16byte boundary.  */
  if (TARGET_64BIT)
    {
      if (AGGREGATE_TYPE_P (type)
           && TYPE_SIZE (type)
           && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
           && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 128
               || TREE_INT_CST_HIGH (TYPE_SIZE (type))) && align < 128)
        return 128;
    }

The comment and wording of
http://refspecs.linuxfoundation.org/elf/x86_64-abi-0.95.pdf
seems to be inconsistent with what the code does.
The comment and 0.95 version of the psABI only talks about arrays:
"An array uses the same alignment as its elements, except that a local or
global
array variable that requires at least 16 bytes, or a C99 local or global
variable-length array variable, always has alignment of at least 16 bytes."
but AGGREGATE_TYPE_P isn't solely about array local/global variables, but any
aggregates (arrays, structs, unions, ...).  ep apparently has size of 16 and
the above code aligns it to 16 bytes, but icc probably aligns it just to 8
bytes, as the maximum alignment of its members.  Now, changing the above to
only look at arrays would probably cause more harm than good, because while
code compiled by fixed gcc would be compatible with icc, it would be
incompatible with code compiled by older gcc.  Guess if we want to change
something, we'd need to change it in a way that such aggregates (non-array
ones) of size 16 and above are still 16-byte aligned, but if the variable isn't
known to bind locally, don't increase DECL_ALIGN of the var, so that no
optimizers actually rely on it.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI, wrong-code
             Target|                            |x86_64-*-*
             Status|WAITING                     |NEW

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-08 11:26:19 UTC ---
Confirmed.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-08 11:35:36 UTC ---
Guess we'd need to split DATA_ALIGNMENT into two different macros (or one with
an extra argument), so that align_variable would know what alignment is part of
ABI and what is just an optimization above that, then align_variable could call
targetm.binds_local_p to see if DECL_ALIGN can be increased to the optimization
level or needs to stay at the ABI guaranteed level.  And then when assembling
vars, we'd increase the emitted alignment to the optimization level.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-08 12:38:20 UTC ---
Smaller testcase (-O2 -fpic):

struct S { long a, b; } s;
int foo (void)
{
  return ((long) &s) & 15;
}

is since http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162943 optimized into
return 0, even when (probably) the psABI doesn't guarantee that.  But e.g. for
__builtin_memset (&s, 0, sizeof (s)); one can see already in 4.0 RTL dumps with
-O2 -fpic that MEM_ALIGN of s is assumed to be 128-bit.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

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

--- Comment #10 from Jan Hubicka <hubicka at gcc dot gnu.org> 2013-04-08 15:22:21 UTC ---
Mine.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Maybe it was original DATA_ALIGNMENT purpose, but it certainly serves for both
right now, which is wrong, we need one for ABI mandated stuff and one for
optimization stuff beyond, where optimization alignment can be used if it can
be proved that we'll bind to the optimized decl, but ABI has to be used
otherwise.

E.g. x86_64 ABI says that certain arrays are aligned that and that way, it is
certainly something beyond what TYPE_ALIGN provides (changing TYPE_ALIGN of the
arrays would affect layout of structures, but that is wrong).

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30275
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30275&action=edit
gcc49-pr56564.patch

Untested fix.  Honza, is the array type >= 16 bytes alignment increase the only
ABI mandated one and all the rest is just optimization?

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Jun 10 15:41:52 2013
New Revision: 199898

URL: http://gcc.gnu.org/viewcvs?rev=199898&root=gcc&view=rev
Log:
    PR target/56564
    * varasm.c (align_variable): Don't use DATA_ALIGNMENT or
    CONSTANT_ALIGNMENT if !decl_binds_to_current_def_p (decl).
    Use DATA_ABI_ALIGNMENT for that case instead if defined.
    (get_variable_align): New function.
    (get_variable_section, emit_bss, emit_common,
    assemble_variable_contents, place_block_symbol): Use
    get_variable_align instead of DECL_ALIGN.
    (assemble_noswitch_variable): Add align argument, use it
    instead of DECL_ALIGN.
    (assemble_variable): Adjust caller.  Use get_variable_align
    instead of DECL_ALIGN.
    * config/i386/i386.h (DATA_ALIGNMENT): Adjust x86_data_alignment
    caller.
    (DATA_ABI_ALIGNMENT): Define.
    * config/i386/i386-protos.h (x86_data_alignment): Adjust prototype.
    * config/i386/i386.c (x86_data_alignment): Add opt argument.  If
    opt is false, only return the psABI mandated alignment increase.
    * config/c6x/c6x.h (DATA_ALIGNMENT): Renamed to...
    (DATA_ABI_ALIGNMENT): ... this.
    * config/mmix/mmix.h (DATA_ALIGNMENT): Renamed to...
    (DATA_ABI_ALIGNMENT): ... this.
    * config/mmix/mmix.c (mmix_data_alignment): Adjust function comment.
    * config/s390/s390.h (DATA_ALIGNMENT): Renamed to...
    (DATA_ABI_ALIGNMENT): ... this.
    * doc/tm.texi.in (DATA_ABI_ALIGNMENT): Document.
    * doc/tm.texi: Regenerated.

    * gcc.target/i386/pr56564-1.c: New test.
    * gcc.target/i386/pr56564-2.c: New test.
    * gcc.target/i386/pr56564-3.c: New test.
    * gcc.target/i386/pr56564-4.c: New test.
    * gcc.target/i386/avx256-unaligned-load-4.c: Add -fno-common.
    * gcc.target/i386/avx256-unaligned-store-1.c: Likewise.
    * gcc.target/i386/avx256-unaligned-store-3.c: Likewise.
    * gcc.target/i386/avx256-unaligned-store-4.c: Likewise.
    * gcc.target/i386/vect-sizes-1.c: Likewise.
    * gcc.target/i386/memcpy-1.c: Likewise.
    * gcc.dg/vect/costmodel/i386/costmodel-vect-31.c (tmp): Initialize.
    * gcc.dg/vect/costmodel/x86_64/costmodel-vect-31.c (tmp): Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr56564-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr56564-2.c
    trunk/gcc/testsuite/gcc.target/i386/pr56564-3.c
    trunk/gcc/testsuite/gcc.target/i386/pr56564-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/c6x/c6x.h
    trunk/gcc/config/i386/i386-protos.h
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.h
    trunk/gcc/config/mmix/mmix.c
    trunk/gcc/config/mmix/mmix.h
    trunk/gcc/config/s390/s390.h
    trunk/gcc/doc/tm.texi
    trunk/gcc/doc/tm.texi.in
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/vect/costmodel/i386/costmodel-vect-31.c
    trunk/gcc/testsuite/gcc.dg/vect/costmodel/x86_64/costmodel-vect-31.c
    trunk/gcc/testsuite/gcc.target/i386/avx256-unaligned-load-4.c
    trunk/gcc/testsuite/gcc.target/i386/avx256-unaligned-store-1.c
    trunk/gcc/testsuite/gcc.target/i386/avx256-unaligned-store-3.c
    trunk/gcc/testsuite/gcc.target/i386/avx256-unaligned-store-4.c
    trunk/gcc/testsuite/gcc.target/i386/memcpy-1.c
    trunk/gcc/testsuite/gcc.target/i386/vect-sizes-1.c
    trunk/gcc/varasm.c

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Tue Jun 11 06:03:46 2013
New Revision: 199934

URL: http://gcc.gnu.org/viewcvs?rev=199934&root=gcc&view=rev
Log:
    PR target/56564
    * varasm.c (get_variable_align): Move #endif to the right place.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/varasm.c

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[dominiq at lps dot ens.fr]

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

--- Comment #16 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
On x86_64-apple-darwin10.8 at revision 199935, I get the following failures for
the tests added at revision 199898:

FAIL: gcc.target/i386/pr56564-1.c scan-tree-dump-times optimized "&s" 1
FAIL: gcc.target/i386/pr56564-1.c scan-tree-dump-times optimized "return 0" 1
FAIL: gcc.target/i386/pr56564-3.c scan-tree-dump-times optimized "&s" 1
FAIL: gcc.target/i386/pr56564-3.c scan-tree-dump-times optimized "&t" 1

The optimized dumps are (blank lines removed):

[macbook] f90/bug% cat pr56564-1.c.165t.optimized
;; Function foo (foo, funcdef_no=0, decl_uid=1741, symbol_order=2)
foo ()
{
  <bb 2>:
  return 0;
}
;; Function bar (bar, funcdef_no=1, decl_uid=1744, symbol_order=3)
bar ()
{
  <bb 2>:
  return 0;
}

[macbook] f90/bug% cat pr56564-3.c.165t.optimized
;; Function foo (foo, funcdef_no=0, decl_uid=1741, symbol_order=2)
foo ()
{
  struct S * D.1770;
  long int s.0;
  int _2;
  int _3;
  <bb 2>:
  _5 = __builtin___emutls_get_address (&__emutls_v.s);
  s.0_1 = (long int) _5;
  _2 = (int) s.0_1;
  _3 = _2 & 15;
  return _3;
}
;; Function bar (bar, funcdef_no=1, decl_uid=1744, symbol_order=3)
bar ()
{
  char * D.1769;
  char[16] * D.1768;
  long int _1;
  int _2;
  int _3;
  <bb 2>:
  _5 = __builtin___emutls_get_address (&__emutls_v.t);
  _6 = &*_5[0];
  _1 = (long int) _6;
  _2 = (int) _1;
  _3 = _2 & 15;
  return _3;
}

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Dominique d'Humieres from comment #16)
> On x86_64-apple-darwin10.8 at revision 199935, I get the following failures
> for the tests added at revision 199898:
> 
> FAIL: gcc.target/i386/pr56564-1.c scan-tree-dump-times optimized "&s" 1
> FAIL: gcc.target/i386/pr56564-1.c scan-tree-dump-times optimized "return 0" 1
> FAIL: gcc.target/i386/pr56564-3.c scan-tree-dump-times optimized "&s" 1
> FAIL: gcc.target/i386/pr56564-3.c scan-tree-dump-times optimized "&t" 1

Yeah, MachO is broken by design, guess the tests need to be restricted to
non-darwin non-PE.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[dominiq at lps dot ens.fr]

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

--- Comment #18 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
(In reply to comment #17)
> Yeah, MachO is broken by design, guess the tests need to be restricted 
> to non-darwin non-PE.

Questions:
(1) What is PE?
(2) Is the second "return 0;" wrong code or valid optimization? If the former,
why?
(3) Is the decoration "__emutls_v." the same for all the emutls platforms? If
not, where can I find the variants?

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The mingw/cygwin stuff.  The testcases assume that the symbols have
decl_binds_to_current_def_p false, if that isn't the case (because darwin/mingw
apparently don't allow symbol interposition), then the testcase can't work on
those.

[Bug target/56564] movdqa on possibly-8-byte-aligned struct with -O3

[jakub at gcc dot gnu.org]

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

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Wed Jun 12 06:43:05 2013
New Revision: 199984

URL: http://gcc.gnu.org/viewcvs?rev=199984&root=gcc&view=rev
Log:
    PR target/56564
    * varasm.c (decl_binds_to_current_def_p): Call binds_local_p
    target hook even for !TREE_PUBLIC decls.  If no resolution info
    is available, return false for common and external decls.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/varasm.c

Author: jakub
Date: Wed Jun 12 06:46:53 2013
New Revision: 199985

URL: http://gcc.gnu.org/viewcvs?rev=199985&root=gcc&view=rev
Log:
    PR target/56564
    * gcc.target/i386/pr56564-1.c: Skip on darwin, mingw and cygwin.
    * gcc.target/i386/pr56564-3.c: Likewise.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/i386/pr56564-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr56564-3.c