Hi,
here is updated patch which I finally comitted today. It addresses all the
comments
and also fixes one nasty bug that really cost me a lot of time to understand.
+ /* LTO type merging does not make any difference between
+ component pointer types. We may have
+
+ struct foo {int *a;};
+
+ as TYPE_CANONICAL of
+
+ struct bar {float *a;};
+
+ Because accesses to int * and float * do not alias, we would get
+ false negative when accessing the same memory location by
+ float ** and bar *. We thus record the canonical type as:
+
+ struct {void *a;};
+
+ void * is special cased and works as a universal pointer type.
+ Accesses to it conflicts with accesses to any other pointer
+ type. */
This problem manifested itself only as a lto-bootstrap miscompare on 32bit
build and I spent a lot of time localizing the wrong code since it reproduces
only in quite large programs where we get conficts in canonical type merging
like this.
The patch thus updates record_component_aliases to substitute void_ptr_type for
all pointer types. I re-did the stats. Now the improvement on dealII is 14%
that is quite a bit lower than earlier, but still substantial. Since we have
voidptr globing counter, I know that the number of disambiguations would go at
least 19% up if we did not do it.
THere is a lot of low hanging fruit in that area now, but the real solution is
to
track types that needs to be merge by fortran rules and don't do all this fancy
globing for C/C++ types. I will open branch for IPA work and try to prepare
this
for next stage 1.
bootstrapped/regtested x86_64-linux and ppc64-linux, earlier version tested on
i386-linux
and also on some bigger apps, committed
Note that we still have bootstrap miscompare with LTO build and
--disable-checking,
I am looking for that now. Additoinally after fixing the ICEs preventing us to
build
the gnat1 binary, gnat1 aborts. Both these are independent of the patch.
Honza
* lto.c (iterative_hash_canonical_type): Always recurse for pointers.
(gimple_register_canonical_type_1): Check that pointers do not get
canonical types.
(gimple_register_canonical_type): Do not register pointers.
* tree.c (build_pointer_type_for_mode,build_reference_type_for_mode):
In LTO we do not compute TYPE_CANONICAL of pointers.
(gimple_canonical_types_compatible_p): Improve coments; sanity check
that pointers do not have canonical type that would make us believe
they are different.
* alias.c (get_alias_set): Do structural type equality on pointers;
enable pointer path for LTO; also glob pointer to vector with pointer
to vector element; glob pointers and references for LTO; do more strict
sanity checking about build_pointer_type returning the canonical type
which is also the main variant.
(record_component_aliases): When component type is pointer and we
do LTO; record void_type_node alias set.
Index: tree.c
===================================================================
--- tree.c (revision 230714)
+++ tree.c (working copy)
@@ -7919,7 +7919,8 @@ build_pointer_type_for_mode (tree to_typ
TYPE_NEXT_PTR_TO (t) = TYPE_POINTER_TO (to_type);
TYPE_POINTER_TO (to_type) = t;
- if (TYPE_STRUCTURAL_EQUALITY_P (to_type))
+ /* During LTO we do not set TYPE_CANONICAL of pointers and references. */
+ if (TYPE_STRUCTURAL_EQUALITY_P (to_type) || in_lto_p)
SET_TYPE_STRUCTURAL_EQUALITY (t);
else if (TYPE_CANONICAL (to_type) != to_type || could_alias)
TYPE_CANONICAL (t)
@@ -7987,7 +7988,8 @@ build_reference_type_for_mode (tree to_t
TYPE_NEXT_REF_TO (t) = TYPE_REFERENCE_TO (to_type);
TYPE_REFERENCE_TO (to_type) = t;
- if (TYPE_STRUCTURAL_EQUALITY_P (to_type))
+ /* During LTO we do not set TYPE_CANONICAL of pointers and references. */
+ if (TYPE_STRUCTURAL_EQUALITY_P (to_type) || in_lto_p)
SET_TYPE_STRUCTURAL_EQUALITY (t);
else if (TYPE_CANONICAL (to_type) != to_type || could_alias)
TYPE_CANONICAL (t)
@@ -13224,7 +13226,9 @@ type_with_interoperable_signedness (cons
TBAA is concerned.
This function is used both by lto.c canonical type merging and by the
verifier. If TRUST_TYPE_CANONICAL we do not look into structure of types
- that have TYPE_CANONICAL defined and assume them equivalent. */
+ that have TYPE_CANONICAL defined and assume them equivalent. This is useful
+ only for LTO because only in these cases TYPE_CANONICAL equivalence
+ correspond to one defined by gimple_canonical_types_compatible_p. */
bool
gimple_canonical_types_compatible_p (const_tree t1, const_tree t2,
@@ -13265,9 +13269,19 @@ gimple_canonical_types_compatible_p (con
|| (type_with_alias_set_p (t1) && type_with_alias_set_p (t2)));
/* If the types have been previously registered and found equal
they still are. */
+
if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t2)
&& trust_type_canonical)
- return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
+ {
+ /* Do not use TYPE_CANONICAL of pointer types. For LTO streamed types
+ they are always NULL, but they are set to non-NULL for types
+ constructed by build_pointer_type and variants. In this case the
+ TYPE_CANONICAL is more fine grained than the equivalnce we test (where
+ all pointers are considered equal. Be sure to not return false
+ negatives. */
+ gcc_checking_assert (!POINTER_TYPE_P (t1) && !POINTER_TYPE_P (t2));
+ return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
+ }
/* Can't be the same type if the types don't have the same code. */
enum tree_code code = tree_code_for_canonical_type_merging (TREE_CODE (t1));
Index: lto/lto.c
===================================================================
--- lto/lto.c (revision 230714)
+++ lto/lto.c (working copy)
@@ -388,8 +388,13 @@ iterative_hash_canonical_type (tree type
/* All type variants have same TYPE_CANONICAL. */
type = TYPE_MAIN_VARIANT (type);
+
+ /* We do not compute TYPE_CANONICAl of POINTER_TYPE because the aliasing
+ code never use it anyway. */
+ if (POINTER_TYPE_P (type))
+ v = hash_canonical_type (type);
/* An already processed type. */
- if (TYPE_CANONICAL (type))
+ else if (TYPE_CANONICAL (type))
{
type = TYPE_CANONICAL (type);
v = gimple_canonical_type_hash (type);
@@ -437,7 +442,9 @@ gimple_register_canonical_type_1 (tree t
{
void **slot;
- gcc_checking_assert (TYPE_P (t) && !TYPE_CANONICAL (t));
+ gcc_checking_assert (TYPE_P (t) && !TYPE_CANONICAL (t)
+ && type_with_alias_set_p (t)
+ && !POINTER_TYPE_P (t));
slot = htab_find_slot_with_hash (gimple_canonical_types, t, hash, INSERT);
if (*slot)
@@ -470,7 +477,7 @@ gimple_register_canonical_type_1 (tree t
static void
gimple_register_canonical_type (tree t)
{
- if (TYPE_CANONICAL (t) || !type_with_alias_set_p (t))
+ if (TYPE_CANONICAL (t) || !type_with_alias_set_p (t) || POINTER_TYPE_P (t))
return;
/* Canonical types are same among all complete variants. */
Index: alias.c
===================================================================
--- alias.c (revision 230714)
+++ alias.c (working copy)
@@ -869,13 +869,23 @@ get_alias_set (tree t)
set = lang_hooks.get_alias_set (t);
if (set != -1)
return set;
- return 0;
+ /* Handle structure type equality for pointer types. This is easy
+ to do, because the code bellow ignore canonical types on these anyway.
+ This is important for LTO, where TYPE_CANONICAL for pointers can not
+ be meaningfuly computed by the frotnend. */
+ if (!POINTER_TYPE_P (t))
+ {
+ /* In LTO we set canonical types for all types where it makes
+ sense to do so. Double check we did not miss some type. */
+ gcc_checking_assert (!in_lto_p || !type_with_alias_set_p (t));
+ return 0;
+ }
+ }
+ else
+ {
+ t = TYPE_CANONICAL (t);
+ gcc_checking_assert (!TYPE_STRUCTURAL_EQUALITY_P (t));
}
-
- t = TYPE_CANONICAL (t);
-
- /* The canonical type should not require structural equality checks. */
- gcc_checking_assert (!TYPE_STRUCTURAL_EQUALITY_P (t));
/* If this is a type with a known alias set, return it. */
if (TYPE_ALIAS_SET_KNOWN_P (t))
@@ -952,20 +962,23 @@ get_alias_set (tree t)
ptr_type_node but that is a bad idea, because it prevents disabiguations
in between pointers. For Firefox this accounts about 20% of all
disambiguations in the program. */
- else if (POINTER_TYPE_P (t) && t != ptr_type_node && !in_lto_p)
+ else if (POINTER_TYPE_P (t) && t != ptr_type_node)
{
tree p;
auto_vec <bool, 8> reference;
/* Unnest all pointers and references.
- We also want to make pointer to array equivalent to pointer to its
- element. So skip all array types, too. */
+ We also want to make pointer to array/vector equivalent to pointer to
+ its element (see the reasoning above). Skip all those types, too. */
for (p = t; POINTER_TYPE_P (p)
- || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p));
+ || (TREE_CODE (p) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (p))
+ || TREE_CODE (p) == VECTOR_TYPE;
p = TREE_TYPE (p))
{
if (TREE_CODE (p) == REFERENCE_TYPE)
- reference.safe_push (true);
+ /* In LTO we want languages that use references to be compatible
+ with languages that use pointers. */
+ reference.safe_push (true && !in_lto_p);
if (TREE_CODE (p) == POINTER_TYPE)
reference.safe_push (false);
}
@@ -981,7 +994,7 @@ get_alias_set (tree t)
set = get_alias_set (ptr_type_node);
else
{
- /* Rebuild pointer type from starting from canonical types using
+ /* Rebuild pointer type starting from canonical types using
unqualified pointers and references only. This way all such
pointers will have the same alias set and will conflict with
each other.
@@ -998,9 +1011,15 @@ get_alias_set (tree t)
p = build_reference_type (p);
else
p = build_pointer_type (p);
- p = TYPE_CANONICAL (TYPE_MAIN_VARIANT (p));
+ gcc_checking_assert (p == TYPE_MAIN_VARIANT (p));
+ /* build_pointer_type should always return the canonical type.
+ For LTO TYPE_CANOINCAL may be NULL, because we do not compute
+ them. Be sure that frontends do not glob canonical types of
+ pointers in unexpected way and that p == TYPE_CANONICAL (p)
+ in all other cases. */
+ gcc_checking_assert (!TYPE_CANONICAL (p)
+ || p == TYPE_CANONICAL (p));
}
- gcc_checking_assert (TYPE_CANONICAL (p) == p);
/* Assign the alias set to both p and t.
We can not call get_alias_set (p) here as that would trigger
@@ -1015,11 +1034,12 @@ get_alias_set (tree t)
}
}
}
- /* In LTO the rules above needs to be part of canonical type machinery.
- For now just punt. */
- else if (POINTER_TYPE_P (t)
- && t != TYPE_CANONICAL (ptr_type_node) && in_lto_p)
- set = get_alias_set (TYPE_CANONICAL (ptr_type_node));
+ /* Alias set of ptr_type_node is special and serve as universal pointer which
+ is TBAA compatible with every other pointer type. Be sure we have the
+ alias set built even for LTO which otherwise keeps all TYPE_CANONICAL
+ of pointer types NULL. */
+ else if (t == ptr_type_node)
+ set = new_alias_set ();
/* Otherwise make a new alias set for this type. */
else
@@ -1155,7 +1175,42 @@ record_component_aliases (tree type)
case QUAL_UNION_TYPE:
for (field = TYPE_FIELDS (type); field != 0; field = DECL_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL && !DECL_NONADDRESSABLE_P (field))
- record_alias_subset (superset, get_alias_set (TREE_TYPE (field)));
+ {
+ /* LTO type merging does not make any difference between
+ component pointer types. We may have
+
+ struct foo {int *a;};
+
+ as TYPE_CANONICAL of
+
+ struct bar {float *a;};
+
+ Because accesses to int * and float * do not alias, we would get
+ false negative when accessing the same memory location by
+ float ** and bar *. We thus record the canonical type as:
+
+ struct {void *a;};
+
+ void * is special cased and works as a universal pointer type.
+ Accesses to it conflicts with accesses to any other pointer
+ type. */
+ tree t = TREE_TYPE (field);
+ if (in_lto_p)
+ {
+ /* VECTOR_TYPE and ARRAY_TYPE share the alias set with their
+ element type and that type has to be normalized to void *,
+ too, in the case it is a pointer. */
+ while ((TREE_CODE (t) == ARRAY_TYPE
+ && (!COMPLETE_TYPE_P (t)
+ || TYPE_NONALIASED_COMPONENT (t)))
+ || TREE_CODE (t) == VECTOR_TYPE)
+ t = TREE_TYPE (t);
+ if (POINTER_TYPE_P (t))
+ t = ptr_type_node;
+ }
+
+ record_alias_subset (superset, get_alias_set (t));
+ }
break;
case COMPLEX_TYPE:
