On Thu, Mar 21, 2019 at 11:25:38AM +0100, Richard Biener wrote:
> On Thu, 21 Mar 2019, Jakub Jelinek wrote:
>
> > On Thu, Mar 21, 2019 at 11:13:41AM +0100, Richard Biener wrote:
> > > > Would you prefer to have say add_tree_to_fld_list that does what it
> > > > does now
> > > > and has just 2 callers in find_decls_types_r and a new
> > > > maybe_add_tree_to_fld_list that would do
> > > > if (!fld->pset.add (t))
> > > > add_tree_to_fld_list (t, fld);
> > > > and change all the other add_tree_to_fld_list callers?
> > >
> > > Not sure if that's needed, if you leave find_decls_types_r alone then
> > > always doing the check should work, no?
> >
> > Well, find_decls_types_r uses add_tree_to_fld_list, so the if (fld->pset.add
> > (t)) return; can't be in that function. If we want to simplify all the
> > other callers, so that they don't need to do it manually, then the only
> > options I see is call some other function in those spots that does this
> > additional check, or call another function that doesn't do that in the
> > two spots of find_decls_types_r, or add another argument to the function
> > say with default argument and test that argument before the fld->pset.add.
> > E.g. , bool do_add = true and if (do_add && fld->pset.add (t)) return;
> > and adjust the two calls in find_decls_types_r to call
> > add_tree_to_fld_list (t, fld, false);
> > Or keep the patch as is, do the tests before each but the two
> > add_tree_to_fld_list calls.
>
> I'd have kept the patch but made the add_tree_to_fld_list following
> the new fld->pset.add (t) calls conditional on the result.
Ok, so incremental diff:
--- gcc/tree.c 2019-03-20 20:16:12.277091827 +0100
+++ gcc/tree.c 2019-03-21 11:33:17.008936452 +0100
@@ -5215,8 +5215,8 @@
if (inner_type)
TREE_TYPE (v) = inner_type;
gcc_checking_assert (fld_type_variant_equal_p (t,v, inner_type));
- fld->pset.add (v);
- add_tree_to_fld_list (v, fld);
+ if (!fld->pset.add (v))
+ add_tree_to_fld_list (v, fld);
return v;
}
@@ -5324,8 +5324,8 @@
copy = build_distinct_type_copy (t);
/* It is possible that type was not seen by free_lang_data yet. */
- fld->pset.add (copy);
- add_tree_to_fld_list (copy, fld);
+ if (!fld->pset.add (copy))
+ add_tree_to_fld_list (copy, fld);
TYPE_SIZE (copy) = NULL;
TYPE_USER_ALIGN (copy) = 0;
TYPE_SIZE_UNIT (copy) = NULL;
@@ -5480,8 +5480,8 @@
& ~TYPE_QUAL_CONST
& ~TYPE_QUAL_VOLATILE;
TREE_VALUE (p) = build_qualified_type (arg_type, quals);
- fld->pset.add (TREE_VALUE (p));
- free_lang_data_in_type (TREE_VALUE (p), fld);
+ if (!fld->pset.add (TREE_VALUE (p)))
+ free_lang_data_in_type (TREE_VALUE (p), fld);
}
/* C++ FE uses TREE_PURPOSE to store initial values. */
TREE_PURPOSE (p) = NULL;
and full patch below? Ok for trunk if it passes bootstrap/regtest?
2019-03-21 Jan Hubicka <hubi...@ucw.cz>
Jakub Jelinek <ja...@redhat.com>
PR lto/89692
* tree.c (fld_type_variant, fld_incomplete_type_of,
fld_process_array_type): Call fld->pset.add and don't call
add_tree_to_fld_list if it returns true.
(free_lang_data_in_type): Likewise. Purge non-marked types from
TYPE_NEXT_VARIANT list.
(find_decls_types_r): Call fld_worklist_push for TYPE_CANONICAL (t).
* g++.dg/other/pr89692.C: New test.
--- gcc/tree.c.jj 2019-03-20 12:24:56.443322228 +0100
+++ gcc/tree.c 2019-03-21 11:33:17.008936452 +0100
@@ -5215,7 +5215,8 @@ fld_type_variant (tree first, tree t, st
if (inner_type)
TREE_TYPE (v) = inner_type;
gcc_checking_assert (fld_type_variant_equal_p (t,v, inner_type));
- add_tree_to_fld_list (v, fld);
+ if (!fld->pset.add (v))
+ add_tree_to_fld_list (v, fld);
return v;
}
@@ -5253,7 +5254,8 @@ fld_process_array_type (tree t, tree t2,
array = build_array_type_1 (t2, TYPE_DOMAIN (t),
TYPE_TYPELESS_STORAGE (t), false);
TYPE_CANONICAL (array) = TYPE_CANONICAL (t);
- add_tree_to_fld_list (array, fld);
+ if (!fld->pset.add (array))
+ add_tree_to_fld_list (array, fld);
}
return array;
}
@@ -5298,7 +5300,8 @@ fld_incomplete_type_of (tree t, struct f
TYPE_REF_CAN_ALIAS_ALL (t));
gcc_assert (TYPE_CANONICAL (t2) != t2
&& TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)));
- add_tree_to_fld_list (first, fld);
+ if (!fld->pset.add (first))
+ add_tree_to_fld_list (first, fld);
return fld_type_variant (first, t, fld);
}
return t;
@@ -5321,7 +5324,8 @@ fld_incomplete_type_of (tree t, struct f
copy = build_distinct_type_copy (t);
/* It is possible that type was not seen by free_lang_data yet. */
- add_tree_to_fld_list (copy, fld);
+ if (!fld->pset.add (copy))
+ add_tree_to_fld_list (copy, fld);
TYPE_SIZE (copy) = NULL;
TYPE_USER_ALIGN (copy) = 0;
TYPE_SIZE_UNIT (copy) = NULL;
@@ -5445,6 +5449,18 @@ free_lang_data_in_type (tree type, struc
TYPE_NEEDS_CONSTRUCTING (type) = 0;
+ /* Purge non-marked variants from the variants chain, so that they
+ don't reappear in the IL after free_lang_data. */
+ while (TYPE_NEXT_VARIANT (type)
+ && !fld->pset.contains (TYPE_NEXT_VARIANT (type)))
+ {
+ tree t = TYPE_NEXT_VARIANT (type);
+ TYPE_NEXT_VARIANT (type) = TYPE_NEXT_VARIANT (t);
+ /* Turn the removed types into distinct types. */
+ TYPE_MAIN_VARIANT (t) = t;
+ TYPE_NEXT_VARIANT (t) = NULL_TREE;
+ }
+
if (TREE_CODE (type) == FUNCTION_TYPE)
{
TREE_TYPE (type) = fld_simplified_type (TREE_TYPE (type), fld);
@@ -5464,7 +5480,8 @@ free_lang_data_in_type (tree type, struc
& ~TYPE_QUAL_CONST
& ~TYPE_QUAL_VOLATILE;
TREE_VALUE (p) = build_qualified_type (arg_type, quals);
- free_lang_data_in_type (TREE_VALUE (p), fld);
+ if (!fld->pset.add (TREE_VALUE (p)))
+ free_lang_data_in_type (TREE_VALUE (p), fld);
}
/* C++ FE uses TREE_PURPOSE to store initial values. */
TREE_PURPOSE (p) = NULL;
@@ -5886,8 +5903,7 @@ find_decls_types_r (tree *tp, int *ws, v
ctx = BLOCK_SUPERCONTEXT (ctx);
fld_worklist_push (ctx, fld);
}
- /* Do not walk TYPE_CANONICAL. We do not stream it and thus do not
- and want not to reach unused types this way. */
+ fld_worklist_push (TYPE_CANONICAL (t), fld);
if (RECORD_OR_UNION_TYPE_P (t) && TYPE_BINFO (t))
{
--- gcc/testsuite/g++.dg/other/pr89692.C.jj 2019-03-20 20:20:10.233278697
+0100
+++ gcc/testsuite/g++.dg/other/pr89692.C 2019-03-20 20:19:53.193551778
+0100
@@ -0,0 +1,20 @@
+// PR lto/89692
+// { dg-do compile }
+// { dg-require-effective-target lto }
+// { dg-options "-flto -O2" }
+
+struct S {
+ short int a, b;
+ unsigned char c : 1;
+};
+
+bool
+foo (void)
+{
+ unsigned char d[sizeof (S)] = { 0 };
+ S e;
+
+ __builtin_memcpy (&e, d, sizeof (d));
+
+ return e.c == d[0];
+}
Jakub
