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