public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Jason Merrill <jason@redhat.com>
Cc: Jan Hubicka <hubicka@ucw.cz>, Jakub Jelinek <jakub@redhat.com>,
	Ilya Verbin <iverbin@gmail.com>,
	gcc-patches@gcc.gnu.org,	rguenther@suse.de
Subject: Re: Silence merge warnings on artificial types
Date: Tue, 31 Mar 2015 07:51:00 -0000	[thread overview]
Message-ID: <20150331075121.GB62830@kam.mff.cuni.cz> (raw)
In-Reply-To: <55198D94.5020707@redhat.com>

> On 03/30/2015 01:23 PM, Jan Hubicka wrote:
> >Jason probably knows better, but I think only real C++ types comply the One Defintion
> >Type and should be merged. Anything we create artifically in compiler is probably
> >not covered by this.
> 
> Agreed, compiler internals are outside the scope of the language.  :)

:)

Hi,
this patch adds the ARTIFICIAL flag check to avoid ODR merging to these.
I oriignally tested DECL_ARTIFICIAL (decl) (that is TYPE_NAME) that randomly
dropped type names on some classes but not all.

Jason, please do you know what is meaning of DECL_ARTIFICIAL on class type
names? Perhaps we can drop them to 0 in free lang data?

With this bug I triggered wrong devirtualization because we no longer insert
non-odr types into a type inheritance graph.  This is fixed by the lto_read_decls
change and finally I triggered an ICE in ipa-devirt that due to the bug
output a warning late and ICEd on streamer cache being NULL.  I guess it is
better to guard it even though all wanrings should be output early.

Bootsrapped/regtested x86_64-linux, will commit it after chromium rebuild.

Honza

	* tree.c (need_assembler_name_p): Artificial types have no ODR
	names.
	* ipa-devirt.c (warn_odr): Do not try to apply ODR cache when
	no caching is done.

	* lto.c (lto_read_decls): Move code registering odr types out
	of TYPE_CANONICAL conditional and also register polymorphic types.
Index: tree.c
===================================================================
--- tree.c	(revision 221777)
@@ -5139,6 +5145,7 @@ need_assembler_name_p (tree decl)
       && decl == TYPE_NAME (TREE_TYPE (decl))
       && !is_lang_specific (TREE_TYPE (decl))
       && AGGREGATE_TYPE_P (TREE_TYPE (decl))
+      && !TYPE_ARTIFICIAL (TREE_TYPE (decl))
       && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE)
       && !type_in_anonymous_namespace_p (TREE_TYPE (decl)))
     return !DECL_ASSEMBLER_NAME_SET_P (decl);
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 221777)
+++ lto/lto.c	(working copy)
@@ -1944,13 +1944,24 @@ lto_read_decls (struct lto_file_decl_dat
 		  lto_fixup_prevailing_type (t);
 		}
 	      /* Compute the canonical type of all types.
-		 ???  Should be able to assert that !TYPE_CANONICAL.  */
+
+		 Inside a strongly connected component
+		 gimple_register_canonical_type may recurse and insert
+		 main variant ahead of time.  Thus the need to check
+		 TYPE_CANONICAL. */
 	      if (TYPE_P (t) && !TYPE_CANONICAL (t))
-		{
-		  gimple_register_canonical_type (t);
-		  if (odr_type_p (t))
-		    register_odr_type (t);
-		}
+		gimple_register_canonical_type (t);
+
+	      /* Reigster types to ODR hash.  If we compile unit w/o
+		 -fno-lto-odr-type-merging, also insert types with virtual
+		 tables to keep type inheritance graph complete on
+		 polymorphic types.  */
+	      if (TYPE_P (t)
+		  && (odr_type_p (t)
+		      || (TYPE_MAIN_VARIANT (t) == t
+			  && TREE_CODE (t) == RECORD_TYPE
+			  && TYPE_BINFO (t) && BINFO_VTABLE (TYPE_BINFO (t)))))
+		register_odr_type (t);
 	      /* Link shared INTEGER_CSTs into TYPE_CACHED_VALUEs of its
 		 type which is also member of this SCC.  */
 	      if (TREE_CODE (t) == INTEGER_CST
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 221777)
+++ ipa-devirt.c	(working copy)
@@ -939,7 +939,8 @@ warn_odr (tree t1, tree t2, tree st1, tr
 
   /* ODR warnings are output druing LTO streaming; we must apply location
      cache for potential warnings to be output correctly.  */
-  lto_location_cache::current_cache->apply_location_cache ();
+  if (lto_location_cache::current_cache)
+    lto_location_cache::current_cache->apply_location_cache ();
 
   if (!warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (t1)), OPT_Wodr,
 		   "type %qT violates one definition rule",

  reply	other threads:[~2015-03-31  7:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-30  3:03 Silence merge warnings on artiical types Jan Hubicka
2015-03-30  8:30 ` Richard Biener
2015-03-30  8:48   ` Jan Hubicka
2015-03-30 15:36 ` Ilya Verbin
2015-03-30 17:06   ` Jan Hubicka
2015-03-30 17:11     ` Ilya Verbin
2015-03-30 17:16       ` Jan Hubicka
2015-03-30 17:21     ` Jakub Jelinek
2015-03-30 17:23       ` Jan Hubicka
2015-03-30 17:53         ` Jason Merrill
2015-03-31  7:51           ` Jan Hubicka [this message]
2015-03-31 13:26             ` Silence merge warnings on artificial types Jason Merrill
2015-04-02 18:23             ` Ilya Verbin
2015-04-02 18:37               ` Jan Hubicka
2015-04-02 19:06               ` Jakub Jelinek
2015-04-03 13:36                 ` Jakub Jelinek

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=20150331075121.GB62830@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=iverbin@gmail.com \
    --cc=jakub@redhat.com \
    --cc=jason@redhat.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).