public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: Jan Hubicka <hubicka@ucw.cz>, gcc-patches@gcc.gnu.org
Subject: Re: [C++ PATCH] Fix alignment handling in build_cplus_array_type/cp_build_qualified_type_real (PR c++/65690)
Date: Wed, 08 Apr 2015 15:08:00 -0000	[thread overview]
Message-ID: <20150408150759.GS19273@tucnak.redhat.com> (raw)
In-Reply-To: <55253F73.1000902@redhat.com>

On Wed, Apr 08, 2015 at 10:47:15AM -0400, Jason Merrill wrote:
> On 04/08/2015 06:02 AM, Jakub Jelinek wrote:
> >	(cp_build_qualified_type_real): Use check_base_type.  Build a
> >	variant and copy over even TYPE_CONTEXT and
> >	TYPE_ALIGN/TYPE_USER_ALIGN if any of those are different.
> 
> This seems wrong.  If there is an array with the same name, attributes and
> element type, it should have the same alignment; if it doesn't, that
> probably means that one of the types hasn't been laid out yet.  We don't
> want to have two variants of the same array that are distinguished only by
> whether they've been laid out, especially since later probably both will be
> laid out and the two types will be the same.

But that is how handle_aligned_attribute works, since forever (checked it
back to 3.2).  In <= 3.4.x, it used to create it using build_type_copy,
since 4.0.0 using build_variant_type_copy, but both those routines behave
the same - build a type variant which is linked in the TYPE_NEXT_VARIANT
chain, and differs from the other type in there possibly just by
TYPE_ALIGN/TYPE_USER_ALIGN.  Perhaps it should check TYPE_ALIGN only if
at least one of the two types has TYPE_USER_ALIGN set?
As for why TYPE_ATTRIBUTES are NULL, the reason for that is that
these are attributes on a typedef, so the attributes go into DECL_ATTRIBUTES
of the TYPE_DECL instead.

Anyway, the P1 regression is just about the first hunk, so if you have
issues just with the second hunk and not the first hunk (from either of the
patches), I can just comment out the tests for alignof (const T), and open
a separate PR for that for later.

	Jakub

  reply	other threads:[~2015-04-08 15:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-08 10:02 Jakub Jelinek
2015-04-08 14:47 ` Jason Merrill
2015-04-08 15:08   ` Jakub Jelinek [this message]
2015-04-09 14:48     ` Jason Merrill
2015-04-09 15:19       ` Jakub Jelinek
2015-04-08 16:22   ` Jan Hubicka
2015-04-08 16:28     ` Jakub Jelinek
2015-04-08 16:32       ` Jan Hubicka
2015-04-08 17:00         ` Jakub Jelinek
2015-04-09  9:39 ` Jakub Jelinek
2015-04-09 14:51   ` Jason Merrill
2015-04-09 18:12     ` [C++ PATCH] Fix alignment handling in build_cplus_array_type/cp_build_qualified_type_real (PR c++/65715) Jakub Jelinek
2015-04-10 14:28       ` Jason Merrill
2015-04-10 14:31         ` Jakub Jelinek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150408150759.GS19273@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).