Patchwork Make some comdats implicitly hidden

login
register
mail settings
Submitter Jan Hubicka
Date Aug. 26, 2013, 7:57 p.m.
Message ID <20130826195724.GB13069@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/269964/
State New
Headers show

Comments

Jan Hubicka - Aug. 26, 2013, 7:57 p.m.
Hi,
for -flto (and for -fhwhole-program as well) I for a while privatize comdat
symbols based on fact that I know their address can not be compared for
equivality (virtuals/ctors/dtors and functions with no address taken).

While analyzing the relocations of libreoffice I noticed that I can play
the same game w/o LTO at linker level.  Making those symbols hidden truns
external relocations to internal and should improve runtime, too: comdat
sharing by dynamic linker is expensive and won't save duplicated functions
from the binary.

There is ext/visibility/template2.C that fails with the patch. It tests that
visibility pragma does not bring the symbol local, but now we do so based
on logic above.

Jason, do you have any idea how to fix the testcase? I tried to use different
visility but that doesn't work, since we do not have corresponding scan-not-*

Bootstrapped/regtested x86_64-linux

Honza

	* ipa.c (function_and_variable_visibility): Make comdats that can be
	unshared hidden.
Jason Merrill - Aug. 27, 2013, 1:04 a.m.
On 08/26/2013 03:57 PM, Jan Hubicka wrote:
> While analyzing the relocations of libreoffice I noticed that I can play
> the same game w/o LTO at linker level.  Making those symbols hidden truns
> external relocations to internal and should improve runtime, too: comdat
> sharing by dynamic linker is expensive and won't save duplicated functions
> from the binary.

Makes sense.

> There is ext/visibility/template2.C that fails with the patch. It tests that
> visibility pragma does not bring the symbol local, but now we do so based
> on logic above.
>
> Jason, do you have any idea how to fix the testcase? I tried to use different
> visility but that doesn't work, since we do not have corresponding scan-not-*

Maybe change it to use a function that has its address taken, so this 
optimization doesn't apply.

Jason

Patch

Index: ipa.c
===================================================================
--- ipa.c	(revision 202001)
+++ ipa.c	(working copy)
@@ -873,6 +873,17 @@  function_and_variable_visibility (bool w
 	       segfault though. */
 	    symtab_dissolve_same_comdat_group_list ((symtab_node) node);
 	}
+      if (node->symbol.externally_visible
+          && DECL_COMDAT (node->symbol.decl)
+	  && comdat_can_be_unshared_p ((symtab_node) node))
+	{
+	  if (dump_file
+	      && DECL_VISIBILITY (node->symbol.decl) != VISIBILITY_HIDDEN)
+	    fprintf (dump_file, "Promoting visibility to hidden: %s/%i\n",
+		     cgraph_node_name (node), node->symbol.order);
+	  DECL_VISIBILITY (node->symbol.decl) = VISIBILITY_HIDDEN;
+	  DECL_VISIBILITY_SPECIFIED (node->symbol.decl) = true;
+	}
 
       if (node->thunk.thunk_p
 	  && TREE_PUBLIC (node->symbol.decl))
@@ -980,6 +991,17 @@  function_and_variable_visibility (bool w
 	    symtab_dissolve_same_comdat_group_list ((symtab_node) vnode);
 	  vnode->symbol.resolution = LDPR_PREVAILING_DEF_IRONLY;
 	}
+      if (vnode->symbol.externally_visible
+          && DECL_COMDAT (vnode->symbol.decl)
+	  && comdat_can_be_unshared_p ((symtab_node) vnode))
+	{
+	  if (dump_file
+	      && DECL_VISIBILITY (vnode->symbol.decl) == VISIBILITY_HIDDEN)
+	    fprintf (dump_file, "Promoting visibility to hidden: %s/%i\n",
+		     varpool_node_name (vnode), vnode->symbol.order);
+	  DECL_VISIBILITY (vnode->symbol.decl) = VISIBILITY_HIDDEN;
+	  DECL_VISIBILITY_SPECIFIED (vnode->symbol.decl) = true;
+	}
     }
 
   if (dump_file)