public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Transparent alias suport part 7 (lto-symtab support)
@ 2015-12-08 22:30 Jan Hubicka
  2015-12-12 19:00 ` H.J. Lu
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Hubicka @ 2015-12-08 22:30 UTC (permalink / raw)
  To: gcc-patches

Hi,
this patch adds support to lto-symtab.c to prevent merging of certain
declarations.  The new predicate lto_symtab_merge_p can be used to decide
about this.   The change is pretty straigforward.  lto_symtab_merge_decls_2
makes the decisions about what decls to merge and lto_symtab_merge_symbols_1
does the corresponding symbol merging/creation of transparent aliases.

I re-implemented lto_symtab_prevailing_decl and moved it to inline so the
merging pass is also considerably faster (it used to do quite expensive
assembler name hash lookup on every occurenceof the decl).  Merging decisions
are represented by setting DECL_ABSTRACT_ORIGIN to error_mark_node and
DECL_CHAIN to the replacement decl.  This is quite arbitrary decision - I needed
a pointer and flag that is not going to mess with the warnings we output
during the lto-symtab's operation.

The patch does not disable any of the wrong merging we are hitting.  I will
dot hat separately and with a testcases.  I only revisited a bit the way
builtins are handled.

Bootstrapped/regtested x86_64-linux. I am re-running the lto-bootstrap after
some last minute changes and plan commit once it converges.

Honza

	PR ipa/61886
	* lto-streamer.h (lto_symtab_merge_decls, lto_symtab_merge_symbols,
	lto_symtab_prevailing_decl): MOve to lto-symtab.h.
										
	* lto-symtab.c: Include lto-symtab.h.
	(lto_cgraph_replace_node): Do not merge profiles here.
	(lto_symtab_merge_p): New function.
	(lto_symtab_merge_decls_2): Honor lto_symtab_merge_p.
	(lto_symtab_merge_symbols_1): Turn unmerged decls into transparent
	aliases.
	(lto_symtab_merge_symbols): Do not clear node->aux; we no longer use it.
	(lto_symtab_prevailing_decl): Move to lto-symtab.h; rewrite.
	* lto.c: Include lto-symtab.h
	* lto-symtab.h: New.

Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c	(revision 231424)
+++ lto/lto-symtab.c	(working copy)
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.
 #include "ipa-utils.h"
 #include "builtins.h"
 #include "alias.h"
+#include "lto-symtab.h"
 
 /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
    all edges and removing the old node.  */
@@ -99,7 +100,6 @@ lto_cgraph_replace_node (struct cgraph_n
       node->instrumented_version = NULL;
     }
 
-  ipa_merge_profiles (prevailing_node, node);
   lto_free_function_in_decl_state_for_node (node);
 
   if (node->decl != prevailing_node->decl)
@@ -503,6 +503,30 @@ lto_symtab_resolve_symbols (symtab_node
   return prevailing;
 }
 
+/* Decide if it is OK to merge DECL into PREVAILING.
+   Because we wrap most of uses of declarations in MEM_REF, we can tolerate
+   some differences but other code may inspect directly the DECL.  */
+
+static bool
+lto_symtab_merge_p (tree prevailing, tree decl)
+{
+  if (TREE_CODE (prevailing) != TREE_CODE (decl))
+    return false;
+  if (TREE_CODE (prevailing) == FUNCTION_DECL)
+    {
+      if (DECL_BUILT_IN (prevailing) != DECL_BUILT_IN (decl))
+	return false;
+      if (DECL_BUILT_IN (prevailing)
+	  && (DECL_BUILT_IN_CLASS (prevailing) != DECL_BUILT_IN_CLASS (decl)
+	      || DECL_FUNCTION_CODE (prevailing) != DECL_FUNCTION_CODE (decl)))
+	return false;
+    }
+  /* There are several other cases where merging can not be done, but until
+     aliasing code is fixed to support aliases it we can not really return
+     false on non-readonly var, yet.  */
+  return true;
+}
+
 /* Merge all decls in the symbol table chain to the prevailing decl and
    issue diagnostics about type mismatches.  If DIAGNOSED_P is true
    do not issue further diagnostics.*/
@@ -590,6 +614,54 @@ lto_symtab_merge_decls_2 (symtab_node *f
 	    "-fno-strict-aliasing is used");
 
   mismatches.release ();
+  symtab_node *last_prevailing = prevailing, *next;
+  for (e = prevailing->next_sharing_asm_name; e; e = next)
+    {
+      next = e->next_sharing_asm_name;
+
+      /* Skip non-LTO symbols and symbols whose declaration we already
+	 visited.  */
+      if (lto_symtab_prevailing_decl (e->decl) != e->decl
+	  || !lto_symtab_symbol_p (e)
+          || e->decl == prevailing->decl)
+	continue;
+
+      symtab_node *this_prevailing;
+      for (this_prevailing = prevailing; ;
+	   this_prevailing = this_prevailing->next_sharing_asm_name)
+	{
+	  if (lto_symtab_merge_p (this_prevailing->decl, e->decl))
+	    break;
+	  if (this_prevailing == last_prevailing)
+	    {
+	      this_prevailing = NULL;
+	      break;
+	    }
+	}
+
+      if (this_prevailing)
+	lto_symtab_prevail_decl (this_prevailing->decl, e->decl);
+      /* Maintain LRU list: relink the new prevaililng symbol
+	 just after previaling node in the chain and update last_prevailing.
+	 Since the number of possible declarations of a given symbol is
+	 small, this should be faster than building a hash.  */
+      else if (e == prevailing->next_sharing_asm_name)
+	last_prevailing = e;
+      else
+	{
+	  if (e->next_sharing_asm_name)
+	    e->next_sharing_asm_name->previous_sharing_asm_name
+	      = e->previous_sharing_asm_name;
+	  e->previous_sharing_asm_name->next_sharing_asm_name
+	    = e->next_sharing_asm_name;
+	  e->previous_sharing_asm_name = prevailing;
+	  e->next_sharing_asm_name = prevailing->next_sharing_asm_name;
+	  prevailing->next_sharing_asm_name->previous_sharing_asm_name = e;
+	  prevailing->next_sharing_asm_name = e;
+	  if (last_prevailing == prevailing)
+	    last_prevailing = e;
+	}
+    }
 }
 
 /* Helper to process the decl chain for the symbol table entry *SLOT.  */
@@ -729,6 +801,8 @@ lto_symtab_merge_symbols_1 (symtab_node
   symtab_node *e;
   symtab_node *next;
 
+  prevailing->decl->decl_with_vis.symtab_node = prevailing;
+
   /* Replace the cgraph node of each entry with the prevailing one.  */
   for (e = prevailing->next_sharing_asm_name; e;
        e = next)
@@ -738,10 +812,47 @@ lto_symtab_merge_symbols_1 (symtab_node
       if (!lto_symtab_symbol_p (e))
 	continue;
       cgraph_node *ce = dyn_cast <cgraph_node *> (e);
-      if (ce && !DECL_BUILT_IN (e->decl))
-	lto_cgraph_replace_node (ce, dyn_cast<cgraph_node *> (prevailing));
-      if (varpool_node *ve = dyn_cast <varpool_node *> (e))
-	lto_varpool_replace_node (ve, dyn_cast<varpool_node *> (prevailing));
+      symtab_node *to = symtab_node::get (lto_symtab_prevailing_decl (e->decl));
+
+      /* No matter how we are going to deal with resolution, we will ultimately
+	 use prevailing definition.  */
+      if (ce)
+          ipa_merge_profiles (dyn_cast<cgraph_node *> (prevailing),
+			      dyn_cast<cgraph_node *> (e));
+
+      /* If we decided to replace the node by TO, do it.  */
+      if (e != to)
+	{
+	  if (ce)
+	    lto_cgraph_replace_node (ce, dyn_cast<cgraph_node *> (to));
+	  else if (varpool_node *ve = dyn_cast <varpool_node *> (e))
+	    lto_varpool_replace_node (ve, dyn_cast<varpool_node *> (to));
+	}
+      /* Watch out for duplicated symbols for a given declaration.  */
+      else if (!e->transparent_alias
+	       || !e->definition || e->get_alias_target () != to)
+	{
+	  /* We got a new declaration we do not want to merge.  In this case
+	     get rid of the existing definition and create a transparent
+	     alias.  */
+	  if (ce)
+	    {
+	      lto_free_function_in_decl_state_for_node (ce);
+	      if (!ce->weakref)
+	        ce->release_body ();
+	      ce->reset ();
+	      symtab->call_cgraph_removal_hooks (ce);
+	    }
+	  else
+	    {
+	      DECL_INITIAL (e->decl) = error_mark_node;
+	      symtab->call_varpool_removal_hooks (dyn_cast<varpool_node *> (e));
+	    }
+	  e->remove_all_references ();
+	  e->analyzed = e->body_removed = false;
+	  e->resolve_alias (prevailing, true);
+	  gcc_assert (e != prevailing);
+	}
     }
 
   return;
@@ -782,9 +893,8 @@ lto_symtab_merge_symbols (void)
 	      symtab_node *tgt = symtab_node::get_for_asmname (node->alias_target);
 	      gcc_assert (node->weakref);
 	      if (tgt)
-		node->resolve_alias (tgt);
+		node->resolve_alias (tgt, true);
 	    }
-	  node->aux = NULL;
 
 	  if (!(cnode = dyn_cast <cgraph_node *> (node))
 	      || !cnode->clone_of
@@ -821,45 +931,3 @@ lto_symtab_merge_symbols (void)
 	}
     }
 }
-
-/* Given the decl DECL, return the prevailing decl with the same name. */
-
-tree
-lto_symtab_prevailing_decl (tree decl)
-{
-  symtab_node *ret;
-
-  /* Builtins and local symbols are their own prevailing decl.  */
-  if ((!TREE_PUBLIC (decl) && !DECL_EXTERNAL (decl)) || is_builtin_fn (decl))
-    return decl;
-
-  /* DECL_ABSTRACT_Ps are their own prevailing decl.  */
-  if (TREE_CODE (decl) == FUNCTION_DECL && DECL_ABSTRACT_P (decl))
-    return decl;
-
-  /* When decl did not participate in symbol resolution leave it alone.
-     This can happen when we streamed the decl as abstract origin
-     from the block tree of inlining a partially inlined function.
-     If all, the split function and the original function end up
-     optimized away early we do not put the abstract origin into the
-     ltrans boundary and we'll end up ICEing in
-     dwarf2out.c:gen_inlined_subroutine_die because we eventually
-     replace a decl with DECL_POSSIBLY_INLINED set with one without.  */
-  if (TREE_CODE (decl) == FUNCTION_DECL
-      && ! cgraph_node::get (decl))
-    return decl;
-
-  /* Ensure DECL_ASSEMBLER_NAME will not set assembler name.  */
-  gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
-
-  /* Walk through the list of candidates and return the one we merged to.  */
-  ret = symtab_node::get_for_asmname (DECL_ASSEMBLER_NAME (decl));
-  if (!ret)
-    return decl;
-
-  /* Do not replace a non-builtin with a builtin.  */
-  if (is_builtin_fn (ret->decl))
-    return decl;
-
-  return ret->decl;
-}
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 231424)
+++ lto/lto.c	(working copy)
@@ -49,6 +49,7 @@ along with GCC; see the file COPYING3.
 #include "params.h"
 #include "ipa-utils.h"
 #include "gomp-constants.h"
+#include "lto-symtab.h"
 
 
 /* Number of parallel tasks to run, -1 if we want to use GNU Make jobserver.  */
Index: lto/lto-symtab.h
===================================================================
--- lto/lto-symtab.h	(revision 0)
+++ lto/lto-symtab.h	(revision 0)
@@ -0,0 +1,47 @@
+/* LTO symbol table merging.
+   Copyright (C) 2009-2015 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+extern void lto_symtab_merge_decls (void);
+extern void lto_symtab_merge_symbols (void);
+extern tree lto_symtab_prevailing_decl (tree decl);
+
+/* Mark DECL to be previailed by PREVAILING.
+   Use DECL_ABSTRACT_ORIGIN and DECL_CHAIN as special markers; those do not
+   disturb debug_tree and diagnostics.
+   We are safe to modify them as we wish, becuase the declarations disappear
+   from the IL after the merging.  */
+
+inline void
+lto_symtab_prevail_decl (tree prevailing, tree decl)
+{
+  gcc_checking_assert (DECL_ABSTRACT_ORIGIN (decl) != error_mark_node);
+  DECL_CHAIN (decl) = prevailing;
+  DECL_ABSTRACT_ORIGIN (decl) = error_mark_node;
+}
+
+/* Given the decl DECL, return the prevailing decl with the same name. */
+
+inline tree
+lto_symtab_prevailing_decl (tree decl)
+{
+  if (DECL_ABSTRACT_ORIGIN (decl) == error_mark_node)
+    return DECL_CHAIN (decl);
+  else
+    return decl;
+}
Index: lto-streamer.h
===================================================================
--- lto-streamer.h	(revision 231424)
+++ lto-streamer.h	(working copy)
@@ -927,11 +927,6 @@ void cl_optimization_stream_out (struct
 void cl_optimization_stream_in (struct bitpack_d *, struct cl_optimization *);
 
 
-/* In lto-symtab.c.  */
-extern void lto_symtab_merge_decls (void);
-extern void lto_symtab_merge_symbols (void);
-extern tree lto_symtab_prevailing_decl (tree decl);
-
 
 /* In lto-opts.c.  */
 extern void lto_write_options (void);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Transparent alias suport part 7 (lto-symtab support)
  2015-12-08 22:30 Transparent alias suport part 7 (lto-symtab support) Jan Hubicka
@ 2015-12-12 19:00 ` H.J. Lu
  0 siblings, 0 replies; 2+ messages in thread
From: H.J. Lu @ 2015-12-12 19:00 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: GCC Patches

On Tue, Dec 8, 2015 at 2:30 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch adds support to lto-symtab.c to prevent merging of certain
> declarations.  The new predicate lto_symtab_merge_p can be used to decide
> about this.   The change is pretty straigforward.  lto_symtab_merge_decls_2
> makes the decisions about what decls to merge and lto_symtab_merge_symbols_1
> does the corresponding symbol merging/creation of transparent aliases.
>
> I re-implemented lto_symtab_prevailing_decl and moved it to inline so the
> merging pass is also considerably faster (it used to do quite expensive
> assembler name hash lookup on every occurenceof the decl).  Merging decisions
> are represented by setting DECL_ABSTRACT_ORIGIN to error_mark_node and
> DECL_CHAIN to the replacement decl.  This is quite arbitrary decision - I needed
> a pointer and flag that is not going to mess with the warnings we output
> during the lto-symtab's operation.
>
> The patch does not disable any of the wrong merging we are hitting.  I will
> dot hat separately and with a testcases.  I only revisited a bit the way
> builtins are handled.
>
> Bootstrapped/regtested x86_64-linux. I am re-running the lto-bootstrap after
> some last minute changes and plan commit once it converges.
>
> Honza
>
>         PR ipa/61886
>         * lto-streamer.h (lto_symtab_merge_decls, lto_symtab_merge_symbols,
>         lto_symtab_prevailing_decl): MOve to lto-symtab.h.
>
>         * lto-symtab.c: Include lto-symtab.h.
>         (lto_cgraph_replace_node): Do not merge profiles here.
>         (lto_symtab_merge_p): New function.
>         (lto_symtab_merge_decls_2): Honor lto_symtab_merge_p.
>         (lto_symtab_merge_symbols_1): Turn unmerged decls into transparent
>         aliases.
>         (lto_symtab_merge_symbols): Do not clear node->aux; we no longer use it.
>         (lto_symtab_prevailing_decl): Move to lto-symtab.h; rewrite.
>         * lto.c: Include lto-symtab.h
>         * lto-symtab.h: New.
>

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68878


-- 
H.J.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-12-12 19:00 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08 22:30 Transparent alias suport part 7 (lto-symtab support) Jan Hubicka
2015-12-12 19:00 ` H.J. Lu

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