Calculate TYPE_CANONICAL only for types that can be accessed in memory

Calculate TYPE_CANONICAL only for types that can be accessed in memory

[Jan Hubicka]

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)

Re: Calculate TYPE_CANONICAL only for types that can be accessed in memory

[Richard Biener]

On Thu, 21 May 2015, Jan Hubicka wrote:

> 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.

Yes, that notion makes sense.

> 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;

Just occured to me that it might make sense to remove the
FUNCTION/METHOD_TYPE case in useless_type_conversion_p (I wonder
in which cases we enter up in that path...).

> +  /* 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

matching

> +	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

gcc_checking_assert, like the one in the hashing path?

> +	      || ((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))))));

Now we have it spelled out 4 times ... makes sense to create a new
macro for it?  (though I cannot think of a good name...
UNACCESSIBLE_TYPE_P ()?)


Otherwise looks ok to me.

Thanks,
Richard.

Re: Calculate TYPE_CANONICAL only for types that can be accessed in memory

[Jan Hubicka]

> Now we have it spelled out 4 times ... makes sense to create a new
> macro for it?  (though I cannot think of a good name...
> UNACCESSIBLE_TYPE_P ()?)

Yep, actually I already made that version of patch yesterday but then got
hooked by beers.  This is better version (also with more sensible comments).
I will commit it at afternoon if you have no further comments.


	* lto.c (hash_canonical_type): Be sure we hash only types that
	need alias set.
	(gimple_register_canonical_type_1): Do not produce canonical
	types for types that do not need alias sets.
	* tree.c (gimple_canonical_types_compatible_p): Sanity check that
	we do not try to compute canonical type for type that does not need
	alias set.
	(verify_type): Drop FIXME for METHOD_TYPE, update FIXME for
	FUNCITON_TYPE.
	* tree.h (type_with_alias_set_p): New.
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 223508)
+++ lto/lto.c	(working copy)
@@ -309,6 +309,12 @@ hash_canonical_type (tree type)
 {
   inchash::hash hstate;
 
+  /* We compute alias sets only for types that needs them.
+     Be sure we do not recurse to something else as we can not hash incomplete
+     types in a way they would have same hash value as compatible complete
+     types.  */
+  gcc_checking_assert (type_with_alias_set_p (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
@@ -493,7 +495,7 @@ gimple_register_canonical_type_1 (tree t
 static void
 gimple_register_canonical_type (tree t)
 {
-  if (TYPE_CANONICAL (t))
+  if (TYPE_CANONICAL (t) || !type_with_alias_set_p (t))
     return;
 
   gimple_register_canonical_type_1 (t, hash_canonical_type (t));
Index: tree.c
===================================================================
--- tree.c	(revision 223508)
+++ tree.c	(working copy)
@@ -12720,6 +12720,23 @@ 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
+	      || (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)
@@ -12939,10 +12953,9 @@ verify_type (const_tree t)
   /* Method and function types can not be used to address memory and thus
      TYPE_CANONICAL really matters only for determining useless conversions.
 
-     FIXME: C++ FE does not agree with gimple_canonical_types_compatible_p
-     here.  gimple_canonical_types_compatible_p calls comp_type_attributes
-     while for C++ FE the attributes does not make difference.  */
-  else if (TREE_CODE (t) == FUNCTION_TYPE || TREE_CODE (t) == METHOD_TYPE)
+     FIXME: C++ FE produce declarations of builtin functions that are not
+     compatible with main variants.  */
+  else if (TREE_CODE (t) == FUNCTION_TYPE)
     ;
   else if (t != ct
 	   /* FIXME: gimple_canonical_types_compatible_p can not compare types
Index: tree.h
===================================================================
--- tree.h	(revision 223508)
+++ tree.h	(working copy)
@@ -5090,6 +5090,26 @@ int_bit_position (const_tree field)
 	  + wi::to_offset (DECL_FIELD_BIT_OFFSET (field))).to_shwi ();
 }
 
+/* Return true if it makes sense to consider alias set for a type T.  */
+
+inline bool
+type_with_alias_set_p (const_tree t)
+{
+  /* Function and method types are never accessed as memory locations.  */
+  if (TREE_CODE (t) == FUNCTION_TYPE || TREE_CODE (t) == METHOD_TYPE)
+    return false;
+
+  if (COMPLETE_TYPE_P (t))
+    return true;
+
+  /* Incomplete types can not be accessed in general except for arrays
+     where we can fetch its element despite we have no array bounds.  */
+  if (TREE_CODE (t) == ARRAY_TYPE && COMPLETE_TYPE_P (TREE_TYPE (t)))
+    return true;
+
+  return false;
+}
+
 extern void gt_ggc_mx (tree &);
 extern void gt_pch_nx (tree &);
 extern void gt_pch_nx (tree &, gt_pointer_operator, void *);

Re: Calculate TYPE_CANONICAL only for types that can be accessed in memory

[Jan Hubicka]

> > +  /* 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;
> 
> Just occured to me that it might make sense to remove the
> FUNCTION/METHOD_TYPE case in useless_type_conversion_p (I wonder
> in which cases we enter up in that path...).

Yes, I think that is unreachable (it was used only by that ignored code path in
ipa-symtab.c) and I also had patch for it somewhere.  Will look it up at
afternoon (I am teaching at morning).
I think useless_type_conversion_p should also just work on types that do have
memory representation.

Honza

Re: Calculate TYPE_CANONICAL only for types that can be accessed in memory

[H.J. Lu]

On Fri, May 22, 2015 at 5:00 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Now we have it spelled out 4 times ... makes sense to create a new
>> macro for it?  (though I cannot think of a good name...
>> UNACCESSIBLE_TYPE_P ()?)
>
> Yep, actually I already made that version of patch yesterday but then got
> hooked by beers.  This is better version (also with more sensible comments).
> I will commit it at afternoon if you have no further comments.
>
>
>         * lto.c (hash_canonical_type): Be sure we hash only types that
>         need alias set.
>         (gimple_register_canonical_type_1): Do not produce canonical
>         types for types that do not need alias sets.
>         * tree.c (gimple_canonical_types_compatible_p): Sanity check that
>         we do not try to compute canonical type for type that does not need
>         alias set.
>         (verify_type): Drop FIXME for METHOD_TYPE, update FIXME for
>         FUNCITON_TYPE.
>         * tree.h (type_with_alias_set_p): New.

This caused:

https://gcc.gnu.org/ml/gcc-cvs/2015-05/msg00948.html

H.J.

Re: Calculate TYPE_CANONICAL only for types that can be accessed in memory

[H.J. Lu]

On Sun, May 24, 2015 at 5:49 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, May 22, 2015 at 5:00 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> Now we have it spelled out 4 times ... makes sense to create a new
>>> macro for it?  (though I cannot think of a good name...
>>> UNACCESSIBLE_TYPE_P ()?)
>>
>> Yep, actually I already made that version of patch yesterday but then got
>> hooked by beers.  This is better version (also with more sensible comments).
>> I will commit it at afternoon if you have no further comments.
>>
>>
>>         * lto.c (hash_canonical_type): Be sure we hash only types that
>>         need alias set.
>>         (gimple_register_canonical_type_1): Do not produce canonical
>>         types for types that do not need alias sets.
>>         * tree.c (gimple_canonical_types_compatible_p): Sanity check that
>>         we do not try to compute canonical type for type that does not need
>>         alias set.
>>         (verify_type): Drop FIXME for METHOD_TYPE, update FIXME for
>>         FUNCITON_TYPE.
>>         * tree.h (type_with_alias_set_p): New.
>
> This caused:
>
> https://gcc.gnu.org/ml/gcc-cvs/2015-05/msg00948.html

I meant:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66273


-- 
H.J.