On Sun, Jun 29, 2014 at 12:44 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hi,
> this patch fixes another problem where we manage to produce a variant of
> variadic array that has different TYPE_SIZE (but an equivalent expression).
> This happens because remap_type_1 blindly copies all expressions referred by
> variadic type while for variants it may just reuse ones it earlier copied when
> producing a copy of the main variant.
>
> Bootstrapped/regtested x86_64-linux, lto-bootstrapped and tested on Firefox
> build with the main variant checking patch in, comitted.
Note that we do have variant types which do _not_ share the fields list
of the main variant (see PR61456). Not sure if your patches fix that.
But if they didn't yet the following patch silently re-write those types.
Richard.
> Honza
>
> * tree-inline.c (remap_type_1): Do not duplicate fields
> that are shared in between type and its main variant.
> Index: tree-inline.c
> ===================================================================
> --- tree-inline.c (revision 212098)
> +++ tree-inline.c (working copy)
> @@ -451,6 +451,8 @@ remap_type_1 (tree type, copy_body_data
> TYPE_POINTER_TO (new_tree) = NULL;
> TYPE_REFERENCE_TO (new_tree) = NULL;
>
> + /* Copy all types that may contain references to local variables; be sure
> to
> + preserve sharing in between type and its main variant when possible. */
> switch (TREE_CODE (new_tree))
> {
> case INTEGER_TYPE:
> @@ -458,40 +460,72 @@ remap_type_1 (tree type, copy_body_data
> case FIXED_POINT_TYPE:
> case ENUMERAL_TYPE:
> case BOOLEAN_TYPE:
> - t = TYPE_MIN_VALUE (new_tree);
> - if (t && TREE_CODE (t) != INTEGER_CST)
> - walk_tree (&TYPE_MIN_VALUE (new_tree), copy_tree_body_r, id, NULL);
> -
> - t = TYPE_MAX_VALUE (new_tree);
> - if (t && TREE_CODE (t) != INTEGER_CST)
> - walk_tree (&TYPE_MAX_VALUE (new_tree), copy_tree_body_r, id, NULL);
> + if (TYPE_MAIN_VARIANT (new_tree) != new_tree)
> + {
> + gcc_checking_assert (TYPE_MIN_VALUE (type) == TYPE_MIN_VALUE
> (TYPE_MAIN_VARIANT (type)));
> + gcc_checking_assert (TYPE_MAX_VALUE (type) == TYPE_MAX_VALUE
> (TYPE_MAIN_VARIANT (type)));
> +
> + TYPE_MIN_VALUE (new_tree) = TYPE_MIN_VALUE (TYPE_MAIN_VARIANT
> (new_tree));
> + TYPE_MAX_VALUE (new_tree) = TYPE_MAX_VALUE (TYPE_MAIN_VARIANT
> (new_tree));
> + }
> + else
> + {
> + t = TYPE_MIN_VALUE (new_tree);
> + if (t && TREE_CODE (t) != INTEGER_CST)
> + walk_tree (&TYPE_MIN_VALUE (new_tree), copy_tree_body_r, id,
> NULL);
> +
> + t = TYPE_MAX_VALUE (new_tree);
> + if (t && TREE_CODE (t) != INTEGER_CST)
> + walk_tree (&TYPE_MAX_VALUE (new_tree), copy_tree_body_r, id,
> NULL);
> + }
> return new_tree;
>
> case FUNCTION_TYPE:
> - TREE_TYPE (new_tree) = remap_type (TREE_TYPE (new_tree), id);
> - walk_tree (&TYPE_ARG_TYPES (new_tree), copy_tree_body_r, id, NULL);
> + if (TYPE_MAIN_VARIANT (new_tree) != new_tree
> + && TREE_TYPE (type) == TREE_TYPE (TYPE_MAIN_VARIANT (type)))
> + TREE_TYPE (new_tree) = TREE_TYPE (TYPE_MAIN_VARIANT (new_tree));
> + else
> + TREE_TYPE (new_tree) = remap_type (TREE_TYPE (new_tree), id);
> + if (TYPE_MAIN_VARIANT (new_tree) != new_tree
> + && TYPE_ARG_TYPES (type) == TYPE_ARG_TYPES (TYPE_MAIN_VARIANT
> (type)))
> + TYPE_ARG_TYPES (new_tree) = TYPE_ARG_TYPES (TYPE_MAIN_VARIANT
> (new_tree));
> + else
> + walk_tree (&TYPE_ARG_TYPES (new_tree), copy_tree_body_r, id, NULL);
> return new_tree;
>
> case ARRAY_TYPE:
> - TREE_TYPE (new_tree) = remap_type (TREE_TYPE (new_tree), id);
> - TYPE_DOMAIN (new_tree) = remap_type (TYPE_DOMAIN (new_tree), id);
> + if (TYPE_MAIN_VARIANT (new_tree) != new_tree
> + && TREE_TYPE (type) == TREE_TYPE (TYPE_MAIN_VARIANT (type)))
> + TREE_TYPE (new_tree) = TREE_TYPE (TYPE_MAIN_VARIANT (new_tree));
> +
> + if (TYPE_MAIN_VARIANT (new_tree) != new_tree)
> + {
> + gcc_checking_assert (TYPE_DOMAIN (type) == TYPE_DOMAIN
> (TYPE_MAIN_VARIANT (type)));
> + TYPE_DOMAIN (new_tree) = TYPE_DOMAIN (TYPE_MAIN_VARIANT (new_tree));
> + }
> + else
> + TYPE_DOMAIN (new_tree) = remap_type (TYPE_DOMAIN (new_tree), id);
> break;
>
> case RECORD_TYPE:
> case UNION_TYPE:
> case QUAL_UNION_TYPE:
> - {
> - tree f, nf = NULL;
> -
> - for (f = TYPE_FIELDS (new_tree); f ; f = DECL_CHAIN (f))
> - {
> - t = remap_decl (f, id);
> - DECL_CONTEXT (t) = new_tree;
> - DECL_CHAIN (t) = nf;
> - nf = t;
> - }
> - TYPE_FIELDS (new_tree) = nreverse (nf);
> - }
> + if (TYPE_MAIN_VARIANT (type) != type
> + && TYPE_FIELDS (type) == TYPE_FIELDS (TYPE_MAIN_VARIANT (type)))
> + TYPE_FIELDS (new_tree) = TYPE_FIELDS (TYPE_MAIN_VARIANT (new_tree));
> + else
> + {
> + tree f, nf = NULL;
> +
> + for (f = TYPE_FIELDS (new_tree); f ; f = DECL_CHAIN (f))
> + {
> + t = remap_decl (f, id);
> + DECL_CONTEXT (t) = new_tree;
> + DECL_CHAIN (t) = nf;
> + nf = t;
> + }
> + TYPE_FIELDS (new_tree) = nreverse (nf);
> + }
> break;
>
> case OFFSET_TYPE:
> @@ -500,8 +534,20 @@ remap_type_1 (tree type, copy_body_data
> gcc_unreachable ();
> }
>
> - walk_tree (&TYPE_SIZE (new_tree), copy_tree_body_r, id, NULL);
> - walk_tree (&TYPE_SIZE_UNIT (new_tree), copy_tree_body_r, id, NULL);
> + /* All variants of type share the same size, so use the already remaped
> data. */
> + if (TYPE_MAIN_VARIANT (new_tree) != new_tree)
> + {
> + gcc_checking_assert (TYPE_SIZE (type) == TYPE_SIZE (TYPE_MAIN_VARIANT
> (type)));
> + gcc_checking_assert (TYPE_SIZE_UNIT (type) == TYPE_SIZE_UNIT
> (TYPE_MAIN_VARIANT (type)));
> +
> + TYPE_SIZE (new_tree) = TYPE_SIZE (TYPE_MAIN_VARIANT (new_tree));
> + TYPE_SIZE_UNIT (new_tree) = TYPE_SIZE_UNIT (TYPE_MAIN_VARIANT
> (new_tree));
> + }
> + else
> + {
> + walk_tree (&TYPE_SIZE (new_tree), copy_tree_body_r, id, NULL);
> + walk_tree (&TYPE_SIZE_UNIT (new_tree), copy_tree_body_r, id, NULL);
> + }
>
> return new_tree;
> }
