On Wed, 13 Aug 2014, Richard Biener wrote:
>
> The following fixes the LTO bootstrap miscompares on the branches.
> They happen because type_hash_canon can end up returning different
> types (a main variant or a variant type) dependent on whether
> we garbage collected before or not.
>
> The patch makes us never return a variant type from type_hash_canon
> if we fed it a main variant (and vice versa).
Ok, that was the wrong idea - type_hash_canon says:
tree
type_hash_canon (unsigned int hashcode, tree type)
{
tree t1;
/* The hash table only contains main variants, so ensure that's what we're
being passed. */
gcc_assert (TYPE_MAIN_VARIANT (type) == type);
ok - but if I place an assert that we only get main variants from it
as well like with
@@ -6759,6 +6759,7 @@ type_hash_canon (unsigned int hashcode,
t1 = type_hash_lookup (hashcode, type);
if (t1 != 0)
{
+ gcc_assert (TYPE_MAIN_VARIANT (t1) == t1);
if (GATHER_STATISTICS)
{
tree_code_counts[(int) TREE_CODE (type)]--;
then we ICE very quickly because the C++ frontend happily modifies
array types that are already recorded in the type-hash-canon hashtable.
The following patch tries to avoid that and puts in the above assert.
Sofar the patch survived building stage2 in a LTO bootstrap on the
4.9 branch, full testing is scheduled for trunk. The patch is
aimed at all active branches (where LTO bootstrap is currently
broken because of it).
Jason, are you happy with that (esp. ripping out the odd
type completion stuff that also messes with types recorded in
said hashtable)? That code was put in to fix PR57325 (the testcase
doesn't ICE after the patch but is rejected) - I suppose the
fix should have been in the processing_template_decl path only
that doesn't go through the type-hash-canon machinery?
Otherwise can you please take over?
Thanks,
Richard.
2014-08-13 Richard Biener <rguent...@suse.de>
PR middle-end/62077
* tree.c (type_hash_canon): Assert we only get main
variants out of the hash.
cp/
* tree.c (build_cplus_array_type): Properly build
qualified types.
Index: gcc/tree.c
===================================================================
--- gcc/tree.c (revision 213814)
+++ gcc/tree.c (working copy)
@@ -6759,6 +6759,7 @@ type_hash_canon (unsigned int hashcode,
t1 = type_hash_lookup (hashcode, type);
if (t1 != 0)
{
+ gcc_assert (TYPE_MAIN_VARIANT (t1) == t1);
if (GATHER_STATISTICS)
{
tree_code_counts[(int) TREE_CODE (type)]--;
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c (revision 213814)
+++ gcc/cp/tree.c (working copy)
@@ -824,7 +824,11 @@ build_cplus_array_type (tree elt_type, t
build_cplus_array_type
(TYPE_CANONICAL (elt_type),
index_type ? TYPE_CANONICAL (index_type) : index_type);
- t = build_array_type (elt_type, index_type);
+ if (elt_type != TYPE_MAIN_VARIANT (elt_type))
+ t = build_cplus_array_type (TYPE_MAIN_VARIANT (elt_type),
+ index_type);
+ else
+ t = build_array_type (elt_type, index_type);
}
/* Push these needs up so that initialization takes place
@@ -840,37 +844,17 @@ build_cplus_array_type (tree elt_type, t
element type as well, so fix it up if needed. */
if (elt_type != TYPE_MAIN_VARIANT (elt_type))
{
- tree m = build_cplus_array_type (TYPE_MAIN_VARIANT (elt_type),
- index_type);
-
- if (TYPE_MAIN_VARIANT (t) != m)
+ tree t1;
+ for (t1 = TYPE_MAIN_VARIANT (t); t1; t1 = TYPE_NEXT_VARIANT (t1))
+ if (TREE_TYPE (t1) == elt_type)
+ {
+ t = t1;
+ break;
+ }
+ if (!t1)
{
- if (COMPLETE_TYPE_P (TREE_TYPE (t)) && !COMPLETE_TYPE_P (m))
- {
- /* m was built before the element type was complete, so we
- also need to copy the layout info from t. We might
- end up doing this multiple times if t is an array of
- unknown bound. */
- tree size = TYPE_SIZE (t);
- tree size_unit = TYPE_SIZE_UNIT (t);
- unsigned int align = TYPE_ALIGN (t);
- unsigned int user_align = TYPE_USER_ALIGN (t);
- enum machine_mode mode = TYPE_MODE (t);
- for (tree var = m; var; var = TYPE_NEXT_VARIANT (var))
- {
- TYPE_SIZE (var) = size;
- TYPE_SIZE_UNIT (var) = size_unit;
- TYPE_ALIGN (var) = align;
- TYPE_USER_ALIGN (var) = user_align;
- SET_TYPE_MODE (var, mode);
- TYPE_NEEDS_CONSTRUCTING (var) = needs_ctor;
- TYPE_HAS_NONTRIVIAL_DESTRUCTOR (var) = needs_dtor;
- }
- }
-
- TYPE_MAIN_VARIANT (t) = m;
- TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (m);
- TYPE_NEXT_VARIANT (m) = t;
+ t = build_variant_type_copy (t);
+ TREE_TYPE (t) = elt_type;
}
}
