Hi,
this is next part of the series. It disables canonical type calculation for
incomplete types with exception of arrays based on claim that we do not have
good notion of those.
I can botostrap this with additional checks in alias.c that canonical types are
always present with LTO but I need fix to ICF that compare alias sets of types
it does not need to and trips incomplete types otherwise. I will push out
these fixes separately and incrementally add the fix. The purpose of those
checks is to avoid alias.c degenerating to structural equality path for no good
reason.
I tried the alternative to disable it on ARRAY_TYPES too and add avoid
recursion to those for fields. THis does not fly because we can have
ARRAY_REFS of incomplete types:
<array_ref 0x7ffff69c2968
type <pointer_type 0x7ffff6942150
type <integer_type 0x7ffff6cd50a8 char readonly string-flag QI
size <integer_cst 0x7ffff6ad7ca8 constant 8>
unit size <integer_cst 0x7ffff6ad7cc0 constant 1>
align 8 symtab -158253232 alias set 0 canonical type 0x7ffff6adb498
precision 8 min <integer_cst 0x7ffff6cd2090 -128> max <integer_cst
0x7ffff6cd2078 127>
pointer_to_this <pointer_type 0x7ffff6cd5150>>
readonly unsigned DI
size <integer_cst 0x7ffff6ad7bb8 constant 64>
unit size <integer_cst 0x7ffff6ad7bd0 constant 8>
align 64 symtab 0 alias set -1 canonical type 0x7ffff6af37e0
pointer_to_this <pointer_type 0x7ffff69479d8>>
readonly
arg 0 <mem_ref 0x7ffff69d1028
type <array_type 0x7ffff694b348 type <pointer_type 0x7ffff6942150>
BLK
align 64 symtab 0 alias set -1 structural equality
pointer_to_this <pointer_type 0x7ffff694b3f0>>
arg 0 <addr_expr 0x7ffff69ce380 type <pointer_type 0x7ffff694b3f0>
constant arg 0 <var_decl 0x7ffff6941480 reg_note_name>>
arg 1 <integer_cst 0x7ffff69b6678 constant 0>>
arg 1 <ssa_name 0x7ffff69c5630
type <integer_type 0x7ffff6adb690 int asm_written public SI
size <integer_cst 0x7ffff6ad7df8 constant 32>
unit size <integer_cst 0x7ffff6ad7e10 constant 4>
align 32 symtab -158421968 alias set 3 canonical type
0x7ffff6adb690 precision 32 min <integer_cst 0x7ffff6ad7db0 -2147483648> max
<integer_cst 0x7ffff6ad7dc8 2147483647>
pointer_to_this <pointer_type 0x7ffff6af37e0> reference_to_this
<reference_type 0x7ffff6942b28>>
visiteddef_stmt _103 = (int) _101;
version 103
ptr-info 0x7ffff69f04a0>
../../gcc/print-rtl.c:173:4>
and we compute alias set for it via:
#0 internal_error (gmsgid=0x1b86c8f "in %s, at %s:%d") at
../../gcc/diagnostic.c:1271
#1 0x00000000015e2416 in fancy_abort (file=0x167ea2a "../../gcc/alias.c",
line=823, function=0x167f7d6 <get_alias_set(tree_node*)::__FUNCTION__>
"get_alias_set")
at ../../gcc/diagnostic.c:1341
#2 0x00000000007109b9 in get_alias_set (t=0x7ffff694b2a0) at
../../gcc/alias.c:823
#3 0x000000000070fecf in component_uses_parent_alias_set_from
(t=0x7ffff69c2968) at ../../gcc/alias.c:607
#4 0x0000000000710497 in reference_alias_ptr_type_1 (t=0x7fffffffe068) at
../../gcc/alias.c:719
#5 0x00000000007107e8 in get_alias_set (t=0x7ffff69c2968) at
../../gcc/alias.c:799
#6 0x0000000000ebca97 in vn_reference_lookup (op=0x7ffff69c2968,
vuse=0x7ffff69ca798, kind=VN_WALKREWRITE, vnresult=0x0) at
../../gcc/tree-ssa-sccvn.c:2217
#7 0x0000000000ebea99 in visit_reference_op_load (lhs=0x7ffff69c5678,
op=0x7ffff69c2968, stmt=0x7ffff69cf730) at ../../gcc/tree-ssa-sccvn.c:3030
#8 0x0000000000ec05ec in visit_use (use=0x7ffff69c5678) at
../../gcc/tree-ssa-sccvn.c:3685
#9 0x0000000000ec1047 in process_scc (scc=...) at
../../gcc/tree-ssa-sccvn.c:3927
#10 0x0000000000ec1679 in extract_and_process_scc_for_name
(name=0x7ffff69c5678) at ../../gcc/tree-ssa-sccvn.c:4013
#11 0x0000000000ec1848 in DFS (name=0x7ffff69c5678) at
../../gcc/tree-ssa-sccvn.c:4065
#12 0x0000000000ec26d1 in cond_dom_walker::before_dom_children
(this=0x7fffffffe5a0, bb=0x7ffff69b9888) at ../../gcc/tree-ssa-sccvn.c:4345
#13 0x00000000014c05c0 in dom_walker::walk (this=0x7fffffffe5a0,
bb=0x7ffff69b9888) at ../../gcc/domwalk.c:188
#14 0x0000000000ec2b0e in run_scc_vn (default_vn_walk_kind_=VN_WALKREWRITE) at
../../gcc/tree-ssa-sccvn.c:4436
#15 0x0000000000e98d59 in (anonymous namespace)::pass_fre::execute
(this=0x1f621b0, fun=0x7ffff698db28) at ../../gcc/tree-ssa-pre.c:4972
#16 0x0000000000bb6c8f in execute_one_pass (pass=0x1f621b0) at
../../gcc/passes.c:2317
#17 0x0000000000bb6ede in execute_pass_list_1 (pass=0x1f621b0) at
../../gcc/passes.c:2370
#18 0x0000000000bb6f0f in execute_pass_list_1 (pass=0x1f61d90) at
../../gcc/passes.c:2371
#19 0x0000000000bb6f51 in execute_pass_list (fn=0x7ffff698db28, pass=0x1f61cd0)
at ../../gcc/passes.c:2381
#20 0x00000000007bb3f6 in cgraph_node::expand (this=0x7ffff695b000) at
../../gcc/cgraphunit.c:1895
#21 0x00000000007bba15 in expand_all_functions () at ../../gcc/cgraphunit.c:2031
#22 0x00000000007bc4e9 in symbol_table::compile (this=0x7ffff6adb000) at
../../gcc/cgraphunit.c:2384
#23 0x00000000006f846c in lto_main () at ../../gcc/lto/lto.c:3315
#24 0x0000000000cb465f in compile_file () at ../../gcc/toplev.c:594
#25 0x0000000000cb6bb8 in do_compile () at ../../gcc/toplev.c:2081
#26 0x0000000000cb6e04 in toplev::main (this=0x7fffffffe860, argc=33,
argv=0x7fffffffe968) at ../../gcc/toplev.c:2182
#27 0x00000000015c9739 in main (argc=33, argv=0x7fffffffe968) at
../../gcc/main.c:39
Though few lines down alias.c globs to element type:
if (TREE_CODE (t) == ARRAY_TYPE && !TYPE_NONALIASED_COMPONENT (t))
set = get_alias_set (TREE_TYPE (t));
I supose we can move it up and then skip calculation of those, but sollution
bellow makes sense to me. Type has TYPE_CANONICAL if it (or its parts) can be
accessed. Incomplete array fields can be accessed and thus need
TYPE_CANONICAL.
LTO bootstrapped on x86_64-linux, regtested with other patches, I am re-testing
in isolation OK?
Honza
* lto/lto.c (hash_canonical_type): Check that we hash only
complete types or incomplete arrays.
(gimple_register_canonical_type): Do not compute canonical type
of types where it does not matter.
* tree.c (gimple_canonical_types_compatible_p): Sanity check that
we do not compute canonical type based on incomplete type.
Index: lto/lto.c
===================================================================
--- lto/lto.c (revision 223490)
+++ lto/lto.c (working copy)
@@ -309,6 +309,13 @@ hash_canonical_type (tree type)
{
inchash::hash hstate;
+ /* We can hash only types that can be accessed in memory.
+ Those are complete types and arrays of incomplete type but complete
+ TREE_TYPE. Verify we do not recurse to something else. */
+ gcc_checking_assert (COMPLETE_TYPE_P (type)
+ || (TREE_CODE (type) == ARRAY_TYPE
+ && COMPLETE_TYPE_P (TREE_TYPE (type))));
+
/* Combine a few common features of types so that types are grouped into
smaller sets; when searching for existing matching types to merge,
only existing types having the same features as the new type will be
@@ -496,6 +499,22 @@ gimple_register_canonical_type (tree t)
if (TYPE_CANONICAL (t))
return;
+ /* No need for canonical types of functions and methods; those are never
+ accessed as memory locations. */
+ if (TREE_CODE (t) == FUNCTION_TYPE || TREE_CODE (t) == METHOD_TYPE)
+ return;
+ /* Incomplete structures/unions does not have good definition of
+ canonical type at inter-module level because they are compatible with
+ every complete type (as oposed to every complete type of same name
+ with non-LTO build). Do not compute canonical types for those, because
+ they can not be accessed anyway.
+
+ However canonical types of arrays matters, because even if outer dimension
+ is unknown, its elements still can be accessed. */
+ if (!COMPLETE_TYPE_P (t)
+ && (TREE_CODE (t) != ARRAY_TYPE || !COMPLETE_TYPE_P (TREE_TYPE (t))))
+ return;
+
gimple_register_canonical_type_1 (t, hash_canonical_type (t));
}
Index: tree.c
===================================================================
--- tree.c (revision 223490)
+++ tree.c (working copy)
@@ -12720,6 +12720,28 @@ gimple_canonical_types_compatible_p (con
if (t1 == NULL_TREE || t2 == NULL_TREE)
return false;
+ /* We consider complete types always compatible with incomplete type.
+ This does not make sense for canonical type calculation and thus we
+ need to ensure that we are never called on it.
+
+ FIXME: For more correctness the function probably should have three modes
+ 1) mode assuming that types are complete mathcing their structure
+ 2) mode allowing incomplete types but producing equivalence classes
+ and thus ignoring all info from complete types
+ 3) mode allowing incomplete types to match complete but checking
+ compatibility between complete types.
+
+ 1 and 2 can be used for canonical type calculation. 3 is the real
+ definition of type compatibility that can be used i.e. for warnings during
+ declaration merging. */
+
+ gcc_assert (!trust_type_canonical
+ || ((COMPLETE_TYPE_P (t1)
+ || (TREE_CODE (t1) == ARRAY_TYPE
+ && COMPLETE_TYPE_P (TREE_TYPE (t1))))
+ && (COMPLETE_TYPE_P (t2)
+ || (TREE_CODE (t2) == ARRAY_TYPE
+ && COMPLETE_TYPE_P (TREE_TYPE (t2))))));
/* If the types have been previously registered and found equal
they still are. */
if (TYPE_CANONICAL (t1) && TYPE_CANONICAL (t2)
