Re: Get LTO correct for Fortran C_PTR type

[Jan Hubicka <hubicka at ucw dot cz>]

Hi,
this is variant of patch I ended up comitting.

Tobias: I would still welcome a final word about validity of my fortran
testcase.

The original version run into checking failures with nested functions
tranpolines because we now all pointers have the same TYPE_CANONICAL that makes
useless_type_conversion_p to return true when converting function pointer to
non-function.  This eventually triggers verify_stmt ICE because we insist on
gimple_call to have function pointer as parameter.

To fix this, I simply reordered the existing checks so we do not return early.
I will be able to revert this once the followup patch to improve canonical
types on pointers gets in.

In general however useless_type_conversion_p IMO should not be tied to
canonical type machinery.  useless_type_conversion_p is about two types being
compatible in a sense that all operations on inner type have same semantics as
operations on outer type.  TBAA notion of canonical types for LTO is weaker
than that due to the need to produce equivalence classes.  I suppose the
TYPE_CANONICAL check is an useful compile time optimization for cases where
canonical types have same strength, but we ought to add sanity checks for that.
I will send patch for that.

Finally I noticed that I ended up comitting wrong version of the c-compatible-types
testcase and replaced it by the final one.  I did not found a way to test for
short-enums in LTO test, but I simply added enum value of INT_MAX that makes it
sure the enum will be large enough with -fshort-enum build for testcase to pass.

Bootstrapped/regtested ppc64le-linux after working around the miscompare,
comitted.


Honza

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 224200)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@
+2015-06-06  Jan Hubicka  <hubicka@ucw.cz>
+
+	* alias.c (get_alias_set): Be ready for TYPE_CANONICAL
+	of ptr_type_node to not be ptr_to_node.
+	* tree.c (gimple_types_compatible_p): Do not match TREE_CODE of
+	TREE_TYPE of pointers.
+	* gimple-expr.c (useless_type_conversion): Reorder the check for
+	function pointers and TYPE_CANONICAL.
+
 2015-06-06  John David Anglin  <danglin@gcc.gnu.org>
 
 	PR bootstrap/66319
Index: alias.c
===================================================================
--- alias.c	(revision 224200)
+++ alias.c	(working copy)
@@ -1072,8 +1072,9 @@
     }
   /* In LTO the rules above needs to be part of canonical type machinery.
      For now just punt.  */
-  else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
-    set = get_alias_set (ptr_type_node);
+  else if (POINTER_TYPE_P (t)
+	   && t != TYPE_CANONICAL (ptr_type_node) && in_lto_p)
+    set = get_alias_set (TYPE_CANONICAL (ptr_type_node));
 
   /* Otherwise make a new alias set for this type.  */
   else
Index: gimple-expr.c
===================================================================
--- gimple-expr.c	(revision 224200)
+++ gimple-expr.c	(working copy)
@@ -84,6 +84,12 @@
       if (TYPE_ADDR_SPACE (TREE_TYPE (outer_type))
 	  != TYPE_ADDR_SPACE (TREE_TYPE (inner_type)))
 	return false;
+      /* Do not lose casts to function pointer types.  */
+      if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
+	   || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
+	  && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
+	       || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
+	return false;
     }
 
   /* From now on qualifiers on value types do not matter.  */
@@ -142,13 +148,6 @@
   else if (POINTER_TYPE_P (inner_type)
 	   && POINTER_TYPE_P (outer_type))
     {
-      /* Do not lose casts to function pointer types.  */
-      if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
-	   || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
-	  && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
-	       || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
-	return false;
-
       /* We do not care for const qualification of the pointed-to types
 	 as const qualification has no semantic value to the middle-end.  */
 
Index: lto/ChangeLog
===================================================================
--- lto/ChangeLog	(revision 224200)
+++ lto/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2015-06-06  Jan Hubicka  <hubicka@ucw.cz>
+
+	* lto.c (hash_canonical_type): Do not hash TREE_CODE of TREE_TYPE of
+	pointers.
+
 2015-06-05  Aldy Hernandez  <aldyh@redhat.com>
 
 	* lto-lang.c (lto_write_globals): Remove.
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 224200)
+++ lto/lto.c	(working copy)
@@ -337,12 +337,12 @@
   if (TREE_CODE (type) == COMPLEX_TYPE)
     hstate.add_int (TYPE_UNSIGNED (type));
 
-  /* For pointer and reference types, fold in information about the type
-     pointed to but do not recurse to the pointed-to type.  */
+  /* Fortran standard define C_PTR type that is compatible with every
+     C pointer.  For this reason we need to glob all pointers into one.
+     Still pointers in different address spaces are not compatible.  */
   if (POINTER_TYPE_P (type))
     {
       hstate.add_int (TYPE_ADDR_SPACE (TREE_TYPE (type)));
-      hstate.add_int (TREE_CODE (TREE_TYPE (type)));
     }
 
   /* For integer types hash only the string flag.  */
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 224200)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,13 @@
+2015-06-06  Jan Hubicka  <hubicka@ucw.cz>
+
+	* gfortran.dg/lto/bind_c-1_0.f90: New testcase.
+	* gfortran.dg/lto/bind_c-1_1.c: New testcase.
+	* gcc.dg/lto/c-compatible-types_0.c: Rename to ...
+	* gcc.dg/lto/c-compatible-types-1_0.c: this one; fix template
+	* gcc.dg/lto/c-compatible-types_1.c: Rename to ...
+	* gcc.dg/lto/c-compatible-types-1_1.c: this one; harden for
+	-fshort-enum.
+
 2015-06-06  Thomas Koenig  <tkoenig@netcologne.de>
 
 	PR fortran/47659
Index: testsuite/gcc.dg/lto/c-compatible-types-1_0.c
===================================================================
--- testsuite/gcc.dg/lto/c-compatible-types-1_0.c	(revision 224200)
+++ testsuite/gcc.dg/lto/c-compatible-types-1_0.c	(working copy)
@@ -1,6 +1,5 @@
-/* { dg-do run } */
-/* { dg-options "-O3" } */
-/* { dg-skip-if "require -fno-short-enums to work" {target short_enums} } */
+/* { dg-lto-do run } */
+/* { dg-lto-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
Index: testsuite/gcc.dg/lto/c-compatible-types-1_1.c
===================================================================
--- testsuite/gcc.dg/lto/c-compatible-types-1_1.c	(revision 224199)
+++ testsuite/gcc.dg/lto/c-compatible-types-1_1.c	(working copy)
@@ -1,4 +1,5 @@
-enum a {test1, test2};
+#include <limits.h>
+enum a {test1, test2, test3=INT_MAX};
 enum a a;
 enum a *b;
 
Index: testsuite/gcc.dg/lto/c-compatible-types_0.c
===================================================================
--- testsuite/gcc.dg/lto/c-compatible-types_0.c	(revision 224200)
+++ testsuite/gcc.dg/lto/c-compatible-types_0.c	(working copy)
@@ -1,23 +0,0 @@
-/* { dg-do run } */
-/* { dg-options "-O3" } */
-/* { dg-skip-if "require -fno-short-enums to work" {target short_enums} } */
-
-/* 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 224200)
+++ testsuite/gcc.dg/lto/c-compatible-types_1.c	(working copy)
@@ -1,19 +0,0 @@
-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: testsuite/gfortran.dg/lto/bind_c-1_0.f90
===================================================================
--- testsuite/gfortran.dg/lto/bind_c-1_0.f90	(revision 0)
+++ testsuite/gfortran.dg/lto/bind_c-1_0.f90	(working copy)
@@ -0,0 +1,21 @@
+! { dg-do run }
+! { dg-lto-options {{ -O3 -flto }} }
+! This testcase will abort if C_PTR is not interoperable with both int *
+! and float *
+module lto_type_merge_test
+  use, intrinsic :: iso_c_binding
+  implicit none
+
+  type, bind(c) :: MYFTYPE_1
+     type(c_ptr) :: ptr
+     type(c_ptr) :: ptrb
+  end type MYFTYPE_1
+
+  type(myftype_1), bind(c, name="myVar") :: myVar
+
+contains
+  subroutine types_test() bind(c)
+    myVar%ptr = myVar%ptrb
+  end subroutine types_test
+end module lto_type_merge_test
+
Index: testsuite/gfortran.dg/lto/bind_c-1_1.c
===================================================================
--- testsuite/gfortran.dg/lto/bind_c-1_1.c	(revision 0)
+++ testsuite/gfortran.dg/lto/bind_c-1_1.c	(working copy)
@@ -0,0 +1,36 @@
+#include <stdlib.h>
+/* interopse with myftype_1 */
+typedef struct {
+   float *ptr;
+   int *ptr2;
+} myctype_t;
+
+
+extern void abort(void);
+void types_test(void);
+/* declared in the fortran module */
+extern myctype_t myVar;
+
+int main(int argc, char **argv)
+{
+   myctype_t *cptr;
+   asm("":"=r"(cptr):"0"(&myVar));
+   cptr->ptr = (float *)(size_t) (void *)1;
+   cptr->ptr2 = (int *)(size_t) (void *)2;
+
+   types_test();
+
+   if(cptr->ptr != (float *)(size_t) (void *)2)
+      abort();
+   if(cptr->ptr2 != (int *)(size_t) (void *)2)
+      abort();
+   myVar.ptr2 = (int *)(size_t) (void *)3;
+   types_test();
+
+   if(myVar.ptr != (float *)(size_t) (void *)3)
+      abort();
+   if(myVar.ptr2 != (int *)(size_t) (void *)3)
+      abort();
+   return 0;
+}
+
Index: tree.c
===================================================================
--- tree.c	(revision 224200)
+++ tree.c	(working copy)
@@ -12958,18 +12958,14 @@
 	  && TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2))
 	return false;
 
-      /* For canonical type comparisons we do not want to build SCCs
-	 so we cannot compare pointed-to types.  But we can, for now,
-	 require the same pointed-to type kind and match what
-	 useless_type_conversion_p would do.  */
+      /* Fortran standard define C_PTR type that is compatible with every
+ 	 C pointer.  For this reason we need to glob all pointers into one.
+	 Still pointers in different address spaces are not compatible.  */
       if (POINTER_TYPE_P (t1))
 	{
 	  if (TYPE_ADDR_SPACE (TREE_TYPE (t1))
 	      != TYPE_ADDR_SPACE (TREE_TYPE (t2)))
 	    return false;
-
-	  if (TREE_CODE (TREE_TYPE (t1)) != TREE_CODE (TREE_TYPE (t2)))
-	    return false;
 	}
 
       /* Tail-recurse to components.  */