This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

More of icf::merge TLC


Hi,
the prevoius bug in COMDAT handling made me to revisit the code once again.
There was few bugs fixed bellow.

1) If we have resolution info and we know original is going to be discarded
   we need to prevail COMDAT logic
2) It is possible to merge non-comdat to comdat and comdat to non-comdat.
   It is just not possible to merge two comdats to avoid possible cycles.
3) We do not want to rpoduce local aliases inside comdats since these will be
   comdat locals and prevent inlining.

Bootstrapped/regtested x86_64-linux, commited.

Honza

	* ipa-icf.c (sem_function::merge): Fix handling of COMDAT.
	(sem_variable::merge) Likewise.
Index: ipa-icf.c
===================================================================
--- ipa-icf.c	(revision 221079)
+++ ipa-icf.c	(working copy)
@@ -709,6 +709,7 @@ sem_function::merge (sem_item *alias_ite
   bool remove = false;
 
   bool original_discardable = false;
+  bool original_discarded = false;
 
   bool original_address_matters = original->address_matters_p ();
   bool alias_address_matters = alias->address_matters_p ();
@@ -737,15 +738,16 @@ sem_function::merge (sem_item *alias_ite
     }
 
   /* See if original is in a section that can be discarded if the main
-     symbol is not used.
+     symbol is not used.  */
 
-     Also consider case where we have resolution info and we know that
+  if (original->can_be_discarded_p ())
+    original_discardable = true;
+  /* 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;
+  if (node->resolution != LDPR_UNKNOWN
+      && !decl_binds_to_current_def_p (node->decl))
+    original_discardable = original_discarded = true;
 
   /* Creating a symtab alias is the optimal way to merge.
      It however can not be used in the following cases:
@@ -764,6 +766,7 @@ sem_function::merge (sem_item *alias_ite
 	  && (!DECL_COMDAT_GROUP (alias->decl)
 	      || (DECL_COMDAT_GROUP (alias->decl)
 		  != DECL_COMDAT_GROUP (original->decl))))
+      || original_discarded
       || !sem_item::target_supports_symbol_aliases_p ()
       || DECL_COMDAT_GROUP (alias->decl) != DECL_COMDAT_GROUP (original->decl))
     {
@@ -773,7 +776,7 @@ sem_function::merge (sem_item *alias_ite
 	 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)
+      if (DECL_COMDAT_GROUP (original->decl) && DECL_COMDAT_GROUP (alias->decl)
 	  && (DECL_COMDAT_GROUP (alias->decl)
 	      != DECL_COMDAT_GROUP (original->decl)))
 	{
@@ -830,26 +833,27 @@ sem_function::merge (sem_item *alias_ite
 
       /* 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))))
+	 Also produce local alias (if possible) as an optimization.
+
+	 Local aliases can not be created inside comdat groups because that
+	 prevents inlining.  */
+      if (!original_discardable && !original->get_comdat_group ())
 	{
 	  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 original is COMDAT local, we can not really redirect calls outside
+	 of its comdat group to it.  */
+      if (original->comdat_local_p ())
+        redirect_callers = false;
       if (!local_original)
 	{
 	  if (dump_file)
@@ -1510,11 +1514,16 @@ sem_variable::merge (sem_item *alias_ite
 		 "adress of original and alias may be compared.\n\n");
       return false;
     }
+  if (DECL_COMDAT_GROUP (original->decl) != DECL_COMDAT_GROUP (alias->decl))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Not unifying; alias cannot be created; "
+		 "across comdat group boundary\n\n");
+
+      return false;
+    }
 
-  if (original_discardable
-      && (!DECL_COMDAT_GROUP (original->decl)
-	  || (DECL_COMDAT_GROUP (original->decl)
-	      != DECL_COMDAT_GROUP (alias->decl))))
+  if (original_discardable)
     {
       if (dump_file)
 	fprintf (dump_file, "Not unifying; alias cannot be created; "


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]