Patchwork Fix varpool confussion about what to remove and what not

login
register
mail settings
Submitter Jan Hubicka
Date Nov. 16, 2010, 4:49 p.m.
Message ID <20101116164922.GB6660@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/71432/
State New
Headers show

Comments

Jan Hubicka - Nov. 16, 2010, 4:49 p.m.
Hi,
this patch makes varpool less confused about removal of unreferenced variables.
This is done twice. First as part of cgraph_remove_unreachable_nodes that currently
contains a bug that makes it to remove unused vars even with -fno-toplevel-reorder
and at varpool_remove_unreferenced_decls (after function bodies has been assembled)
that gets -fno-toplevel-reorder but uses decide_is_variable_needed that is intended
for varpool_finalize_node use only and is completely confused about whole program
and LTO and consequently makes wore decisions with -fuse-linker-plugin than with
-fwhole-program.

I noticed another bug; we used to mark all variables referenced after late
GIMPLE optimization but we don't do this anymore.  This makes the dubious
DECL_RTL_SET_P neccesary.  This check was introduced to work around dwarf2out
referencing unreferenced vars. This problem was solved at dwarf2out side some
time ago. I will look into this problem incrementally.

Boostrapped/regtested x86_64-linux, will commit it later today.

	* cgraph.h (varpool_can_remove_if_no_refs): Move here from ...;
	when !flag_toplevel_reorder do not remove unless variable is
	COMDAT or ARTIFICIAL.
	* ipa.c (varpool_can_remove_if_no_refs): ... here.
	(cgraph_remove_unreachable_nodes): Only analyzed nodes needs to stay.
	* cgraphunit.c (cgraph_analyze_functions): Dump varpool, too.
	* varpool.c (decide_is_variable_needed): Do not handle visibility issues.
	(varpool_finalize_decl): Likewise.
	(varpool_remove_unreferenced_decls): Use varpool_mark_needed_node; update
	outdated comment on DECL_RTL_SET_P check.
IainS - Nov. 17, 2010, 8:34 a.m.
Hi Honza...

On 16 Nov 2010, at 16:49, Jan Hubicka wrote:

> Hi,
> this patch makes varpool less confused about removal of unreferenced  
> variables.
> This is done twice. First as part of cgraph_remove_unreachable_nodes  
> that currently
> contains a bug that makes it to remove unused vars even with -fno- 
> toplevel-reorder
> and at varpool_remove_unreferenced_decls (after function bodies has  
> been assembled)
> that gets -fno-toplevel-reorder but uses decide_is_variable_needed  
> that is intended
> for varpool_finalize_node use only and is completely confused about  
> whole program
> and LTO and consequently makes wore decisions with -fuse-linker- 
> plugin than with
> -fwhole-program.
>
> I noticed another bug; we used to mark all variables referenced  
> after late
> GIMPLE optimization but we don't do this anymore.  This makes the  
> dubious
> DECL_RTL_SET_P neccesary.  This check was introduced to work around  
> dwarf2out
> referencing unreferenced vars. This problem was solved at dwarf2out  
> side some
> time ago. I will look into this problem incrementally.
>
> Boostrapped/regtested x86_64-linux, will commit it later today.
>
> 	* cgraph.h (varpool_can_remove_if_no_refs): Move here from ...;
> 	when !flag_toplevel_reorder do not remove unless variable is
> 	COMDAT or ARTIFICIAL.
> 	* ipa.c (varpool_can_remove_if_no_refs): ... here.
> 	(cgraph_remove_unreachable_nodes): Only analyzed nodes needs to stay.
> 	* cgraphunit.c (cgraph_analyze_functions): Dump varpool, too.
> 	* varpool.c (decide_is_variable_needed): Do not handle visibility  
> issues.
> 	(varpool_finalize_decl): Likewise.
> 	(varpool_remove_unreferenced_decls): Use varpool_mark_needed_node;  
> update
> 	outdated comment on DECL_RTL_SET_P check.

r166812 breaks *-darwin9 bootstrap @ stage1/libgomp with:

bin/sh ./libtool --tag=CC   --mode=compile /GCC/gcc-4-6-reghunt- 
build/./gcc/xgcc -B/GCC/gcc-4-6-reghunt-build/./gcc/ -B/GCC/gcc-4-6-rh- 
install/i686-apple-darwin9/bin/ -B/GCC/gcc-4-6-rh-install/i686-apple- 
darwin9/lib/ -isystem /GCC/gcc-4-6-rh-install/i686-apple-darwin9/ 
include -isystem /GCC/gcc-4-6-rh-install/i686-apple-darwin9/sys- 
include    -DHAVE_CONFIG_H -I. -I/GCC/gcc-4-6-reghunt/libgomp  -I/GCC/ 
gcc-4-6-reghunt/libgomp/config/bsd -I/GCC/gcc-4-6-reghunt/libgomp/ 
config/posix -I/GCC/gcc-4-6-reghunt/libgomp  -Wall -Werror -Wc,- 
pthread -g -O2 -MT barrier.lo -MD -MP -MF .deps/barrier.Tpo -c -o  
barrier.lo /GCC/gcc-4-6-reghunt/libgomp/barrier.c
libtool: compile:  /GCC/gcc-4-6-reghunt-build/./gcc/xgcc -B/GCC/ 
gcc-4-6-reghunt-build/./gcc/ -B/GCC/gcc-4-6-rh-install/i686-apple- 
darwin9/bin/ -B/GCC/gcc-4-6-rh-install/i686-apple-darwin9/lib/ - 
isystem /GCC/gcc-4-6-rh-install/i686-apple-darwin9/include -isystem / 
GCC/gcc-4-6-rh-install/i686-apple-darwin9/sys-include -DHAVE_CONFIG_H - 
I. -I/GCC/gcc-4-6-reghunt/libgomp -I/GCC/gcc-4-6-reghunt/libgomp/ 
config/bsd -I/GCC/gcc-4-6-reghunt/libgomp/config/posix -I/GCC/gcc-4-6- 
reghunt/libgomp -Wall -pthread -Werror -g -O2 -MT barrier.lo -MD -MP - 
MF .deps/barrier.Tpo -c /GCC/gcc-4-6-reghunt/libgomp/barrier.c  -fno- 
common -DPIC -o .libs/barrier.o
/GCC/gcc-4-6-reghunt/libgomp/barrier.c:41:1: internal compiler error:  
in decide_is_variable_needed, at varpool.c:338
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[5]: *** [barrier.lo] Error 1

any more info needed ... let me know.
cheers
Iain

Patch

Index: cgraph.h
===================================================================
--- cgraph.h	(revision 166777)
+++ cgraph.h	(working copy)
@@ -928,6 +928,18 @@  cgraph_can_remove_if_no_direct_calls_p (
   return !node->address_taken && cgraph_can_remove_if_no_direct_calls_and_refs_p (node);
 }
 
+/* Return true when function NODE can be removed from callgraph
+   if all direct calls are eliminated.  */
+
+static inline bool
+varpool_can_remove_if_no_refs (struct varpool_node *node)
+{
+  return (!node->force_output && !node->used_from_other_partition
+	  && (flag_toplevel_reorder || DECL_COMDAT (node->decl)
+	      || DECL_ARTIFICIAL (node->decl))
+  	  && (DECL_COMDAT (node->decl) || !node->externally_visible));
+}
+
 /* Return true when all references to VNODE must be visible in ipa_ref_list.
    i.e. if the variable is not externally visible or not used in some magic
    way (asm statement or such).
Index: cgraphunit.c
===================================================================
--- cgraphunit.c	(revision 166777)
+++ cgraphunit.c	(working copy)
@@ -943,6 +943,7 @@  cgraph_analyze_functions (void)
 	  fprintf (cgraph_dump_file, " %s", cgraph_node_name (node));
       fprintf (cgraph_dump_file, "\n\nInitial ");
       dump_cgraph (cgraph_dump_file);
+      dump_varpool (cgraph_dump_file);
     }
 
   if (cgraph_dump_file)
@@ -972,6 +973,7 @@  cgraph_analyze_functions (void)
     {
       fprintf (cgraph_dump_file, "\n\nReclaimed ");
       dump_cgraph (cgraph_dump_file);
+      dump_varpool (cgraph_dump_file);
     }
   bitmap_obstack_release (NULL);
   first_analyzed = cgraph_nodes;
@@ -1816,6 +1818,7 @@  cgraph_optimize (void)
     {
       fprintf (cgraph_dump_file, "\nFinal ");
       dump_cgraph (cgraph_dump_file);
+      dump_varpool (cgraph_dump_file);
     }
 #ifdef ENABLE_CHECKING
   verify_cgraph ();
Index: ipa.c
===================================================================
--- ipa.c	(revision 166777)
+++ ipa.c	(working copy)
@@ -188,16 +188,6 @@  process_references (struct ipa_ref_list 
     }
 }
 
-/* Return true when function NODE can be removed from callgraph
-   if all direct calls are eliminated.  */
-
-static inline bool
-varpool_can_remove_if_no_refs (struct varpool_node *node)
-{
-  return (!node->force_output && !node->used_from_other_partition
-  	  && (DECL_COMDAT (node->decl) || !node->externally_visible));
-}
-
 /* Return true when function can be marked local.  */
 
 static bool
@@ -269,7 +259,8 @@  cgraph_remove_unreachable_nodes (bool be
     {
       vnode->next_needed = NULL;
       vnode->prev_needed = NULL;
-      if (!varpool_can_remove_if_no_refs (vnode))
+      if (vnode->analyzed
+	  && !varpool_can_remove_if_no_refs (vnode))
 	{
 	  vnode->needed = false;
 	  varpool_mark_needed_node (vnode);
Index: varpool.c
===================================================================
--- varpool.c	(revision 166777)
+++ varpool.c	(working copy)
@@ -331,31 +331,24 @@  varpool_reset_queue (void)
 bool
 decide_is_variable_needed (struct varpool_node *node, tree decl)
 {
-  if (node->used_from_other_partition)
-    return true;
   /* If the user told us it is used, then it must be so.  */
-  if ((node->externally_visible && !DECL_COMDAT (decl))
-      || node->force_output)
+  if (node->force_output)
     return true;
 
+  gcc_assert (!DECL_EXTERNAL (decl));
+
   /* Externally visible variables must be output.  The exception is
      COMDAT variables that must be output only when they are needed.  */
   if (TREE_PUBLIC (decl)
-      && !flag_whole_program
-      && !flag_lto
       && !DECL_COMDAT (decl)
       && !DECL_EXTERNAL (decl))
     return true;
 
   /* When not reordering top level variables, we have to assume that
      we are going to keep everything.  */
-  if (flag_toplevel_reorder)
-    return false;
-
-  /* We want to emit COMDAT variables only when absolutely necessary.  */
-  if (DECL_COMDAT (decl))
-    return false;
-  return true;
+  if (!flag_toplevel_reorder)
+    return true;
+  return false;
 }
 
 /* Return if DECL is constant and its initial value is known (so we can do
@@ -427,11 +420,6 @@  varpool_finalize_decl (tree decl)
 
   if (decide_is_variable_needed (node, decl))
     varpool_mark_needed_node (node);
-  /* Since we reclaim unreachable nodes at the end of every language
-     level unit, we need to be conservative about possible entry points
-     there.  */
-  else if (TREE_PUBLIC (decl) && !DECL_COMDAT (decl) && !DECL_EXTERNAL (decl))
-    varpool_mark_needed_node (node);
   if (cgraph_global_info_ready)
     varpool_assemble_pending_decls ();
 }
@@ -557,18 +545,14 @@  varpool_remove_unreferenced_decls (void)
 
   while (node)
     {
-      tree decl = node->decl;
       next = node->next_needed;
       node->needed = 0;
 
-      if (node->finalized
-	  && (decide_is_variable_needed (node, decl)
-	      /* ??? Cgraph does not yet rule the world with an iron hand,
-		 and does not control the emission of debug information.
-		 After a variable has its DECL_RTL set, we must assume that
-		 it may be referenced by the debug information, and we can
-		 no longer elide it.  */
-	      || DECL_RTL_SET_P (decl)))
+      if (node->analyzed
+	  && (!varpool_can_remove_if_no_refs (node)
+	      /* We just expanded all function bodies.  See if any of
+		 them needed the variable.  */
+	      || DECL_RTL_SET_P (node->decl)))
 	varpool_mark_needed_node (node);
 
       node = next;