Teach gimple_canonical_types_compatible_p about incomplete types

Joseph, Richard,
this is patch implementing the ENUM/INGEGER globbing and also POINTER/REFERENCE
(though I don't know if that one follows by some standard rules).
Joseph, does the attached testcase make sense for you? Is it defined? It is my
first attempt to really interpret C standard to detail.

Ideally I would like to have testcases for all the globbing we do and reasoning
why it is needed.

Bootstraped/regtested ppc64le-linux. OK?

Honza

	* lto.c (hash_canonical_type): Use tree_code_for_canonical_type_merging.

	* tree.h (tree_code_for_canonical_type_merging): New function.
	* tree.c (gimple_canonical_types_compatible_p): Use
	tree_code_for_canonical_type_merging..
	* gcc.dg/lto/c-compatible-types_0.c: New testcase.
	* gcc.dg/lto/c-compatible-types_1.c: New testcase.

On May 30, 2015 12:56:26 AM GMT+02:00, Jan Hubicka <hubicka@ucw.cz> wrote:

>Index: tree.h
>===================================================================
>--- tree.h	(revision 223877)
>+++ tree.h	(working copy)
>@@ -4598,7 +4598,28 @@
> extern void DEBUG_FUNCTION verify_type (const_tree t);
>extern bool gimple_canonical_types_compatible_p (const_tree,
>const_tree,
> 						 bool trust_type_canonical = true);
>+/* Return simplified tree code of type that is used for canonical type
>merging.  */
>+inline enum tree_code
>+tree_code_for_canonical_type_merging (enum tree_code code)
>+{
>+  /* By C standard, each enumerated type shall be compatible with
>char,
>+     a signed integer, or an unsigned integer.  The choice of type is
>+     implementation defined (in our case it depends on -fshort-enum).

Please drop the mention of -fshort-enum as Joseph clarified.

thanks,

> 
>+     For this reason we make no distinction between ENUMERAL_TYPE and
>INTEGER
>+     type and compare only by their signedness and precision.  */
>+  if (code == ENUMERAL_TYPE)
>+    return INTEGER_TYPE;
>+  /* To allow inter-operability between languages having references
>and
>+     C, we consider reference types and pointers alike.  Note that
>this is
>+     not strictly necessary for C-Fortran 2008 interoperability
>because
>+     Fortran define C_PTR type that needs to be compatible with C
>pointers
>+     and we handle this one as ptr_type_node.  */
>+  if (code == REFERENCE_TYPE)
>+    return POINTER_TYPE;
>+  return code;
>+}
>+
> #define tree_map_eq tree_map_base_eq
> extern unsigned int tree_map_hash (const void *);
> #define tree_map_marked_p tree_map_base_marked_p

> On May 30, 2015 12:56:26 AM GMT+02:00, Jan Hubicka <hubicka@ucw.cz> wrote:
> 
> >Index: tree.h
> >===================================================================
> >--- tree.h	(revision 223877)
> >+++ tree.h	(working copy)
> >@@ -4598,7 +4598,28 @@
> > extern void DEBUG_FUNCTION verify_type (const_tree t);
> >extern bool gimple_canonical_types_compatible_p (const_tree,
> >const_tree,
> > 						 bool trust_type_canonical = true);
> >+/* Return simplified tree code of type that is used for canonical type
> >merging.  */
> >+inline enum tree_code
> >+tree_code_for_canonical_type_merging (enum tree_code code)
> >+{
> >+  /* By C standard, each enumerated type shall be compatible with
> >char,
> >+     a signed integer, or an unsigned integer.  The choice of type is
> >+     implementation defined (in our case it depends on -fshort-enum).
> 
> Please drop the mention of -fshort-enum as Joseph clarified.

I think the comment there is correct -fshort-enum will make us to pick different
integer types based on number of values, but they will always interoperable with
some normal integer type.

Honza
> 
> thanks,

On Sat, 30 May 2015, Jan Hubicka wrote:

> Joseph, does the attached testcase make sense for you? Is it defined? It is my
> first attempt to really interpret C standard to detail.

I suppose it's defined if unsigned int is the type chosen as compatible 
with that enum.  The test should be skipped for short_enums targets 
(arm-eabi bare metal) (you can't simply use -fno-short-enums as then that 
will fail the link-time compatibility checking).

> On Sat, 30 May 2015, Jan Hubicka wrote:
> 
> > Joseph, does the attached testcase make sense for you? Is it defined? It is my
> > first attempt to really interpret C standard to detail.
> 
> I suppose it's defined if unsigned int is the type chosen as compatible 
> with that enum.  The test should be skipped for short_enums targets 
> (arm-eabi bare metal) (you can't simply use -fno-short-enums as then that 
> will fail the link-time compatibility checking).

thanks. I did not notice we have -fshort-enum by default targets. I suppose we want:
/* { dg-xfail-if "" { arm-eabi-* } { "*" } { "" } } */

Honza
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

On Tue, 2 Jun 2015, Jan Hubicka wrote:

> > On Sat, 30 May 2015, Jan Hubicka wrote:
> > 
> > > Joseph, does the attached testcase make sense for you? Is it defined? It is my
> > > first attempt to really interpret C standard to detail.
> > 
> > I suppose it's defined if unsigned int is the type chosen as compatible 
> > with that enum.  The test should be skipped for short_enums targets 
> > (arm-eabi bare metal) (you can't simply use -fno-short-enums as then that 
> > will fail the link-time compatibility checking).
> 
> thanks. I did not notice we have -fshort-enum by default targets. I suppose we want:
> /* { dg-xfail-if "" { arm-eabi-* } { "*" } { "" } } */

Well, not that (which matches "eabi" against the vendor part of the 
triplet), but skip for the short_enums effective-target keyword.

> > thanks. I did not notice we have -fshort-enum by default targets. I suppose we want:
> > /* { dg-xfail-if "" { arm-eabi-* } { "*" } { "" } } */
> 
> Well, not that (which matches "eabi" against the vendor part of the 
> triplet), but skip for the short_enums effective-target keyword.
Ok. Did not know about short_enums (my dejagnu-fu is still very limited :( )

/* { dg-skip-if "require -fno-short-enums to work" {target short_enums} } */

Alternatively I suppose I can add enum value set to INT_MAX to force enum to be large.

Honza
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

On Sat, 30 May 2015, Jan Hubicka wrote:

> Joseph, Richard,
> this is patch implementing the ENUM/INGEGER globbing and also POINTER/REFERENCE
> (though I don't know if that one follows by some standard rules).
> Joseph, does the attached testcase make sense for you? Is it defined? It is my
> first attempt to really interpret C standard to detail.
> 
> Ideally I would like to have testcases for all the globbing we do and reasoning
> why it is needed.
> 
> Bootstraped/regtested ppc64le-linux. OK?

Works for me.  (what about BOOLEAN_TYPE?)

Thanks,
Richard.

> Honza
> 
> 	* lto.c (hash_canonical_type): Use tree_code_for_canonical_type_merging.
> 
> 	* tree.h (tree_code_for_canonical_type_merging): New function.
> 	* tree.c (gimple_canonical_types_compatible_p): Use
> 	tree_code_for_canonical_type_merging..
> 	* gcc.dg/lto/c-compatible-types_0.c: New testcase.
> 	* gcc.dg/lto/c-compatible-types_1.c: New testcase.
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c	(revision 223877)
> +++ lto/lto.c	(working copy)
> @@ -319,7 +319,7 @@
>       smaller sets; when searching for existing matching types to merge,
>       only existing types having the same features as the new type will be
>       checked.  */
> -  hstate.add_int (TREE_CODE (type));
> +  hstate.add_int (tree_code_for_canonical_type_merging (TREE_CODE (type)));
>    hstate.add_int (TYPE_MODE (type));
>  
>    /* Incorporate common features of numerical types.  */
> Index: testsuite/gcc.dg/lto/c-compatible-types_0.c
> ===================================================================
> --- testsuite/gcc.dg/lto/c-compatible-types_0.c	(revision 0)
> +++ testsuite/gcc.dg/lto/c-compatible-types_0.c	(working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do run } */
> +/* { dg-options "-O3" } */
> +/* By C standard Each enumerated type shall be compatible with char, a  signed
> +   integer, type, or an unsigned integer type. The choice of type is
> +   implementation-defined.  Check that enum and unsigned int match.  */
> +unsigned int a;
> +unsigned int *b;
> +void t();
> +
> +void reset ()
> +{
> +  asm("":"=r"(a):"0"(0));
> +}
> +int
> +main()
> +{
> +  asm("":"=r"(a):"0"(1));
> +  asm("":"=r"(b):"0"(&a));
> +  t();
> +  return 0;
> +}
> Index: testsuite/gcc.dg/lto/c-compatible-types_1.c
> ===================================================================
> --- testsuite/gcc.dg/lto/c-compatible-types_1.c	(revision 0)
> +++ testsuite/gcc.dg/lto/c-compatible-types_1.c	(working copy)
> @@ -0,0 +1,19 @@
> +enum a {test1, test2};
> +enum a a;
> +enum a *b;
> +
> +void reset (void);
> +
> +void
> +t()
> +{
> +  if (a != test2)
> +    __builtin_abort ();
> +  if (*b != test2)
> +    __builtin_abort ();
> +  reset ();
> +  if (a != test1)
> +    __builtin_abort ();
> +  if (*b != test1)
> +    __builtin_abort ();
> +}
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 223877)
> +++ tree.c	(working copy)
> @@ -12877,7 +12877,8 @@
>      return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
>  
>    /* Can't be the same type if the types don't have the same code.  */
> -  if (TREE_CODE (t1) != TREE_CODE (t2))
> +  if (tree_code_for_canonical_type_merging (TREE_CODE (t1))
> +      != tree_code_for_canonical_type_merging (TREE_CODE (t2)))
>      return false;
>  
>    /* Qualifiers do not matter for canonical type comparison purposes.  */
> Index: tree.h
> ===================================================================
> --- tree.h	(revision 223877)
> +++ tree.h	(working copy)
> @@ -4598,7 +4598,28 @@
>  extern void DEBUG_FUNCTION verify_type (const_tree t);
>  extern bool gimple_canonical_types_compatible_p (const_tree, const_tree,
>  						 bool trust_type_canonical = true);
> +/* Return simplified tree code of type that is used for canonical type merging.  */
> +inline enum tree_code
> +tree_code_for_canonical_type_merging (enum tree_code code)
> +{
> +  /* By C standard, each enumerated type shall be compatible with char,
> +     a signed integer, or an unsigned integer.  The choice of type is
> +     implementation defined (in our case it depends on -fshort-enum).
>  
> +     For this reason we make no distinction between ENUMERAL_TYPE and INTEGER
> +     type and compare only by their signedness and precision.  */
> +  if (code == ENUMERAL_TYPE)
> +    return INTEGER_TYPE;
> +  /* To allow inter-operability between languages having references and
> +     C, we consider reference types and pointers alike.  Note that this is
> +     not strictly necessary for C-Fortran 2008 interoperability because
> +     Fortran define C_PTR type that needs to be compatible with C pointers
> +     and we handle this one as ptr_type_node.  */
> +  if (code == REFERENCE_TYPE)
> +    return POINTER_TYPE;
> +  return code;
> +}
> +
>  #define tree_map_eq tree_map_base_eq
>  extern unsigned int tree_map_hash (const void *);
>  #define tree_map_marked_p tree_map_base_marked_p
> 
>

> On Sat, 30 May 2015, Jan Hubicka wrote:
> 
> > Joseph, Richard,
> > this is patch implementing the ENUM/INGEGER globbing and also POINTER/REFERENCE
> > (though I don't know if that one follows by some standard rules).
> > Joseph, does the attached testcase make sense for you? Is it defined? It is my
> > first attempt to really interpret C standard to detail.
> > 
> > Ideally I would like to have testcases for all the globbing we do and reasoning
> > why it is needed.
> > 
> > Bootstraped/regtested ppc64le-linux. OK?
> 
> Works for me.  (what about BOOLEAN_TYPE?)

No idea. So far I did not find anything in the language standards that would strictly
require to merge these two though I see it would make sense when mixing K&R and Ansi-C
units...

I am going to push out patch that complains about decl merging where memory locations are
TBAA incompatible. We will get warnings on these then and we shall see how much it hit
us.

Honza