Patchwork [PR48866] three alternative fixes

login
register
mail settings
Submitter Alexandre Oliva
Date June 3, 2011, 1:32 a.m.
Message ID <orvcwnn1hg.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/98506/
State New
Headers show

Comments

Alexandre Oliva - June 3, 2011, 1:32 a.m.
Ugh, failed to refresh the patch file, resending with the correct one.

On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 2. emit placeholders for replaceable DEFs and, when the DEFs are
>> expanded at their point of use, emit the expansion next to the
>> placeholder, rather than at the current stream.  The result of the
>> expansion is saved and used in debug insns that reference the
>> replaceable DEF.  If the result is forced into a REG shortly thereafter,
>> the code resulting from this is also emitted next to the placeholder,
>> and the saved expansion is updated.  If the USE is expanded before the
>> DEF, the insn stream resulting from the expansion is saved and emitted
>> at the point of the DEF.

> IMHO this is the riskiest of the 3 patches, for shuffling expansions
> around isn't exactly something I'm comfortable with.  There's a very
> real risk that moving the expansion of sub-expressions to their
> definition points may end up moving uses before definitions.

Upthread, I posted the wrong patch: instead of the one that tolerated
expanding DEFs before or after USEs, I posted a simplifying experiment
that seemed to fail, but it looks like I misinterpreted the results.

This revised and retested patch also records expansions in an array
rather than a pointer_map, and it avoids re-expanding DEFs when a USE is
expanded for the second time.  Although replaceable DEFs can only have
one USE, when the single USE appears in a call stmt, it can be expanded
twice.  I'm not sure whether it would be better to expand it twice and
let RTL optimizations drop any redundancies, or reuse the result of the
first expansion, like this patch does.

Patch

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

	PR debug/48866
	* cfgexpand.c (def_expansions): New.
	(def_expansion_recent_tree, def_expansion_recent_rtx): New.
	(def_expansions_init): New.
	(def_expansions_remove_placeholder, def_expansions_fini): New.
	(def_get_expansion_ptr): New.
	(def_expansion_recent, def_expansion_record_recent): New.
	(def_expansion_add_insns): New.
	(expand_debug_expr): Use recorded expansion if available.
	(expand_gimple_basic_block): Prepare to record expansion of
	replaceable defs.  Reset recent expansions at the end of the
	block.
	(gimple_expand_cfg): Initialize and finalize expansions cache.
	* expr.c: Include gimple-pretty-print.h.
	(store_expr): Forget recent expansions upon nontemporal moves.
	(expand_expr_real_1): Reuse or record expansion of replaceable
	defs.
	* expr.h (def_get_expansion_ptr, def_expansion_recent): Declare.
	(def_expansion_record_recent, def_expansion_add_insns): Declare.
	* explow.c (force_recent): New.
	(force_reg): Use it.  Split into...
	(force_reg_1): ... this.
	* Makefile.in (expr.o): Depend on gimple-pretty-print.h.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2011-06-02 16:43:03.596818720 -0300
+++ gcc/cfgexpand.c	2011-06-02 17:18:10.217974612 -0300
@@ -2337,6 +2337,144 @@  convert_debug_memory_address (enum machi
   return x;
 }
 
+/* Map replaceable SSA_NAMEs to NOTE_INSN_VAR_LOCATIONs that hold
+   their RTL expansions (once available) in their NOTE_VAR_LOCATIONs
+   (without a VAR_LOCATION rtx).  The SSA_NAME DEF is expanded before
+   its single USE, so the NOTE is inserted in the insn stream, marking
+   the location where the non-replaceable portion of the expansion is
+   to be inserted.  When the single USE is expanded, it will be
+   emitted before the NOTE.  */
+static rtx *def_expansions;
+
+/* The latest expanded SSA name, and its corresponding RTL expansion.
+   These are used to enable the insertion of the insn that stores the
+   expansion in a register at the end of the sequence expanded for the
+   SSA DEF.  */
+static tree def_expansion_recent_tree;
+static rtx def_expansion_recent_rtx;
+
+/* Initialize the def_expansions data structure.  This is to be called
+   before expansion of a function starts.  */
+
+static void
+def_expansions_init (void)
+{
+  gcc_checking_assert (!def_expansions);
+  def_expansions = XCNEWVEC (rtx, num_ssa_names);
+
+  gcc_checking_assert (!def_expansion_recent_tree);
+  gcc_checking_assert (!def_expansion_recent_rtx);
+}
+
+/* Remove the NOTE that marks the insertion location of the expansion
+   of a replaceable SSA note.  */
+
+static bool
+def_expansions_remove_placeholder (rtx note)
+{
+  if (!note)
+    return true;
+
+  gcc_checking_assert (NOTE_P (note));
+  remove_insn (note);
+
+  return true;
+}
+
+/* Finalize the def_expansions data structure.  This is to be called
+   at the end of the expansion of a function.  */
+
+static void
+def_expansions_fini (void)
+{
+  int i = num_ssa_names;
+
+  gcc_checking_assert (def_expansions);
+
+  while (i--)
+    if (def_expansions[i])
+      def_expansions_remove_placeholder (def_expansions[i]);
+  XDELETEVEC (def_expansions);
+  def_expansions = NULL;
+  def_expansion_recent_tree = NULL;
+  def_expansion_recent_rtx = NULL;
+}
+
+/* Return a pointer to the NOTE marking the insertion point for the
+   expansion of EXP.  EXP must be a replaceable SSA_NAME.  */
+
+rtx *
+def_get_expansion_ptr (tree exp)
+{
+  gcc_checking_assert (def_expansions);
+  gcc_checking_assert (TREE_CODE (exp) == SSA_NAME);
+  gcc_checking_assert (bitmap_bit_p (SA.values, SSA_NAME_VERSION (exp)));
+  return &def_expansions[SSA_NAME_VERSION (exp)];
+}
+
+/* Return an SSA name, if any, that was recently expanded to the value
+   X.  */
+
+tree
+def_expansion_recent (rtx x)
+{
+  if (!def_expansion_recent_rtx)
+    return NULL;
+
+  if (x == def_expansion_recent_rtx
+      || rtx_equal_p (x, def_expansion_recent_rtx))
+    return def_expansion_recent_tree;
+
+  return NULL;
+}
+
+/* Record that DEF was recently expanded to the value X.  */
+
+void
+def_expansion_record_recent (tree def, rtx x)
+{
+  def_expansion_recent_tree = def;
+  def_expansion_recent_rtx = x;
+}
+
+/* Forget recent expansions.  */
+
+void
+def_expansion_reset_recent (void)
+{
+  def_expansion_record_recent (NULL, NULL);
+}
+
+/* Add INSNS to the insn seq generated for DEF, and update the
+   RTL value of DEF to VAL.  */
+
+void
+def_expansion_add_insns (tree def, rtx insns, rtx val)
+{
+  rtx note = *def_get_expansion_ptr (def);
+
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      fprintf (dump_file, "\n;; (cont) ");
+      print_gimple_stmt (dump_file, SSA_NAME_DEF_STMT (def), 0,
+			 TDF_SLIM | (dump_flags & TDF_LINENO));
+      fprintf (dump_file, "\n");
+
+      print_rtl (dump_file, insns);
+
+      fprintf (dump_file, "\n=> ");
+      print_rtl (dump_file, val);
+    }
+
+  gcc_checking_assert (note);
+  emit_insn_before (insns, note);
+  gcc_checking_assert (NOTE_VAR_LOCATION (note));
+  NOTE_VAR_LOCATION (note) = val;
+
+  def_expansion_recent_tree = NULL;
+  def_expansion_recent_rtx = NULL;
+}
+
 /* Return an RTX equivalent to the value of the tree expression
    EXP.  */
 
@@ -3131,7 +3269,20 @@  expand_debug_expr (tree exp)
 	gimple g = get_gimple_for_ssa_name (exp);
 	if (g)
 	  {
-	    op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+	    rtx *xp = def_get_expansion_ptr (exp);
+
+	    if (xp)
+	      {
+		rtx note = *xp;
+		gcc_checking_assert (NOTE_P (note));
+		op0 = NOTE_VAR_LOCATION (note);
+	      }
+	    else
+	      op0 = NULL;
+
+	    if (!op0)
+	      op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
+
 	    if (!op0)
 	      return NULL;
 	  }
@@ -3621,22 +3772,34 @@  expand_gimple_basic_block (basic_block b
 	      def_operand_p def_p;
 	      def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
 
-	      if (def_p != NULL)
+	      /* Ignore this stmt if it is in the list of
+		 replaceable expressions.  */
+	      if (def_p != NULL
+		  && SA.values
+		  && bitmap_bit_p (SA.values,
+				   SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
 		{
-		  /* Ignore this stmt if it is in the list of
-		     replaceable expressions.  */
-		  if (SA.values
-		      && bitmap_bit_p (SA.values,
-				       SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
-		    continue;
+		  tree def = DEF_FROM_PTR (def_p);
+		  rtx *xp = def_get_expansion_ptr (def);
+		  rtx note;
+
+		  last = get_last_insn ();
+
+		  gcc_checking_assert (!*xp);
+
+		  note = emit_note (NOTE_INSN_VAR_LOCATION);
+		  NOTE_VAR_LOCATION (note) = NULL;
+		  *xp = note;
 		}
-	      last = expand_gimple_stmt (stmt);
+	      else
+		last = expand_gimple_stmt (stmt);
 	      maybe_dump_rtl_for_gimple_stmt (stmt, last);
 	    }
 	}
     }
 
   currently_expanding_gimple_stmt = NULL;
+  def_expansion_reset_recent ();
 
   /* Expand implicit goto and convert goto_locus.  */
   FOR_EACH_EDGE (e, ei, bb->succs)
@@ -4112,11 +4275,14 @@  gimple_expand_cfg (void)
     e->flags &= ~EDGE_EXECUTABLE;
 
   lab_rtx_for_bb = pointer_map_create ();
+  def_expansions_init ();
+
   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 ();
+  def_expansions_fini ();
 
   execute_free_datastructures ();
   timevar_push (TV_OUT_OF_SSA);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig	2011-06-02 02:16:06.027421293 -0300
+++ gcc/expr.c	2011-06-02 18:15:47.941701093 -0300
@@ -48,6 +48,7 @@  along with GCC; see the file COPYING3.  
 #include "tree-iterator.h"
 #include "tree-pass.h"
 #include "tree-flow.h"
+#include "gimple-pretty-print.h"
 #include "target.h"
 #include "timevar.h"
 #include "df.h"
@@ -4675,6 +4676,9 @@  store_expr (tree exp, rtx target, int ca
 			       (call_param_p
 				? EXPAND_STACK_PARM : EXPAND_NORMAL),
 			       &alt_rtl);
+      /* Don't risk moving loads before stores.  */
+      if (!nontemporal)
+	def_expansion_reset_recent ();
     }
 
   /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
@@ -8424,10 +8428,63 @@  expand_expr_real_1 (tree exp, rtx target
 	  && !SSA_NAME_IS_DEFAULT_DEF (exp)
 	  && (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
 	  && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
-	g = SSA_NAME_DEF_STMT (exp);
+	{
+	  g = SSA_NAME_DEF_STMT (exp);
+	  if (g)
+	    return expand_expr_real (gimple_assign_rhs_to_tree (g),
+				     target, tmode, modifier, NULL);
+	}
       if (g)
-	return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
-				 modifier, NULL);
+	{
+	  rtx retval;
+	  rtx insns;
+	  rtx note = *def_get_expansion_ptr (exp);
+
+	  gcc_assert (NOTE_P (note));
+	  /* Call parameters may be expanded twice.  Reuse the result
+	     of the first expansion.  */
+	  if (NOTE_VAR_LOCATION (note))
+	    return NOTE_VAR_LOCATION (note);
+
+	  start_sequence ();
+
+	  retval = expand_expr_real (gimple_assign_rhs_to_tree (g),
+				     target, tmode, modifier, NULL);
+
+	  insns = get_insns ();
+
+	  if (retval == target)
+	    {
+	      end_sequence ();
+	      emit_insn (insns);
+	      return retval;
+	    }
+
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    {
+	      fprintf (dump_file, "\n;; ");
+	      print_gimple_stmt (dump_file, g, 0,
+				 TDF_SLIM | (dump_flags & TDF_LINENO));
+	      fprintf (dump_file, "\n");
+
+	      print_rtl (dump_file, insns);
+
+	      fprintf (dump_file, "\n=> ");
+	      print_rtl (dump_file, retval);
+	    }
+
+	  end_sequence ();
+
+	  /* Make sure this is in the instruction stream.  */
+	  gcc_checking_assert (PREV_INSN (note));
+	  emit_insn_before (insns, note);
+	  gcc_checking_assert (!NOTE_VAR_LOCATION (note));
+
+	  NOTE_VAR_LOCATION (note) = retval;
+	  def_expansion_record_recent (exp, retval);
+
+	  return retval;
+	}
 
       ssa_name = exp;
       decl_rtl = get_rtx_for_ssa_name (ssa_name);
Index: gcc/expr.h
===================================================================
--- gcc/expr.h.orig	2011-06-02 02:16:06.332420537 -0300
+++ gcc/expr.h	2011-06-02 16:43:13.254766180 -0300
@@ -693,4 +693,11 @@  extern tree build_libfunc_function (cons
 /* Get the personality libfunc for a function decl.  */
 rtx get_personality_function (tree);
 
+/* In cfgexpand.c.  */
+rtx *def_get_expansion_ptr (tree);
+tree def_expansion_recent (rtx);
+void def_expansion_record_recent (tree, rtx);
+void def_expansion_reset_recent (void);
+void def_expansion_add_insns (tree, rtx, rtx);
+
 #endif /* GCC_EXPR_H */
Index: gcc/explow.c
===================================================================
--- gcc/explow.c.orig	2011-06-01 20:39:58.896953301 -0300
+++ gcc/explow.c	2011-06-02 16:43:13.352765648 -0300
@@ -638,22 +638,62 @@  copy_to_mode_reg (enum machine_mode mode
   return temp;
 }
 
+/* If X is the value of a recently-expanded SSA def, emit the insns
+   generated by FN (MODE, X) at the end of the expansion of that def,
+   otherwise emit them in the current insn seq.  */
+
+static inline rtx
+force_recent (enum machine_mode mode, rtx x,
+	      rtx (*fn) (enum machine_mode, rtx))
+{
+  tree def = def_expansion_recent (x);
+  rtx retval;
+  rtx insns;
+
+  if (!def)
+    return fn (mode, x);
+
+  start_sequence ();
+  retval = fn (mode, x);
+  insns = get_insns ();
+  end_sequence ();
+
+  def_expansion_add_insns (def, insns, retval);
+
+  return retval;
+}
+
+static rtx force_reg_1 (enum machine_mode, rtx);
+
 /* Load X into a register if it is not already one.
    Use mode MODE for the register.
    X should be valid for mode MODE, but it may be a constant which
    is valid for all integer modes; that's why caller must specify MODE.
 
+   If X is the value of a recent SSA def expansion, emit the insns at
+   the end of that expansion, rather than into the current seq.
+
    The caller must not alter the value in the register we return,
    since we mark it as a "constant" register.  */
 
 rtx
 force_reg (enum machine_mode mode, rtx x)
 {
-  rtx temp, insn, set;
-
   if (REG_P (x))
     return x;
 
+  return force_recent (mode, x, force_reg_1);
+}
+
+/* Load X into a register if it is not already one.
+   Use mode MODE for the register.
+   Internal implementation of force_reg.  */
+
+static rtx
+force_reg_1 (enum machine_mode mode, rtx x)
+{
+  rtx temp, insn, set;
+
   if (general_operand (x, mode))
     {
       temp = gen_reg_rtx (mode);
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig	2011-06-01 20:39:59.074953270 -0300
+++ gcc/Makefile.in	2011-06-02 16:43:13.484764930 -0300
@@ -2942,7 +2942,8 @@  except.o : except.c $(CONFIG_H) $(SYSTEM
 expr.o : expr.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
    $(TREE_H) $(FLAGS_H) $(FUNCTION_H) $(REGS_H) $(EXPR_H) $(OPTABS_H) \
    $(LIBFUNCS_H) $(INSN_ATTR_H) insn-config.h $(RECOG_H) output.h \
-   typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) hard-reg-set.h $(EXCEPT_H) \
+   typeclass.h hard-reg-set.h toplev.h $(DIAGNOSTIC_CORE_H) \
+   gimple-pretty-print.h hard-reg-set.h $(EXCEPT_H) \
    reload.h langhooks.h intl.h $(TM_P_H) $(TARGET_H) \
    tree-iterator.h gt-expr.h $(MACHMODE_H) $(TIMEVAR_H) $(TREE_FLOW_H) \
    $(TREE_PASS_H) $(DF_H) $(DIAGNOSTIC_H) vecprim.h $(SSAEXPAND_H)