[Bug c++/65690] New: [5 Regression] typedef alignment lost since r219705

[Bug c++/65690] New: [5 Regression] typedef alignment lost since r219705

[jakub at gcc dot gnu.org]

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

            Bug ID: 65690
           Summary: [5 Regression] typedef alignment lost since r219705
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org

typedef double T[4][4] __attribute__((aligned (16)));
void foo (const T);
struct S { T s; };

int
main ()
{
  if (__alignof__ (struct S) < 16 || __alignof__ (T) < 16)
    __builtin_abort ();
  return 0;
}

(distilled from WebKit) fails with C++ starting with r219705 (but keeps working
with C).  Without the const T in the argument list of foo (or with just T in
there) it works.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-07
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org
   Target Milestone|---                         |5.0
     Ever confirmed|0                           |1

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
We are constructing:
(gdb) p debug_tree (t)
 <array_type 0x7ffff6c57498
    type <array_type 0x7ffff6c57348
        type <real_type 0x7ffff6b02a80 double readonly DF
            size <integer_cst 0x7ffff6ad7e58 constant 64>
            unit size <integer_cst 0x7ffff6ad7e70 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff6b02a80
precision 64
            pointer_to_this <pointer_type 0x7ffff6b02b28>>
        BLK
        size <integer_cst 0x7ffff6af91c8 constant 256>
        unit size <integer_cst 0x7ffff6af92b8 constant 32>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff6c57348
        domain <integer_type 0x7ffff6c44e70 type <integer_type 0x7ffff6adb0a8
sizetype>
            type_6 DI size <integer_cst 0x7ffff6ad7e58 64> unit size
<integer_cst 0x7ffff6ad7e70 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff6c44e70
precision 64 min <integer_cst 0x7ffff6ad7e88 0> max <integer_cst 0x7ffff6c51be8
3>>>
    VOID
    align 8 symtab 0 alias set -1 canonical type 0x7ffff6c57498 domain
<integer_type 0x7ffff6c44e70>>
$12 = void

as a variant of:
(gdb) p debug_tree (m)
 <array_type 0x7ffff6c570a8
    type <array_type 0x7ffff6c44f18
        type <real_type 0x7ffff6afd498 double DF
            size <integer_cst 0x7ffff6ad7e58 constant 64>
            unit size <integer_cst 0x7ffff6ad7e70 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff6afd498
precision 64
            pointer_to_this <pointer_type 0x7ffff6afd690>>
        BLK
        size <integer_cst 0x7ffff6af91c8 constant 256>
        unit size <integer_cst 0x7ffff6af92b8 constant 32>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff6c44f18
        domain <integer_type 0x7ffff6c44e70 type <integer_type 0x7ffff6adb0a8
sizetype>
            type_6 DI size <integer_cst 0x7ffff6ad7e58 64> unit size
<integer_cst 0x7ffff6ad7e70 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff6c44e70
precision 64 min <integer_cst 0x7ffff6ad7e88 0> max <integer_cst 0x7ffff6c51be8
3>>
        pointer_to_this <pointer_type 0x7ffff6c57150>>
    BLK
    size <integer_cst 0x7ffff6c26900 type <integer_type 0x7ffff6adb150
bitsizetype> constant 1024>
    unit size <integer_cst 0x7ffff6c51c18 type <integer_type 0x7ffff6adb0a8
sizetype> constant 128>
    align 64 symtab 0 alias set -1 canonical type 0x7ffff6c570a8 domain
<integer_type 0x7ffff6c44e70>>
$13 = void

From:

#0  build_cplus_array_type (elt_type=0x7ffff6c57348, index_type=0x7ffff6c44e70)
at ../../gcc/cp/tree.c:884
#1  0x000000000080a2bb in cp_build_qualified_type_real (type=0x7ffff6c571f8,
type_quals=1, complain=7) at ../../gcc/cp/tree.c:1068
#2  0x0000000000703f8e in grokdeclarator (declarator=0x0, declspecs=0x20b3978,
decl_context=PARM, initialized=0, attrlist=0x20b39d8) at
../../gcc/cp/decl.c:9487

So it is construction of "const T".  For some reason layout_type then drops the
user alignment from the main variant.  Who is supposed to copy the alignment to
the qualified variant?

layout_type does:
TYPE_ALIGN (type) = MAX (TYPE_ALIGN (element), BITS_PER_UNIT);
TYPE_USER_ALIGN (type) = TYPE_USER_ALIGN (element);

and finally copies it to all variants in finalize_type_size.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I see, C++ has special code for building qualified array types.
I would say that build_cplus_array_type should have a path where it is building
a variant and go via build_distinct_type_copy only adjusting the array type w/o
re-doing the layout.

This way also the type attributes and other stuff will be preserved.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Created attachment 35248
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35248&action=edit
Patch I am testing.

Someting like this may work. I think we only want to change element type.  I
also noticed the array loop missed the alignment compare

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #4 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
This is better version of the patch that at least seems to survive early stages
of bootstrap ;)
Index: tree.c
===================================================================
--- tree.c      (revision 221909)
+++ tree.c      (working copy)
@@ -1053,26 +1053,16 @@ cp_build_qualified_type_real (tree type,
       if (element_type == error_mark_node)
        return error_mark_node;

-      /* See if we already have an identically qualified type.  Tests
-        should be equivalent to those in check_qualified_type.  */
+      /* See if we already have an identically qualified type.  */
       for (t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
        if (TREE_TYPE (t) == element_type
-           && TYPE_NAME (t) == TYPE_NAME (type)
-           && TYPE_CONTEXT (t) == TYPE_CONTEXT (type)
-           && attribute_list_equal (TYPE_ATTRIBUTES (t),
-                                    TYPE_ATTRIBUTES (type)))
+           && check_base_type (t, type))
          break;

       if (!t)
        {
-         t = build_cplus_array_type (element_type, TYPE_DOMAIN (type));
-
-         /* Keep the typedef name.  */
-         if (TYPE_NAME (t) != TYPE_NAME (type))
-           {
-             t = build_variant_type_copy (t);
-             TYPE_NAME (t) = TYPE_NAME (type);
-           }
+         t = build_variant_type_copy (type);
+         TREE_TYPE (t) = element_type;
        }

       /* Even if we already had this variant, we update

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Jason,
I think I need a help on this one. I am not able to get canonical types right
in all cases.
Also I added the following sanity check that seems to trigger in the testuiste
Index: ../../gcc/stor-layout.c
===================================================================
--- ../../gcc/stor-layout.c     (revision 221909)
+++ ../../gcc/stor-layout.c     (working copy)
@@ -1831,6 +1831,8 @@ finalize_type_size (tree type)
        {
          TYPE_SIZE (variant) = size;
          TYPE_SIZE_UNIT (variant) = size_unit;
+         if (TYPE_USER_ALIGN (variant))
+           gcc_assert (user_align && TYPE_ALIGN (variant) == align);
          TYPE_ALIGN (variant) = align;
          TYPE_PRECISION (variant) = precision;
          TYPE_USER_ALIGN (variant) = user_align;

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #6 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
This variant of testcase also fails with GCC 4.8

void foo (const T);
struct S { T s; };

int
main ()
{
  if (__alignof__ (struct S) < 16 || __alignof__ (const T) < 16)
    __builtin_abort ();
  return 0;
}

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 35250
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35250&action=edit
gcc5-pr65690.patch

So what about this?  The first hunk being a fix for the 4.9 -> 5 regression,
the
second hunk to fix alignof (const T).
Or instead of layout_type we could just copy over all of
TYPE_SIZE/TYPE_SIZE_UNIT/TYPE_ALIGN/TYPE_USER_ALIGN/TYPE_PRECISION/TYPE_MODE.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[hubicka at gcc dot gnu.org]

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

--- Comment #8 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I also considered just copying over the stuff that layout_decl fills.  I guess
it is better than calling it and copying back TYPE_SIZE, but this is far from
being my area.

I think we also want to preserve attribute lists.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #8)
> I also considered just copying over the stuff that layout_decl fills.  I
> guess it is better than calling it and copying back TYPE_SIZE, but this is
> far from being my area.

Sure, that works for me too.  In build_cplus_array_type we know the
TYPE_ATTRIBUTES is NULL.

> I think we also want to preserve attribute lists.

Seems usually ../tree.c builds a distinct type for it instead, rather than
variant type.  And, I just think it is way too risky to also try to deal with
those at this point, perhaps for stage1, but also verify what the C FE does
with those, add testcases etc.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Regression fixed, for the rest I've created PR65715.

[Bug c++/65690] [5 Regression] typedef alignment lost since r219705

[jason at gcc dot gnu.org]

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

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> ---
Author: jason
Date: Thu Apr  9 20:11:44 2015
New Revision: 221960

URL: https://gcc.gnu.org/viewcvs?rev=221960&root=gcc&view=rev
Log:
    PR c++/65690
     * tree.c (cp_build_qualified_type_real): Copy TYPE_ALIGN and
    TYPE_USER_ALIGN.

Added:
    trunk/gcc/testsuite/c-c++-common/attr-aligned-1.c
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/tree.c