From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Biener <rguenther@suse.de>
Cc: Jan Hubicka <hubicka@ucw.cz>,
gcc-patches@gcc.gnu.org, burnus@net-b.de,
"Joseph S. Myers" <joseph@codesourcery.com>
Subject: Re: Fix more of C/fortran canonical type issues
Date: Mon, 08 Jun 2015 23:01:00 -0000 [thread overview]
Message-ID: <20150608223334.GD2226@kam.mff.cuni.cz> (raw)
In-Reply-To: <alpine.LSU.2.11.1506081536560.30088@zhemvz.fhfr.qr>
> 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.
next prev parent reply other threads:[~2015-06-08 22:33 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-08 1:23 Jan Hubicka
2015-06-08 5:45 ` Jan Hubicka
2015-06-08 7:25 ` Jan Hubicka
2015-06-08 13:43 ` Richard Biener
2015-06-08 14:07 ` Joseph Myers
2015-06-08 14:32 ` Richard Biener
2015-06-08 14:44 ` Joseph Myers
2015-06-08 14:52 ` Jan Hubicka
2015-06-08 14:54 ` Richard Biener
2015-06-08 15:11 ` Jan Hubicka
2015-06-08 15:32 ` Fortran's C_CHAR type Jan Hubicka
2015-06-10 11:50 ` Mikael Morin
2015-06-10 14:55 ` Jan Hubicka
2015-06-10 16:37 ` Mikael Morin
2015-06-11 18:19 ` Jan Hubicka
2015-06-09 9:50 ` Fix more of C/fortran canonical type issues Richard Biener
2015-06-09 17:24 ` Jan Hubicka
2015-06-11 17:58 ` Jan Hubicka
2015-06-22 7:25 ` Jan Hubicka
2015-06-22 15:09 ` Richard Biener
2015-06-22 16:17 ` Jan Hubicka
2015-06-08 15:08 ` Jan Hubicka
2015-06-08 16:54 ` Joseph Myers
2015-06-08 16:57 ` Jan Hubicka
2015-06-08 17:03 ` Joseph Myers
2015-06-08 22:06 ` Jan Hubicka
2015-06-08 23:01 ` Jan Hubicka [this message]
2015-10-08 3:47 ` Jan Hubicka
2015-10-08 7:44 ` Richard Biener
2015-10-08 16:17 ` Jan Hubicka
2015-10-10 19:45 ` Jan Hubicka
2015-06-08 13:37 ` Richard Biener
2015-10-11 8:09 Dominique d'Humières
2015-10-12 7:10 ` Jan Hubicka
2015-10-12 7:41 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150608223334.GD2226@kam.mff.cuni.cz \
--to=hubicka@ucw.cz \
--cc=burnus@net-b.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=joseph@codesourcery.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).