[vta, vta4.4] delay expansion of debug value exprs

[Alexandre Oliva <aoliva at redhat dot com>]

One of the problems I mentioned in the VTA guality assessment was that
some optimized-out variables that referenced addresses within a global
array had this value information lost during expand.

The problem was that the referenced array hadn't been referenced by
non-debug code at the point we expanded the debug insns.  We can't
go ahead and emit references to the symbol before we know it's actually
going to be emitted, so we refrained from emitting debug info.

Delaying the expansion of the debug value expressions until we've
expanded all the non-debug code in the function is likely to catch most
useful cases, replacing the hunting for CONST_STRINGs whose expansion
was sometimes postponed for similar reasons.

One bit of ugliness in the patch is that I temporarily store the value
expr (tree) in an rtx-typed field of the VAR_LOCATION.  I'm wondering if
it would be nicer to abuse the third field of the VAR_LOCATION (an
integral value) instead, to avoid crashes while dumping stuff.
Thoughts?

Regardless, I'm installing this patch in the VTA branches, and merging
it into the VTA patchset for the trunk.

for  gcc/ChangeLog.vta
from  Alexandre Oliva  <aoliva@redhat.com>

	* cfgexpand.c (adjust_debug_string_constant): Remove.
	(debug_string_constants_p): Remove.
	(adjust_debug_string_constants): Remove.
	(expand_debug_expr): Don't delay expansion of strings without
	constant pool entries.  Tweak DECL processing.  Fail more
	verbosely on unrecognized tree nodes, but only when checking
	is enabled.
	(expand_debug_locations): New, sort of extracted from...
	(expand_gimple_basic_block): ... here.
	(gimple_expand_cfg): Expand debug locations after expanding all
	basic blocks.  Don't adjust debug string constants any more.
	
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2009-06-16 00:51:21.000000000 -0300
+++ gcc/cfgexpand.c	2009-06-16 00:59:40.000000000 -0300
@@ -2068,58 +2068,6 @@ round_udiv_adjust (enum machine_mode mod
      const1_rtx, const0_rtx);
 }
 
-/* for_each_rtx function that adjusts VAR_LOCATION placeholders for
-   debug string constants.  */
-
-static int
-adjust_debug_string_constant (rtx *x, void *nothing ATTRIBUTE_UNUSED)
-{
-  rtx rtstr, cst;
-  tree exp;
-
-  if (GET_CODE (*x) != VAR_LOCATION)
-    return 0;
-
-  exp = PAT_VAR_LOCATION_DECL (*x);
-  rtstr = PAT_VAR_LOCATION_LOC (*x);
-
-  gcc_assert (TREE_CODE (exp) == STRING_CST && GET_CODE (rtstr) == CONST_STRING
-	      && TREE_STRING_POINTER (exp) == XSTR (rtstr, 0));
-
-  cst = lookup_constant_def (exp);
-  if (cst && MEM_P (cst))
-    *x = XEXP (cst, 0);
-  else
-    *x = rtstr;
-
-  return -1;
-}
-
-/* Set to true when a debug string constant is expanded as a
-   VAR_LOCATION placeholder rather than as regular rtl.  */
-
-static bool debug_string_constants_p = false;
-
-/* Adjust any debug string constants expanded as VAR_LOCATION
-   placeholders (see how STRING_CST is handled in expand_debug_expr)
-   to regular rtx or debug-only CONST_STRINGs.  */
-
-static void
-adjust_debug_string_constants (void)
-{
-  rtx insn;
-
-  if (!debug_string_constants_p)
-    return;
-
-  for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
-    if (DEBUG_INSN_P (insn))
-      for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
-		    adjust_debug_string_constant, NULL);
-
-  debug_string_constants_p = false;
-}
-
 /* Wrap modeless constants in CONST:MODE.  */
 rtx
 wrap_constant (enum machine_mode mode, rtx x)
@@ -2220,9 +2168,6 @@ expand_debug_expr (tree exp)
       if (!lookup_constant_def (exp))
 	{
 	  op0 = gen_rtx_CONST_STRING (Pmode, TREE_STRING_POINTER (exp));
-	  op0 = gen_rtx_VAR_LOCATION (Pmode, exp, op0, 0);
-	  op0 = gen_rtx_MEM (mode, op0);
-	  debug_string_constants_p = true;
 	  return op0;
 	}
       /* Fall through...  */
@@ -2247,11 +2192,13 @@ expand_debug_expr (tree exp)
     case LABEL_DECL:
     case CONST_DECL:
     case RESULT_DECL:
-      /* This decl was optimized away.  */
-      if (!DECL_RTL_SET_P (exp))
+      op0 = DECL_RTL_IF_SET (exp);
+
+      /* This decl was probably optimized away.  */
+      if (!op0)
 	return NULL;
 
-      op0 = DECL_RTL (exp);
+      op0 = copy_rtx (op0);
 
       if (GET_MODE (op0) == BLKmode)
 	{
@@ -2752,14 +2699,57 @@ expand_debug_expr (tree exp)
 
     default:
     flag_unsupported:
-#if 0
-      return NULL;
-#endif
+#ifdef ENABLE_CHECKING
       debug_tree (exp);
       gcc_unreachable ();
+#else
+      return NULL;
+#endif
     }
 }
 
+/* Expand the _LOCs in debug insns.  We run this after expanding all
+   regular insns, so that any variables referenced in the function
+   will have their DECL_RTLs set.  */
+
+static void
+expand_debug_locations (void)
+{
+  rtx insn;
+  rtx last = get_last_insn ();
+
+  for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
+    if (DEBUG_INSN_P (insn))
+      {
+	tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
+	rtx val;
+	enum machine_mode mode;
+
+	if (value == VAR_DEBUG_VALUE_NOVALUE)
+	  val = NULL_RTX;
+	else
+	  {
+	    val = expand_debug_expr (value);
+	    gcc_assert (last == get_last_insn ());
+	  }
+
+	if (!val)
+	  val = gen_rtx_UNKNOWN_VAR_LOC ();
+	else
+	  {
+	    mode = GET_MODE (INSN_VAR_LOCATION (insn));
+
+	    gcc_assert (mode == GET_MODE (val)
+			|| (GET_MODE (val) == VOIDmode
+			    && (GET_CODE (val) == CONST_INT
+				|| GET_CODE (val) == CONST_FIXED
+				|| GET_CODE (val) == CONST_DOUBLE)));
+	  }
+
+	INSN_VAR_LOCATION_LOC (insn) = val;
+      }
+}
+
 /* Expand basic block BB from GIMPLE trees to RTL.  */
 
 static basic_block
@@ -2874,11 +2864,6 @@ expand_gimple_basic_block (basic_block b
 
 	      last = get_last_insn ();
 
-	      if (value == VAR_DEBUG_VALUE_NOVALUE)
-		val = NULL_RTX;
-	      else
-		val = expand_debug_expr (value);
-
 	      set_curr_insn_source_location (gimple_location (stmt));
 	      set_curr_insn_block (gimple_block (stmt));
 
@@ -2887,27 +2872,18 @@ expand_gimple_basic_block (basic_block b
 	      else
 		mode = TYPE_MODE (TREE_TYPE (var));
 
-	      if (!val)
-		val = gen_rtx_UNKNOWN_VAR_LOC ();
-	      else
-		gcc_assert (mode == GET_MODE (val)
-			    || GET_CODE (val) == CONST_INT
-			    || GET_CODE (val) == CONST_FIXED
-			    || GET_CODE (val) == CONST_DOUBLE);
-
 	      val = gen_rtx_VAR_LOCATION
-		(mode, var, val, VAR_INIT_STATUS_INITIALIZED);
+		(mode, var, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
 
 	      val = emit_debug_insn (val);
 
-	      maybe_dump_rtl_for_gimple_stmt (stmt, last);
-
-	      if (last != PREV_INSN (val))
+	      if (dump_file && (dump_flags & TDF_DETAILS))
 		{
-		  debug_gimple_stmt (stmt);
-		  debug_rtx_range (NEXT_INSN (last), get_last_insn ());
-		  internal_error ("debug stmt expanded to too many insns");
-		  gcc_unreachable ();
+		  /* We can't dump the insn with a TREE where an RTX
+		     is expected.  */
+		  INSN_VAR_LOCATION_LOC (val) = const0_rtx;
+		  maybe_dump_rtl_for_gimple_stmt (stmt, last);
+		  INSN_VAR_LOCATION_LOC (val) = (rtx)value;
 		}
 
 	      gsi = nsi;
@@ -3388,6 +3364,9 @@ gimple_expand_cfg (void)
   FOR_BB_BETWEEN (bb, init_block->next_bb, EXIT_BLOCK_PTR, next_bb)
     bb = expand_gimple_basic_block (bb);
 
+  if (MAY_HAVE_DEBUG_INSNS)
+    expand_debug_locations ();
+
   execute_free_datastructures ();
   finish_out_of_ssa (&SA);
 
@@ -3396,8 +3375,6 @@ gimple_expand_cfg (void)
   pointer_map_destroy (lab_rtx_for_bb);
   free_histograms ();
 
-  adjust_debug_string_constants ();
-
   construct_exit_block ();
   set_curr_insn_block (DECL_INITIAL (current_function_decl));
   insn_locators_finalize ();

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer