Re: Fix more of C/fortran canonical type issues

[Jan Hubicka <hubicka at ucw dot cz>]

> On Mon, 8 Jun 2015, Jan Hubicka wrote:
> 
> > Hi,
> > this is a variant of patch that globs also the rest of integer types.
> > Note that we will still get false warnings out of lto-symtab when the
> > values are not wrapped up in structures.  This is because lto-symtab
> > uses types_compatible_p that in turn uses useless_type_conversion and
> > that one needs to honor signedness.
> > 
> > I suppose we need a way to test representation compatibility and TBAA
> > compatiblity. I will give it a more tought how to reorganize the code.
> > Basically we need
> 
> representation compatibility is TYPE_CANONICAL equivalence, TBAA
> compatibility is get_alias_set equivalence.

Hmm, I still wonder what to use in lto-symtab's warn_type_compatibility_p.

Currently we use types_compatible_p which goes to useless conversion and
honnors TYPE_UNSIGNED, so it will give false positives for Fortran.
Comparing TYPE_CANONICAL for equivalence will be conservatively correct
for now (I will submit patch for that and prepare a testcases), but as
soon as we start computing finer TYPE_CANONICAL for pointers
we really want to avoid warning on C_PTR declaration in Fortran and
say float * in C.  This will have different canonical types (C_PTR is void *)
that are TBAA compatible.

Comparing get_alias_set will block warnings about representation incompatibility
in some cases, like when one of units is compiled with -fno-strict-aliasing.
Even then IMO we ought to warn when fortran declares variable as "C_DOUBLE"
and C declares it as "int *".

So I think we want to factor out the representation compatibility logic better
and make it separate from canonical type machinery.

> 
> So you have to be careful when mangling TYPE_CANONICAL according to
> get_alias_set and make sure to only apply this (signedness for example)
> for aggregate type components.
> 
> Can you please split out the string-flag change?  It is approved.

This is what I commited.

After the discussion I still think the second variant of patch (completely
dropping signed/unsigned) makes sense for C+fortran units though it is
unnecesarily coarse for C/C++ only units.  Given that the current plan is
to get things conservatively correct first, I would stick to it.

Bootstrapped/regtested ppc64le-linux, comitted.

Honza

	* lto.c (hash_canonical_type): Drop hashing of TYPE_STRING_FLAG.
	* tree.c (gimple_canonical_types_compatible_p): Drop comparsion of
	TYPE_STRING_FLAG.

	* gfortran.dg/lto/bind_c-2b_0.f90: New testcase
	* gfortran.dg/lto/bind_c-2b_1.c: New testcase
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 224250)
+++ lto/lto.c	(working copy)
@@ -332,18 +332,16 @@
   if (TREE_CODE (type) == COMPLEX_TYPE)
     hstate.add_int (TYPE_UNSIGNED (type));
 
+  /* Fortran's C_SIGNED_CHAR is !TYPE_STRING_FLAG but needs to be
+     interoperable with "signed char".  Unless all frontends are revisited to
+     agree on these types, we must ignore the flag completely.  */
+
   /* 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 (TYPE_ADDR_SPACE (TREE_TYPE (type)));
 
-  /* For integer types hash only the string flag.  */
-  if (TREE_CODE (type) == INTEGER_TYPE)
-    hstate.add_int (TYPE_STRING_FLAG (type));
-
   /* For array types hash the domain bounds and the string flag.  */
   if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
     {
Index: testsuite/gfortran.dg/lto/bind_c-2b_0.f90
===================================================================
--- testsuite/gfortran.dg/lto/bind_c-2b_0.f90	(revision 0)
+++ testsuite/gfortran.dg/lto/bind_c-2b_0.f90	(working copy)
@@ -0,0 +1,21 @@
+! { dg-lto-do run }
+! { dg-lto-options {{ -O3 -flto }} }
+! This testcase will abort if C_SIGNED_CHAR is not interoperable with signed
+! char
+module lto_type_merge_test
+  use, intrinsic :: iso_c_binding
+  implicit none
+
+  type, bind(c) :: MYFTYPE_1
+     integer(c_signed_char) :: chr
+     integer(c_signed_char) :: chrb
+  end type MYFTYPE_1
+
+  type(myftype_1), bind(c, name="myVar") :: myVar
+
+contains
+  subroutine types_test() bind(c)
+    myVar%chr = myVar%chrb
+  end subroutine types_test
+end module lto_type_merge_test
+
Index: testsuite/gfortran.dg/lto/bind_c-2b_1.c
===================================================================
--- testsuite/gfortran.dg/lto/bind_c-2b_1.c	(revision 0)
+++ testsuite/gfortran.dg/lto/bind_c-2b_1.c	(working copy)
@@ -0,0 +1,36 @@
+#include <stdlib.h>
+/* interopse with myftype_1 */
+typedef struct {
+   signed char chr;
+   signed char chr2;
+} 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 *cchr;
+   asm("":"=r"(cchr):"0"(&myVar));
+   cchr->chr = 1;
+   cchr->chr2 = 2;
+
+   types_test();
+
+   if(cchr->chr != 2)
+      abort();
+   if(cchr->chr2 != 2)
+      abort();
+   myVar.chr2 = 3;
+   types_test();
+
+   if(myVar.chr != 3)
+      abort();
+   if(myVar.chr2 != 3)
+      abort();
+   return 0;
+}
+
Index: tree.c
===================================================================
--- tree.c	(revision 224250)
+++ tree.c	(working copy)
@@ -12948,9 +12948,9 @@
 	  || TYPE_UNSIGNED (t1) != TYPE_UNSIGNED (t2))
 	return false;
 
-      if (TREE_CODE (t1) == INTEGER_TYPE
-	  && TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2))
-	return false;
+      /* Fortran's C_SIGNED_CHAR is !TYPE_STRING_FLAG but needs to be
+	 interoperable with "signed char".  Unless all frontends are revisited
+	 to agree on these types, we must ignore the flag completely.  */
 
       /* Fortran standard define C_PTR type that is compatible with every
  	 C pointer.  For this reason we need to glob all pointers into one.