public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [lto] fix local statics
@ 2007-12-22 19:23 Nathan Froyd
  0 siblings, 0 replies; only message in thread
From: Nathan Froyd @ 2007-12-22 19:23 UTC (permalink / raw)
  To: gcc-patches; +Cc: zadeck

We were miscompiling situations where a function had a static local
variable FOO and was inlined multiple times in a translation unit or was
inlined > 0 times and was called from other translation units.  The
problem was that when reading the LTO objects, the inlined copies were
all given distinct instances of FOO's VAR_DECL, when in the original
code, they all shared the same instance.  This change would cause
multiple distinct declarations to be output to the assembly file,
multiple distinct locations to be updated when we only wanted one, and
general failure.

The patch below to ensure that we only have one copy of a local static
variable everywhere is mostly Kenny's work.  He couldn't commit his
parts without breaking things, however, so he asked me to cmmit his bits
along with whatever I did to make things work.  The dwarf2out bits and
ensuring that local statics have DECL_EXTERNAL == 0 was my doing.

I am not particularly happy with the twiddling of the DECL_IGNORED_P
flag in dwarf2out.c.  This example highlights the mismatch between what
dwarf2out wants to accomplish and what LTO wants to (ab)use it for.  In
its defense, it does make 186.crafty compile properly.

Eventually I think we want to replace the dwarf2 parts of LTO with
something more suited to our purposes.

With this change, 164.gzip now runs successfully.

Committed to the LTO branch.

-Nathan

gcc/
	* dwarf2out.c (force_decl_die): Unset the DECL_IGNORED_P flag if
	we are called from LTO.  Remove the DW_AT_declaration flag if the
	decl has an initial value.
	* lto-function-out.c (output_expr_operand): Add explicit check
	for statics or externs rathen than use DECL_CONTEXT.
	(output_local_vars): Serialize the DECL_INITIAL field of local
	statics and add it the unexpanded_vars_list as necessary.

gcc/lto/
	* lto-read.c (input_expr_operand): Fixed uninitialize var warning.
	(input_local_vars): Read in DECL_INITIAL and context for local
	statics that need to be put in unexpanded_vars_list.

Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 131119)
+++ dwarf2out.c	(working copy)
@@ -13538,6 +13538,7 @@ force_decl_die (tree decl)
 {
   dw_die_ref decl_die;
   unsigned saved_external_flag;
+  unsigned saved_ignored_flag;
   tree save_fn = NULL_TREE;
   decl_die = lookup_decl_die (decl);
   if (!decl_die)
@@ -13571,11 +13572,21 @@ force_decl_die (tree decl)
 	    }
 	  else
 	    {
-	      /* Set external flag to force declaration die. Restore it after
-		 gen_decl_die() call.  */
+	      /* Set external flag to force declaration die, thereby
+		 associating the decl with the generated die.  Unset the
+		 ignored flag if we are running from LTO and are thereby
+		 forcing everything we can.  Restore these flags after
+		 the gen_decl_die() call.  */
 	      saved_external_flag = DECL_EXTERNAL (decl);
+	      if (dwarf2_called_from_lto_p)
+		{
+		  saved_ignored_flag = DECL_IGNORED_P (decl);
+		  DECL_IGNORED_P (decl) = 0;
+		}
 	      DECL_EXTERNAL (decl) = 1;
 	      gen_decl_die (decl, context_die);
+	      if (dwarf2_called_from_lto_p)
+		DECL_IGNORED_P (decl) = saved_ignored_flag;
 	      DECL_EXTERNAL (decl) = saved_external_flag;
 	    }
 	  break;
@@ -13592,6 +13603,11 @@ force_decl_die (tree decl)
       if (!decl_die)
 	decl_die = lookup_decl_die (decl);
       gcc_assert (decl_die);
+      /* The LTO reading code checks the DW_AT_declaration attribute to
+	 decide whether to read the initializer for a variable.  So if
+	 this variable has an initial value, remove that attribute.  */
+      if (TREE_CODE (decl) == VAR_DECL && DECL_INITIAL (decl))
+	remove_AT (decl_die, DW_AT_declaration);
     }
 
   return decl_die;
Index: lto-function-out.c
===================================================================
--- lto-function-out.c	(revision 131119)
+++ lto-function-out.c	(working copy)
@@ -1420,7 +1420,7 @@ output_expr_operand (struct output_block
       break;
 
     case VAR_DECL:
-      if (DECL_CONTEXT (expr) == NULL_TREE)
+      if (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
 	{
 	  /* Static or extern VAR_DECLs.  */
 	  unsigned int index;
@@ -1830,6 +1830,8 @@ output_local_vars (struct output_block *
   tree t;
   int i = 0;
   struct output_stream *tmp_stream = ob->main_stream;
+  bitmap local_statics = BITMAP_ALLOC (NULL);
+
   ob->main_stream = ob->local_decl_stream;
 
   /* We have found MOST of the local vars by scanning the function.
@@ -1837,26 +1839,58 @@ output_local_vars (struct output_block *
      Other local vars can be found by walking the unexpanded vars
      list.  */
 
+  /* Need to put out the local statics first, to avoid the pointer
+     games used for the regular locals.  */
+  LTO_DEBUG_TOKEN ("local statics");
   for (t = fn->unexpanded_var_list; t; t = TREE_CHAIN (t))
     {
       tree lv = TREE_VALUE (t);
-      int j;
 
-      j = output_local_decl_ref (ob, lv, false);
-      /* Just for the fun of it, some of the locals are in the
-	 unexpanded_var_list more than once.  */
-      if (VEC_index (int, ob->unexpanded_local_decls_index, j) == -1)
-	VEC_replace (int, ob->unexpanded_local_decls_index, j, i++);
+      if (TREE_STATIC (lv))
+	{
+	  /* Do not put the static in the chain more than once, even
+	     if it was in the chain more than once to start.  */
+	  if (!bitmap_bit_p (local_statics, DECL_UID (lv)))
+	    {
+	      bitmap_set_bit (local_statics, DECL_UID (lv));
+	      output_expr_operand (ob, lv);
+	      if (DECL_CONTEXT (lv) == fn->decl)
+		{
+		  output_uleb128 (ob, 1); /* Restore context.  */
+		  if (DECL_INITIAL (lv))
+		    output_expr_operand (ob, DECL_INITIAL (lv));
+		  else 
+		    output_zero (ob); /* DECL_INITIAL.  */
+		}
+	      else 
+		{
+		  output_zero (ob); /* Restore context.  */
+		  output_zero (ob); /* DECL_INITIAL.  */
+		}
+	    }
+	}
+      else
+	{
+	  int j = output_local_decl_ref (ob, lv, false);
+	  /* Just for the fun of it, some of the locals are in the
+	     unexpanded_var_list more than once.  */
+	  if (VEC_index (int, ob->unexpanded_local_decls_index, j) == -1)
+	    VEC_replace (int, ob->unexpanded_local_decls_index, j, i++);
+	}
     }
 
+
+  /* End of statics.  */
+  output_zero (ob);
+  BITMAP_FREE (local_statics);
+
   /* The easiest way to get all of this stuff generated is to play
      pointer games with the streams and reuse the code for putting out
      the function bodies for putting out the local decls.  It needs to
      go into a separate stream because the LTO reader will want to
      process the local variables first, rather than have to back patch
-     them.
-  */
-
+     them.  */
+  LTO_DEBUG_TOKEN ("local vars");
   while (index < VEC_length (tree, ob->local_decls))
     output_local_var (ob, index++);
 
@@ -2389,6 +2423,7 @@ output_function (tree function)
   destroy_output_block (ob, true);
 
   current_function_decl = NULL;
+
   pop_cfun ();
 }
 
Index: lto/lto-read.c
===================================================================
--- lto/lto-read.c	(revision 131120)
+++ lto/lto-read.c	(working copy)
@@ -774,6 +774,10 @@ input_expr_operand (struct input_block *
 	  result = data_in->local_decls [lv_index];
 	  if (result == NULL)
 	    {
+	      /* Create a context to read the local variable so that
+		 it does not disturb the position of the code that is
+		 calling for the local variable.  This allows locals
+		 to refer to other locals.  */
 	      struct input_block lib;
 
 #ifdef LTO_STREAM_DEBUGGING
@@ -788,7 +792,6 @@ input_expr_operand (struct input_block *
 	      lto_debug_context.indent = 0;
 	      lto_debug_context.current_data = &debug;
 #endif
-
 	      lib.data = ib->data;
 	      lib.len = ib->len;
 	      lib.p = data_in->local_decls_index[lv_index];
@@ -1043,7 +1046,7 @@ input_expr_operand (struct input_block *
 	  {
 	    tree value;
 	    tree purpose;
-	    tree next;
+	    tree next = NULL;
 	    tree elt;
 	    enum LTO_tags tag = input_record_start (ib);
 
@@ -1392,12 +1395,40 @@ input_local_vars (struct input_block *ib
 		  struct function *fn, unsigned int count)
 {
   int i;
-  
+  unsigned int tag;
+
   data_in->unexpanded_indexes = xcalloc (count, sizeof (int));
   data_in->local_decls = xcalloc (count, sizeof (tree*));
 
   memset (data_in->unexpanded_indexes, -1, count * sizeof (int));
 
+  /* Recreate the unexpanded_var_list.  Put the statics at the end.*/
+  fn->unexpanded_var_list = NULL;
+  LTO_DEBUG_TOKEN ("local statics");
+  tag = input_record_start (ib);
+  
+  while (tag)
+    {
+      tree var = input_expr_operand (ib, data_in, fn, tag);
+      fn->unexpanded_var_list 
+	= tree_cons (NULL_TREE, var, fn->unexpanded_var_list);
+
+      if (input_uleb128 (ib))
+	DECL_CONTEXT (var) = fn->decl;
+	
+      /* DECL_INITIAL.  */
+      tag = input_record_start (ib);
+      if (tag)
+	DECL_INITIAL (var) = input_expr_operand (ib, data_in, fn, tag);
+
+      /* Statics never have external visibility.  */
+      DECL_EXTERNAL (var) = 0;
+
+      /* Next static.  */
+      tag = input_record_start (ib);
+    }
+
+  LTO_DEBUG_TOKEN ("local vars");
   for (i = 0; i < (int)count; i++)
     /* Some local decls may have already been read in if they are used
        as part of a previous local_decl.  */
@@ -1411,8 +1442,7 @@ input_local_vars (struct input_block *ib
 	input_local_var (ib, data_in, fn, i);
       }
 
-  /* Recreate the unexpanded_var_list.  */
-  fn->unexpanded_var_list = NULL;
+  /* Add the regular locals in the proper order.  */
   for (i = count - 1; i >= 0; i--)
     if (data_in->unexpanded_indexes[i] != -1)
       fn->unexpanded_var_list 

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2007-12-22 18:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-22 19:23 [lto] fix local statics Nathan Froyd

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