On Sun, Feb 10, 2019 at 6:21 PM Jan Hubicka <hubi...@ucw.cz> wrote:
>
> Hi,
> build_qualified_type adjusts alignment of atomic types to one of minimal
> alignment needed for atomic operations (I think it does so). For packed
> structures this leads to type variant to be created and alignment to be
> updated later.
>
> If you call again build_qualified_type on packed structures, it won't
> reuse existing type because check_base_type will compare alignment of
> the base type (which is not atomic and has smaller alignment) and will
> end up creating new variant.
>
> When constructing a canonical types C frontned relies on types being
> shared and this eventually leads to ice in type simplification.
>
> I think it is easiest to teach check_base_type about minimal alignment.
>
> Bootstrapped/regtested x86_64-linux.
> PR lto/88585
> * tree.c (find_atomic_core_type): Forward declare.
> (check_base_type): Correctly compare alignments of atomic types.
> Index: tree.c
> ===================================================================
> --- tree.c (revision 268742)
> +++ tree.c (working copy)
> @@ -6329,18 +6329,33 @@ check_lang_type (const_tree cand, const_
> return lang_hooks.types.type_hash_eq (cand, base);
> }
>
> +static tree find_atomic_core_type (const_tree type);
> +
Just move find_atomic_core_type please.
> /* Returns true iff unqualified CAND and BASE are equivalent. */
>
> bool
> check_base_type (const_tree cand, const_tree base)
> {
> - return (TYPE_NAME (cand) == TYPE_NAME (base)
> - /* Apparently this is needed for Objective-C. */
> - && TYPE_CONTEXT (cand) == TYPE_CONTEXT (base)
> - /* Check alignment. */
> - && TYPE_ALIGN (cand) == TYPE_ALIGN (base)
> - && attribute_list_equal (TYPE_ATTRIBUTES (cand),
> - TYPE_ATTRIBUTES (base)));
> + if (TYPE_NAME (cand) != TYPE_NAME (base)
> + /* Apparently this is needed for Objective-C. */
> + || TYPE_CONTEXT (cand) != TYPE_CONTEXT (base)
> + || !attribute_list_equal (TYPE_ATTRIBUTES (cand),
> + TYPE_ATTRIBUTES (base)))
> + return false;
> + /* Check alignment. */
> + if (TYPE_ALIGN (cand) == TYPE_ALIGN (base))
> + return true;
> + /* Atomic types increase minimal alignment. We must to do so as well
> + or we get duplicated canonical types. See PR88686. */
> + if ((TYPE_QUALS (cand) & TYPE_QUAL_ATOMIC))
> + {
> + /* See if this object can map to a basic atomic type. */
> + tree atomic_type = find_atomic_core_type (cand);
build_qualified_type handles the case this function returns NULL,
I think you should as well.
> + if (TYPE_ALIGN (atomic_type) == TYPE_ALIGN (cand)
> + && TYPE_ALIGN (base) < TYPE_ALIGN (cand))
Why the second condition? build_qualified_type simply does
if (((type_quals & TYPE_QUAL_ATOMIC) == TYPE_QUAL_ATOMIC))
{
/* See if this object can map to a basic atomic type. */
tree atomic_type = find_atomic_core_type (type);
if (atomic_type)
{
/* Ensure the alignment of this type is compatible with
the required alignment of the atomic type. */
if (TYPE_ALIGN (atomic_type) > TYPE_ALIGN (t))
SET_TYPE_ALIGN (t, TYPE_ALIGN (atomic_type));
without caring for TYPE_ALIGN of the base type.
Note we seem to happily accept build_aligned_type that produce
under-aligned atomics :/
Richard.
> + return true;
> + }
> + return false;
> }
>
> /* Returns true iff CAND is equivalent to BASE with TYPE_QUALS. */
> @@ -6373,7 +6388,7 @@ check_aligned_type (const_tree cand, con
> atomic types, and returns that core atomic type. */
>
> static tree
> -find_atomic_core_type (tree type)
> +find_atomic_core_type (const_tree type)
> {
> tree base_atomic_type;
>
