Re: ipa-icf::merge TLC

[Jan Hubicka <hubicka at ucw dot cz>]

Hi,
this is the final version of patch I comitted.  It has new fix to make_decl_local
to set TREE_ADDRESSABLE becuase we leave the flag undefined for non-local decls.
I also dropped Optimization from fmerge-all-constants, fmerge-constants
those can not be done in function speicfic way, I made ipa_ref::address_matters_p
to use fmerge-constants, added code to drop UNINLINABLE flag when function is turned
into a wrapper, added check to require DECL_NO_INLINE_WARNING_P match
and added code to set TREE_ADDRESSABLE when non-addressable and addressable vars are merged.
I also disabled merging for DECL_CONSTANT_POOL because it does not work (symtab does not
expect aliases here)

Bootstrapped/regtested x86_64-linux, comitted.

Honza
	* ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton):
	Use address_matters_p.
	(redirect_all_callers, set_addressable): New functions.
	(sem_function::merge): Reorganize and fix merging issues.
	(sem_variable::merge): Likewise.
	(sem_variable::compare_sections): Remove.
	* common.opt (fmerge-all-constants, fmerge-constants): Remove
	Optimization flag.
	* symtab.c (symtab_node::resolve_alias): When alias has aliases,
	redirect them.
	(symtab_node::make_decl_local): Set ADDRESSABLE bit when
	decl is used.
	(address_matters_1): New function.
	(symtab_node::address_matters_p): New function.
	* cgraph.c (cgraph_edge::verify_corresponds_to_fndecl): Fix
	check for merged flag.
	* cgraph.h (address_matters_p): Declare.
	(symtab_node::address_taken_from_non_vtable_p): Remove.
	(symtab_node::address_can_be_compared_p): New method.
	(ipa_ref::address_matters_p): Move here from ipa-ref.c; simplify.
	* ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p):
	Remove.
	(comdat_can_be_unshared_p_1) Use address_matters_p.
	(update_vtable_references): Fix formating.
	* ipa-ref.c (ipa_ref::address_matters_p): Move inline.
	* cgraphunit.c (cgraph_node::create_wrapper): Drop UNINLINABLE flag.
	* cgraphclones.c: Preserve merged and icf_merged flags.

	* gcc.dg/pr64454.c: Disable ICF.
	* gcc.dg/pr28685-1.c: Disable ICF
	* gcc.dg/ipa/iinline-5.c: Disable ICF.
	* g++.dg/warn/Wsuggest-final.C: Force methods to be different.
	* g++.dg/ipa/ipa-icf-4.C: Update template.
Index: symtab.c
===================================================================
--- symtab.c	(revision 221034)
+++ symtab.c	(working copy)
@@ -1156,7 +1156,11 @@ symtab_node::make_decl_local (void)
     return;
 
   if (TREE_CODE (decl) == VAR_DECL)
-    DECL_COMMON (decl) = 0;
+    {
+      DECL_COMMON (decl) = 0;
+      /* ADDRESSABLE flag is not defined for public symbols.  */
+      TREE_ADDRESSABLE (decl) = 1;
+    }
   else gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
 
   DECL_COMDAT (decl) = 0;
@@ -1513,6 +1517,19 @@ symtab_node::resolve_alias (symtab_node
   /* If alias has address taken, so does the target.  */
   if (address_taken)
     target->ultimate_alias_target ()->address_taken = true;
+
+  /* All non-weakref aliases of THIS are now in fact aliases of TARGET.  */
+  ipa_ref *ref;
+  for (unsigned i = 0; iterate_direct_aliases (i, ref);)
+    {
+      struct symtab_node *alias_alias = ref->referring;
+      if (!alias_alias->weakref)
+	{
+	  alias_alias->remove_all_references ();
+	  alias_alias->create_reference (target, IPA_REF_ALIAS, NULL);
+	}
+      else i++;
+    }
   return true;
 }
 
@@ -1863,3 +1880,31 @@ symtab_node::call_for_symbol_and_aliases
     }
   return false;
 }
+
+/* Return ture if address of N is possibly compared.  */
+
+static bool
+address_matters_1 (symtab_node *n, void *)
+{
+  struct ipa_ref *ref;
+
+  if (!n->address_can_be_compared_p ())
+    return false;
+  if (n->externally_visible || n->force_output)
+    return true;
+
+  for (unsigned int i = 0; n->iterate_referring (i, ref); i++)
+    if (ref->address_matters_p ())
+      return true;
+  return false;
+}
+
+/* Return true if symbol's address may possibly be compared to other
+   symbol's address.  */
+
+bool
+symtab_node::address_matters_p ()
+{
+  gcc_assert (!alias);
+  return call_for_symbol_and_aliases (address_matters_1, NULL, true);
+}
Index: common.opt
===================================================================
--- common.opt	(revision 221034)
+++ common.opt	(working copy)
@@ -1644,11 +1644,11 @@ Report on permanent memory allocation in
 ; string constants and constants from constant pool, if 2 also constant
 ; variables.
 fmerge-all-constants
-Common Report Var(flag_merge_constants,2) Init(1) Optimization
+Common Report Var(flag_merge_constants,2) Init(1)
 Attempt to merge identical constants and constant variables
 
 fmerge-constants
-Common Report Var(flag_merge_constants,1) Optimization
+Common Report Var(flag_merge_constants,1)
 Attempt to merge identical constants across compilation units
 
 fmerge-debug-strings
Index: testsuite/gcc.dg/pr64454.c
===================================================================
--- testsuite/gcc.dg/pr64454.c	(revision 221034)
+++ testsuite/gcc.dg/pr64454.c	(working copy)
@@ -1,6 +1,6 @@
 /* PR tree-optimization/64454 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-options "-O2 -fdump-tree-vrp1 -fno-ipa-icf" } */
 
 unsigned
 f1 (unsigned x)
Index: testsuite/gcc.dg/pr28685-1.c
===================================================================
--- testsuite/gcc.dg/pr28685-1.c	(revision 221034)
+++ testsuite/gcc.dg/pr28685-1.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" }  */
+/* { dg-options "-O2 -fdump-tree-optimized -fno-ipa-icf" }  */
 
 /* Should produce <=.  */
 int test1 (int a, int b)
Index: testsuite/gcc.dg/ipa/iinline-5.c
===================================================================
--- testsuite/gcc.dg/ipa/iinline-5.c	(revision 221034)
+++ testsuite/gcc.dg/ipa/iinline-5.c	(working copy)
@@ -1,7 +1,7 @@
 /* Verify that simple indirect calls are inlined even without early
    inlining..  */
 /* { dg-do run } */
-/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining"  } */
+/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-icf"  } */
 
 extern void abort (void);
 
Index: testsuite/g++.dg/warn/Wsuggest-final.C
===================================================================
--- testsuite/g++.dg/warn/Wsuggest-final.C	(revision 221034)
+++ testsuite/g++.dg/warn/Wsuggest-final.C	(working copy)
@@ -1,8 +1,9 @@
 // { dg-do compile }
 // { dg-options "-O2 -Wsuggest-final-types -Wsuggest-final-methods" }
+int c;
 struct A { // { dg-warning "final would enable devirtualization of 4 calls" }
 virtual void a() {} // { dg-warning "final would enable devirtualization of 2 calls" }
- virtual void b() {} // { dg-warning "final would enable devirtualization of 2 calls"  }
+ virtual void b() {c++;} // { dg-warning "final would enable devirtualization of 2 calls"  }
 };
 void
 t(struct A *a)
Index: testsuite/g++.dg/ipa/ipa-icf-4.C
===================================================================
--- testsuite/g++.dg/ipa/ipa-icf-4.C	(revision 221034)
+++ testsuite/g++.dg/ipa/ipa-icf-4.C	(working copy)
@@ -43,6 +43,6 @@ int main()
   return 123;
 }
 
-/* { dg-final { scan-ipa-dump "\(Varpool alias has been created\)|\(Symbol aliases are not supported by target\)" "icf"  } } */
+/* { dg-final { scan-ipa-dump "\(Unified; Variable alias has been created\)|\(Symbol aliases are not supported by target\)" "icf"  } } */
 /* { dg-final { scan-ipa-dump "Equal symbols: 6" "icf"  } } */
 /* { dg-final { cleanup-ipa-dump "icf" } } */
Index: ipa-ref.c
===================================================================
--- ipa-ref.c	(revision 221034)
+++ ipa-ref.c	(working copy)
@@ -124,23 +124,3 @@ ipa_ref::referred_ref_list (void)
 {
   return &referred->ref_list;
 }
-
-/* Return true if refernece may be used in address compare.  */
-bool
-ipa_ref::address_matters_p ()
-{
-  if (use != IPA_REF_ADDR)
-    return false;
-  /* Addresses taken from virtual tables are never compared.  */
-  if (is_a <varpool_node *> (referring)
-      && DECL_VIRTUAL_P (referring->decl))
-    return false;
-  /* Address of virtual tables and functions is never compared.  */
-  if (DECL_VIRTUAL_P (referred->decl))
-    return false;
-  /* Address of C++ cdtors is never compared.  */
-  if (is_a <cgraph_node *> (referred)
-      && (DECL_CXX_CONSTRUCTOR_P (referred->decl) || DECL_CXX_DESTRUCTOR_P (referred->decl)))
-    return false;
-  return true;
-}
Index: cgraph.c
===================================================================
--- cgraph.c	(revision 221034)
+++ cgraph.c	(working copy)
@@ -2630,7 +2630,7 @@ cgraph_edge::verify_corresponds_to_fndec
   if (!node
       || node->body_removed
       || node->in_other_partition
-      || node->icf_merged
+      || callee->icf_merged
       || callee->in_other_partition)
     return false;
 
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 221034)
+++ cgraph.h	(working copy)
@@ -326,9 +326,6 @@ public:
   /* Return true if ONE and TWO are part of the same COMDAT group.  */
   inline bool in_same_comdat_group_p (symtab_node *target);
 
-  /* Return true when there is a reference to node and it is not vtable.  */
-  bool address_taken_from_non_vtable_p (void);
-
   /* Return true if symbol is known to be nonzero.  */
   bool nonzero_address ();
 
@@ -337,6 +334,15 @@ public:
      return 2 otherwise.   */
   int equal_address_to (symtab_node *s2);
 
+  /* Return true if symbol's address may possibly be compared to other
+     symbol's address.  */
+  bool address_matters_p ();
+
+  /* Return true if NODE's address can be compared.  This use properties
+     of NODE only and does not look if the address is actually taken in
+     interesting way.  For that use ADDRESS_MATTERS_P instead.  */
+  bool address_can_be_compared_p (void);
+
   /* Return symbol table node associated with DECL, if any,
      and NULL otherwise.  */
   static inline symtab_node *get (const_tree decl)
@@ -3022,6 +3028,43 @@ varpool_node::call_for_symbol_and_aliase
   return false;
 }
 
+/* Return true if NODE's address can be compared.  */
+
+inline bool
+symtab_node::address_can_be_compared_p ()
+{
+  /* Address of virtual tables and functions is never compared.  */
+  if (DECL_VIRTUAL_P (decl))
+    return false;
+  /* Address of C++ cdtors is never compared.  */
+  if (is_a <cgraph_node *> (this)
+      && (DECL_CXX_CONSTRUCTOR_P (decl)
+	  || DECL_CXX_DESTRUCTOR_P (decl)))
+    return false;
+  /* Constant pool symbols addresses are never compared.
+     flag_merge_constants permits us to assume the same on readonly vars.  */
+  if (is_a <varpool_node *> (this)
+      && (DECL_IN_CONSTANT_POOL (decl)
+	  || (flag_merge_constants >= 2
+	      && TREE_READONLY (decl) && !TREE_THIS_VOLATILE (decl))))
+    return false;
+  return true;
+}
+
+/* Return true if refernece may be used in address compare.  */
+
+inline bool
+ipa_ref::address_matters_p ()
+{
+  if (use != IPA_REF_ADDR)
+    return false;
+  /* Addresses taken from virtual tables are never compared.  */
+  if (is_a <varpool_node *> (referring)
+      && DECL_VIRTUAL_P (referring->decl))
+    return false;
+  return referred->address_can_be_compared_p ();
+}
+
 /* Build polymorphic call context for indirect call E.  */
 
 inline
Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c	(revision 221034)
+++ ipa-visibility.c	(working copy)
@@ -129,27 +129,6 @@ cgraph_node::local_p (void)
 					
 }
 
-/* Return true when there is a reference to node and it is not vtable.  */
-
-bool
-symtab_node::address_taken_from_non_vtable_p (void)
-{
-  int i;
-  struct ipa_ref *ref = NULL;
-
-  for (i = 0; iterate_referring (i, ref); i++)
-    if (ref->use == IPA_REF_ADDR)
-      {
-	varpool_node *node;
-	if (is_a <cgraph_node *> (ref->referring))
-	  return true;
-	node = dyn_cast <varpool_node *> (ref->referring);
-	if (!DECL_VIRTUAL_P (node->decl))
-	  return true;
-      }
-  return false;
-}
-
 /* A helper for comdat_can_be_unshared_p.  */
 
 static bool
@@ -157,16 +136,14 @@ comdat_can_be_unshared_p_1 (symtab_node
 {
   if (!node->externally_visible)
     return true;
-  /* When address is taken, we don't know if equality comparison won't
-     break eventually. Exception are virutal functions, C++
-     constructors/destructors and vtables, where this is not possible by
-     language standard.  */
-  if (!DECL_VIRTUAL_P (node->decl)
-      && (TREE_CODE (node->decl) != FUNCTION_DECL
-	  || (!DECL_CXX_CONSTRUCTOR_P (node->decl)
-	      && !DECL_CXX_DESTRUCTOR_P (node->decl)))
-      && node->address_taken_from_non_vtable_p ())
-    return false;
+  if (node->address_can_be_compared_p ())
+    {
+      struct ipa_ref *ref;
+
+      for (unsigned int i = 0; node->iterate_referring (i, ref); i++)
+	if (ref->address_matters_p ())
+	  return false;
+    }
 
   /* If the symbol is used in some weird way, better to not touch it.  */
   if (node->force_output)
@@ -387,7 +364,8 @@ can_replace_by_local_alias_in_vtable (sy
 /* walk_tree callback that rewrites initializer references.   */
 
 static tree
-update_vtable_references (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
+update_vtable_references (tree *tp, int *walk_subtrees,
+			  void *data ATTRIBUTE_UNUSED)
 {
   if (TREE_CODE (*tp) == VAR_DECL
       || TREE_CODE (*tp) == FUNCTION_DECL)
Index: cgraphunit.c
===================================================================
--- cgraphunit.c	(revision 221034)
+++ cgraphunit.c	(working copy)
@@ -2468,6 +2468,7 @@ cgraph_node::create_wrapper (cgraph_node
   release_body (true);
   reset ();
 
+  DECL_UNINLINABLE (decl) = false;
   DECL_RESULT (decl) = decl_result;
   DECL_INITIAL (decl) = NULL;
   allocate_struct_function (decl, false);
Index: ipa-icf.c
===================================================================
--- ipa-icf.c	(revision 221034)
+++ ipa-icf.c	(working copy)
@@ -147,7 +147,7 @@ symbol_compare_collection::symbol_compar
 
       if (ref->referred->get_availability () <= AVAIL_INTERPOSABLE)
         {
-	  if (ref->use == IPA_REF_ADDR)
+	  if (ref->address_matters_p ())
 	    m_references.safe_push (ref->referred);
 	  else
 	    m_interposables.safe_push (ref->referred);
@@ -632,8 +633,56 @@ set_local (cgraph_node *node, void *data
   return false;
 }
 
+/* TREE_ADDRESSABLE of NODE to true if DATA is non-NULL.
+   Helper for call_for_symbol_thunks_and_aliases.  */
+
+static bool
+set_addressable (varpool_node *node, void *)
+{
+  TREE_ADDRESSABLE (node->decl) = 1;
+  return false;
+}
+
+/* Redirect all callers of N and its aliases to TO.  Remove aliases if
+   possible.  Return number of redirections made.  */
+
+static int
+redirect_all_callers (cgraph_node *n, cgraph_node *to)
+{
+  int nredirected = 0;
+  ipa_ref *ref;
+
+  while (n->callers)
+    {
+      cgraph_edge *e = n->callers;
+      e->redirect_callee (to);
+      nredirected++;
+    }
+  for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
+    {
+      bool removed = false;
+      cgraph_node *n_alias = dyn_cast <cgraph_node *> (ref->referring);
+
+      if ((DECL_COMDAT_GROUP (n->decl)
+	   && (DECL_COMDAT_GROUP (n->decl)
+	       == DECL_COMDAT_GROUP (n_alias->decl)))
+	  || (n_alias->get_availability () > AVAIL_INTERPOSABLE
+	      && n->get_availability () > AVAIL_INTERPOSABLE))
+	{
+	  nredirected += redirect_all_callers (n_alias, to);
+	  if (n_alias->can_remove_if_no_direct_calls_p ()
+	      && !n_alias->has_aliases_p ())
+	    n_alias->remove ();
+	}
+      if (!removed)
+	i++;
+    }
+  return nredirected;
+}
+
 /* Merges instance with an ALIAS_ITEM, where alias, thunk or redirection can
    be applied.  */
+
 bool
 sem_function::merge (sem_item *alias_item)
 {
@@ -642,16 +691,29 @@ sem_function::merge (sem_item *alias_ite
   sem_function *alias_func = static_cast<sem_function *> (alias_item);
 
   cgraph_node *original = get_node ();
-  cgraph_node *local_original = original;
+  cgraph_node *local_original = NULL;
   cgraph_node *alias = alias_func->get_node ();
-  bool original_address_matters;
-  bool alias_address_matters;
 
-  bool create_thunk = false;
+  bool create_wrapper = false;
   bool create_alias = false;
   bool redirect_callers = false;
+  bool remove = false;
+
   bool original_discardable = false;
 
+  bool original_address_matters = original->address_matters_p ();
+  bool alias_address_matters = alias->address_matters_p ();
+
+  if (DECL_NO_INLINE_WARNING_P (original->decl)
+      != DECL_NO_INLINE_WARNING_P (alias->decl))
+    {
+      if (dump_file)
+	fprintf (dump_file,
+		 "Not unifying; "
+		 "DECL_NO_INLINE_WARNING mismatch.\n\n");
+      return false;
+    }
+
   /* Do not attempt to mix functions from different user sections;
      we do not know what user intends with those.  */
   if (((DECL_SECTION_NAME (original->decl) && !original->implicit_section)
@@ -660,123 +722,173 @@ sem_function::merge (sem_item *alias_ite
     {
       if (dump_file)
 	fprintf (dump_file,
-		 "Not unifying; original and alias are in different sections.\n\n");
+		 "Not unifying; "
+		 "original and alias are in different sections.\n\n");
       return false;
     }
 
   /* See if original is in a section that can be discarded if the main
-     symbol is not used.  */
-  if (DECL_EXTERNAL (original->decl))
-    original_discardable = true;
-  if (original->resolution == LDPR_PREEMPTED_REG
-      || original->resolution == LDPR_PREEMPTED_IR)
-    original_discardable = true;
-  if (original->can_be_discarded_p ())
+     symbol is not used.
+
+     Also consider case where we have resolution info and we know that
+     original's definition is not going to be used.  In this case we can not
+     create alias to original.  */
+  if (original->can_be_discarded_p ()
+      || (node->resolution != LDPR_UNKNOWN
+	  && !decl_binds_to_current_def_p (node->decl)))
     original_discardable = true;
 
-  /* See if original and/or alias address can be compared for equality.  */
-  original_address_matters
-    = (!DECL_VIRTUAL_P (original->decl)
-       && (original->externally_visible
-	   || original->address_taken_from_non_vtable_p ()));
-  alias_address_matters
-    = (!DECL_VIRTUAL_P (alias->decl)
-       && (alias->externally_visible
-	   || alias->address_taken_from_non_vtable_p ()));
-
-  /* If alias and original can be compared for address equality, we need
-     to create a thunk.  Also we can not create extra aliases into discardable
-     section (or we risk link failures when section is discarded).  */
-  if ((original_address_matters
-       && alias_address_matters)
+  /* Creating a symtab alias is the optimal way to merge.
+     It however can not be used in the following cases:
+
+     1) if ORIGINAL and ALIAS may be possibly compared for address equality.
+     2) if ORIGINAL is in a section that may be discarded by linker or if
+	it is an external functions where we can not create an alias
+	(ORIGINAL_DISCARDABLE)
+     3) if target do not support symbol aliases.
+
+     If we can not produce alias, we will turn ALIAS into WRAPPER of ORIGINAL
+     and/or redirect all callers from ALIAS to ORIGINAL.  */
+  if ((original_address_matters && alias_address_matters)
       || original_discardable
-      || DECL_COMDAT_GROUP (alias->decl)
       || !sem_item::target_supports_symbol_aliases_p ())
     {
-      create_thunk = !stdarg_p (TREE_TYPE (alias->decl));
-      create_alias = false;
-      /* When both alias and original are not overwritable, we can save
-         the extra thunk wrapper for direct calls.  */
+      /* First see if we can produce wrapper.  */
+
+      /* Do not turn function in one comdat group into wrapper to another
+	 comdat group. Other compiler producing the body of the
+	 another comdat group may make opossite decision and with unfortunate
+	 linker choices this may close a loop.  */
+      if (DECL_COMDAT_GROUP (alias->decl)
+	  && (DECL_COMDAT_GROUP (alias->decl)
+	      != DECL_COMDAT_GROUP (original->decl)))
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "Wrapper cannot be created because of COMDAT\n");
+	}
+      else if (DECL_STATIC_CHAIN (alias->decl))
+        {
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "Can not create wrapper of nested functions.\n");
+        }
+      /* TODO: We can also deal with variadic functions never calling
+	 VA_START.  */
+      else if (stdarg_p (TREE_TYPE (alias->decl)))
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "can not create wrapper of stdarg function.\n");
+	}
+      else if (inline_summaries
+	       && inline_summaries->get (alias)->self_size <= 2)
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Wrapper creation is not "
+		     "profitable (function is too small).\n");
+	}
+      /* If user paid attention to mark function noinline, assume it is
+	 somewhat special and do not try to turn it into a wrapper that can
+	 not be undone by inliner.  */
+      else if (lookup_attribute ("noinline", DECL_ATTRIBUTES (alias->decl)))
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Wrappers are not created for noinline.\n");
+	}
+      else
+        create_wrapper = true;
+
+      /* We can redirect local calls in the case both alias and orignal
+	 are not interposable.  */
       redirect_callers
-	= (!original_discardable
-	   && !DECL_COMDAT_GROUP (alias->decl)
-	   && alias->get_availability () > AVAIL_INTERPOSABLE
-	   && original->get_availability () > AVAIL_INTERPOSABLE
-	   && !alias->instrumented_version);
-    }
-  else
-    {
-      create_alias = true;
-      create_thunk = false;
-      redirect_callers = false;
-    }
+	= alias->get_availability () > AVAIL_INTERPOSABLE
+	  && original->get_availability () > AVAIL_INTERPOSABLE
+	  && !alias->instrumented_version;
 
-  /* We want thunk to always jump to the local function body
-     unless the body is comdat and may be optimized out.  */
-  if ((create_thunk || redirect_callers)
-      && (!original_discardable
+      if (!redirect_callers && !create_wrapper)
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Not unifying; can not redirect callers nor "
+		     "produce wrapper\n\n");
+	  return false;
+	}
+
+      /* Work out the symbol the wrapper should call.
+	 If ORIGINAL is interposable, we need to call a local alias.
+	 Also produce local alias (if possible) as an optimization.  */
+      if (!original_discardable
 	  || (DECL_COMDAT_GROUP (original->decl)
 	      && (DECL_COMDAT_GROUP (original->decl)
-		  == DECL_COMDAT_GROUP (alias->decl)))))
-    local_original
-      = dyn_cast <cgraph_node *> (original->noninterposable_alias ());
-
-    if (!local_original)
-      {
-	if (dump_file)
-	  fprintf (dump_file, "Noninterposable alias cannot be created.\n\n");
-
-	return false;
-      }
+		  == DECL_COMDAT_GROUP (alias->decl))))
+	{
+	  local_original
+	    = dyn_cast <cgraph_node *> (original->noninterposable_alias ());
+	  if (!local_original
+	      && original->get_availability () > AVAIL_INTERPOSABLE)
+	    local_original = original;
+	  /* If original is COMDAT local, we can not really redirect external
+	     callers to it.  */
+	  if (original->comdat_local_p ())
+	    redirect_callers = false;
+	}
+      /* If we can not use local alias, fallback to the original
+	 when possible.  */
+      else if (original->get_availability () > AVAIL_INTERPOSABLE)
+	local_original = original;
+      if (!local_original)
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Not unifying; "
+		     "can not produce local alias.\n\n");
+	  return false;
+	}
 
-  if (!decl_binds_to_current_def_p (alias->decl))
-    {
-      if (dump_file)
-	fprintf (dump_file, "Declaration does not bind to currect definition.\n\n");
-      return false;
+      if (!redirect_callers && !create_wrapper)
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Not unifying; "
+		     "can not redirect callers nor produce a wrapper\n\n");
+	  return false;
+	}
+      if (!create_wrapper
+	  && !alias->can_remove_if_no_direct_calls_p ())
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Not unifying; can not make wrapper and "
+		     "function has other uses than direct calls\n\n");
+	  return false;
+	}
     }
+  else
+    create_alias = true;
 
   if (redirect_callers)
     {
-      /* If alias is non-overwritable then
-         all direct calls are safe to be redirected to the original.  */
-      bool redirected = false;
-      while (alias->callers)
-	{
-	  cgraph_edge *e = alias->callers;
-	  e->redirect_callee (local_original);
-	  push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl));
+      int nredirected = redirect_all_callers (alias, local_original);
 
-	  if (e->call_stmt)
-	    e->redirect_call_stmt_to_callee ();
+      if (nredirected)
+	{
+	  alias->icf_merged = true;
+	  local_original->icf_merged = true;
 
-	  pop_cfun ();
-	  redirected = true;
+	  if (dump_file && nredirected)
+	    fprintf (dump_file, "%i local calls have been "
+		     "redirected.\n", nredirected);
 	}
 
-      alias->icf_merged = true;
-      if (local_original->lto_file_data
-	  && alias->lto_file_data
-	  && local_original->lto_file_data != alias->lto_file_data)
-      local_original->merged = true;
-
-      /* The alias function is removed if symbol address
-         does not matter.  */
-      if (!alias_address_matters)
-	alias->remove ();
-
-      if (dump_file && redirected)
-	fprintf (dump_file, "Callgraph local calls have been redirected.\n\n");
+      /* If all callers was redirected, do not produce wrapper.  */
+      if (alias->can_remove_if_no_direct_calls_p ()
+	  && !alias->has_aliases_p ())
+	{
+	  create_wrapper = false;
+	  remove = true;
+	}
+      gcc_assert (!create_alias);
     }
-  /* If the condtion above is not met, we are lucky and can turn the
-     function into real alias.  */
   else if (create_alias)
     {
       alias->icf_merged = true;
-      if (local_original->lto_file_data
-	  && alias->lto_file_data
-	  && local_original->lto_file_data != alias->lto_file_data)
-      local_original->merged = true;
 
       /* Remove the function's body.  */
       ipa_merge_profiles (original, alias);
@@ -791,39 +903,38 @@ sem_function::merge (sem_item *alias_ite
 	 (set_local, (void *)(size_t) original->local_p (), true);
 
       if (dump_file)
-	fprintf (dump_file, "Callgraph alias has been created.\n\n");
+	fprintf (dump_file, "Unified; Function alias has been created.\n\n");
     }
-  else if (create_thunk)
+  if (create_wrapper)
     {
-      if (DECL_COMDAT_GROUP (alias->decl))
-	{
-	  if (dump_file)
-	    fprintf (dump_file, "Callgraph thunk cannot be created because of COMDAT\n");
+      gcc_assert (!create_alias);
+      alias->icf_merged = true;
+      local_original->icf_merged = true;
 
-	  return 0;
-	}
+      ipa_merge_profiles (local_original, alias, true);
+      alias->create_wrapper (local_original);
 
-      if (DECL_STATIC_CHAIN (alias->decl))
-        {
-         if (dump_file)
-           fprintf (dump_file, "Thunk creation is risky for static-chain functions.\n\n");
+      if (dump_file)
+	fprintf (dump_file, "Unified; Wrapper has been created.\n\n");
+    }
+  gcc_assert (alias->icf_merged || remove);
+  original->icf_merged = true;
 
-         return 0;
-        }
+  /* Inform the inliner about cross-module merging.  */
+  if ((original->lto_file_data || alias->lto_file_data)
+      && original->lto_file_data != alias->lto_file_data)
+    local_original->merged = original->merged = true;
 
+  if (remove)
+    {
+      ipa_merge_profiles (original, alias);
+      alias->release_body ();
+      alias->reset ();
+      alias->body_removed = true;
       alias->icf_merged = true;
-      if (local_original->lto_file_data
-	  && alias->lto_file_data
-	  && local_original->lto_file_data != alias->lto_file_data)
-      local_original->merged = true;
-      ipa_merge_profiles (local_original, alias, true);
-      alias->create_wrapper (local_original);
-
       if (dump_file)
-	fprintf (dump_file, "Callgraph thunk has been created.\n\n");
+	fprintf (dump_file, "Unified; Function body was removed.\n");
     }
-  else if (dump_file)
-    fprintf (dump_file, "Callgraph merge operation cannot be performed.\n\n");
 
   return true;
 }
@@ -1319,7 +1430,8 @@ sem_variable::merge (sem_item *alias_ite
   if (!sem_item::target_supports_symbol_aliases_p ())
     {
       if (dump_file)
-	fprintf (dump_file, "Symbol aliases are not supported by target\n\n");
+	fprintf (dump_file, "Not unifying; "
+		 "Symbol aliases are not supported by target\n\n");
       return false;
     }
 
@@ -1329,73 +1441,94 @@ sem_variable::merge (sem_item *alias_ite
   varpool_node *alias = alias_var->get_node ();
   bool original_discardable = false;
 
+  bool original_address_matters = original->address_matters_p ();
+  bool alias_address_matters = alias->address_matters_p ();
+
   /* See if original is in a section that can be discarded if the main
-     symbol is not used.  */
-  if (DECL_EXTERNAL (original->decl))
-    original_discardable = true;
-  if (original->resolution == LDPR_PREEMPTED_REG
-      || original->resolution == LDPR_PREEMPTED_IR)
-    original_discardable = true;
-  if (original->can_be_discarded_p ())
+     symbol is not used.
+     Also consider case where we have resolution info and we know that
+     original's definition is not going to be used.  In this case we can not
+     create alias to original.  */
+  if (original->can_be_discarded_p ()
+      || (node->resolution != LDPR_UNKNOWN
+	  && !decl_binds_to_current_def_p (node->decl)))
     original_discardable = true;
 
   gcc_assert (!TREE_ASM_WRITTEN (alias->decl));
 
-  if (original_discardable || DECL_EXTERNAL (alias_var->decl) ||
-      !compare_sections (alias_var))
+  /* Constant pool machinery is not quite ready for aliases.
+     TODO: varasm code contains logic for merging DECL_IN_CONSTANT_POOL.
+     For LTO merging does not happen that is an important missing feature.
+     We can enable merging with LTO if the DECL_IN_CONSTANT_POOL
+     flag is dropped and non-local symbol name is assigned.  */
+  if (DECL_IN_CONSTANT_POOL (alias->decl)
+      || DECL_IN_CONSTANT_POOL (original->decl))
     {
       if (dump_file)
-	fprintf (dump_file, "Varpool alias cannot be created\n\n");
+	fprintf (dump_file,
+		 "Not unifying; constant pool variables.\n\n");
+      return false;
+    }
 
+  /* Do not attempt to mix functions from different user sections;
+     we do not know what user intends with those.  */
+  if (((DECL_SECTION_NAME (original->decl) && !original->implicit_section)
+       || (DECL_SECTION_NAME (alias->decl) && !alias->implicit_section))
+      && DECL_SECTION_NAME (original->decl) != DECL_SECTION_NAME (alias->decl))
+    {
+      if (dump_file)
+	fprintf (dump_file,
+		 "Not unifying; "
+		 "original and alias are in different sections.\n\n");
       return false;
     }
-  else
+
+  /* We can not merge if address comparsion metters.  */
+  if (original_address_matters && alias_address_matters
+      && flag_merge_constants < 2)
     {
-      // alias cycle creation check
-      varpool_node *n = original;
+      if (dump_file)
+	fprintf (dump_file,
+		 "Not unifying; "
+		 "adress of original and alias may be compared.\n\n");
+      return false;
+    }
 
-      while (n->alias)
-	{
-	  n = n->get_alias_target ();
-	  if (n == alias)
-	    {
-	      if (dump_file)
-		fprintf (dump_file, "Varpool alias cannot be created (alias cycle).\n\n");
+  if (original_discardable
+      && (!DECL_COMDAT_GROUP (original->decl)
+	  || (DECL_COMDAT_GROUP (original->decl)
+	      != DECL_COMDAT_GROUP (alias->decl))))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Not unifying; alias cannot be created; "
+		 "target is discardable\n\n");
 
-	      return false;
-	    }
-	}
+      return false;
+    }
+  else
+    {
+      gcc_assert (!original->alias);
+      gcc_assert (!alias->alias);
 
       alias->analyzed = false;
 
       DECL_INITIAL (alias->decl) = NULL;
       alias->need_bounds_init = false;
       alias->remove_all_references ();
+      if (TREE_ADDRESSABLE (alias->decl))
+        original->call_for_symbol_thunks_and_aliases
+	 (set_addressable, NULL, true);
 
       varpool_node::create_alias (alias_var->decl, decl);
       alias->resolve_alias (original);
 
       if (dump_file)
-	fprintf (dump_file, "Varpool alias has been created.\n\n");
+	fprintf (dump_file, "Unified; Variable alias has been created.\n\n");
 
       return true;
     }
 }
 
-bool
-sem_variable::compare_sections (sem_variable *alias)
-{
-  const char *source = node->get_section ();
-  const char *target = alias->node->get_section();
-
-  if (source == NULL && target == NULL)
-    return true;
-  else if(!source || !target)
-    return false;
-  else
-    return strcmp (source, target) == 0;
-}
-
 /* Dump symbol to FILE.  */
 
 void
Index: cgraphclones.c
===================================================================
--- cgraphclones.c	(revision 221034)
+++ cgraphclones.c	(working copy)
@@ -471,6 +471,8 @@ cgraph_node::create_clone (tree decl, gc
   new_node->frequency = frequency;
   new_node->tp_first_run = tp_first_run;
   new_node->tm_clone = tm_clone;
+  new_node->icf_merged = icf_merged;
+  new_node->merged = merged;
 
   new_node->clone.tree_map = NULL;
   new_node->clone.args_to_skip = args_to_skip;