Use simplify_replace_rtx rather than wrap_constant

Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Combine and reload need to substitute into debug_insns.  They currently
do this by storing the "to" value at the location of the "from" value
while leaving the enclosing rtx as-is.  The one exception is that they
handle subregs of the "from" value specially.

However, doing a substitution this way can easily create an invalid rtx.
One particular case that we've already hit is that replacing a
register with a CONST_INT doesn't work inside unary operators
like ZERO_EXTEND and SIGN_EXTEND.  The VTA code gets around this
by using wrap_constant to convert the CONST_INT into a
(const (const_int ...)).

I've whined about that a few times recently.  One of the big advantages
of the VTA approach was that it used the same rvalue IR for both "debug"
insns and "real" insns.  I think using wrap_constant is a severe
deviation from that goal.  We're introducing hitherto invalid CONSTs at
arbitrary places within a var location (not just at the top level rtx),
and many generic rtx routines just aren't prepared to deal with that.
Jakub said that it should just be simplify-rtx.c, var-tracking.c and
consumers of cselib that need to handle them, but I think it goes much
wider than that.  E.g. if reload1.c replaces:

    (plus (reg x) (reg y))

with:

    (plus (reg x) (const (const_int C)))

(which it can easily do) then is that supposed to be rtx_equal_p to:

    (plus (reg x) (const_int C))

or not?  If not, when is the result of the reload1.c substitution
supposed to be canonicalised?  And why leave it in a non-canonical
state until then?

The same question goes if we end up making a substitution that
allows an enclosing rtx to be folded to a constant.  Obviously we
don't want that to happen too often (it would probably indicate
an earlier optimisation failure) but it is still possible.

Besides, losing modes is just one example of where this kind of
substitution can go wrong.  We could also end up creating other
kinds of invalid rtx (or at least non-canonical rtx, which IMO
amounts to the same thing).  E.g. the reload1.c substitution
can replace REGs with PLUS elimination targets, which could
lead to an enclosing commutative rtx having its operands
the wrong way around.

debug_insns can't rely on things like recog or constraints to help
detect some types of invalid change, so I think we should simply say that
any valid rvalue is also a valid var location.  We should then call the
generic function for doing a substitution on an arbitrary rvalue, namely
simplify_replace_rtx.  That's what this patch does.

The combine substitution is an interesting case because, on AUTO_INC_DEC
targets, it creates the "to" value lazily on the first substitution.
That seems like a reasonable thing to want to do, so I generalised
simplify_replace_rtx to allow it.

I made a couple of other changes to simplify_replace_rtx too:

  - It now copies the "to" rtx for each replacement.  All but one caller
    seemed to pass an already-used rtx as the "to" value, so current
    substitutions could create invalid sharing.  I've removed the
    now-redundant copy_rtx from the one caller that used it.

    (Doing the copy lazily seems better, and copes with cases where the
    value is used twice within the same expression.)

  - It now uses rtx_equal_p to check for equivalence.  Some callers seem
    to pass MEM rtxes, which can't be shared, so I think using rtx_equal_p
    is better than relying on pointer equivalence for everything except REGs.
    I scanned the current callers, and none of them seemed to be using
    the function in a way that genuinely required strict pointer
    equivalence.

    rtx_equal_p isn't cheap of course.  We could reduce the overhead
    by turning it into an inline wrapper that first checks whether the
    code and mode are the same (or maybe just the code) and only then
    calls an out-of-line function.

I did wonder about using a generic for_each_rtx-like callback that
could match the "from" rtx in other ways besides rtx_equal_p,
and provide an arbitrary "to" rtx.  However, making an indirect call
for every node carries a fair bit of overhead, so I think it'd be
better to leave things be until we find we need something so general.

The reload1.c loop was structured like this:

 	  for (use = DF_REG_USE_CHAIN (i); use; use = next)
 	    {
              ...
 	      insn = DF_REF_INSN (use);
 	      next = DF_REF_NEXT_REG (use);

which was presumably to cope with the case where we replace the location
with an UNKNOWN_VAR_LOC and call df_insn_rescan_debug_internal.  But if
that's the case, then we should skip all consecutive uses of the register
in INSN, not just the current one.  Doing this in the new code also
avoids a redundant substitution in cases where the insn uses the
register more than once.

TBH, I'm not sure why we needed the other new calls to wrap_constant.
There are no comments to justify them, and no-one's explained them
on the list.  I also couldn't get anything to regress when I removed them.
As mentioned before, things based around cselib_expand_value_rtx
(and similar functions) should always know the mode of the value they're
dealing with; the convention is to pass the mode separately if it isn't
obvious.  So if there are cases where we're losing track of the mode somehow,
I think that needs to be fixed.  And if these calls are instead due to
substitutions like the combine.c and reload1.c ones, then I think we
should be doing the substitution differently.

The patch reinstates cselib's original static wrap_constant, so that
we don't accidentally introduce more uses.  Since the wrapped constants
are entirely local to the hash-table lookup, and aren't even entered
into the hash table, I'm tempted to pass the lookup function an
(on-stack) "rtx, enum machine_mode" pair instead of a plain rtx,
and thus avoid the need to create a throw-away CONST.  I'll send a
follow-on patch for that if this one is OK, and if no-one tells me
it's a bad idea.

I've also removed unwrap_constant, which was added by VTA,
but isn't used.

Finally, I've removed the CONST handling in simplify_unary, even though
it predated VTA.  The code is at odds with the other simplification
routines and isn't explained.  I can't see any reason why we'd want
to lose the knowledge that a constant is a constant.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also
regression-tested on arm-eabi (for the AUTO_INC_DEC stuff)
and tested against GDB on x86_64-linux-gnu.  OK to install?

I wouldn't be entirely surprised if this patch introduces some post-VTA
regressions that weren't picked up by the usual testing.  But if it does,
I think that's actually a useful thing, because we'll then be able to
pinpoint the places that are doing invalid substitutions, losing the mode,
or whatever.  And if it doesn't introduce any regressions, well, even better!

Richard


gcc/
	* rtl.h (simplify_replace_fn_rtx): Declare.
	(wrap_constant, unwrap_constant): Delete.
	* cfgexpand.c (unwrap_constant, wrap_constant): Delete.
	(expand_debug_expr): Don't call wrap_constant.
	* combine.c (rtx_subst_pair): Only define for AUTO_INC_DEC.
	(auto_adjust_pair): Fold into...
	(propagate_for_debug_subst): ...here.  Only define for AUTO_INC_DEC.
	Just return a new value.
	(propagate_for_debug): Use simplify_replace_fn_rtx for AUTO_INC_DEC,
	otherwise use simplify_replace_rtx.
	* cselib.c (wrap_constant): Reinstate old definition.
	(cselib_expand_value_rtx_1): Don't wrap constants.
	* reload1.c (reload): Skip all uses for an insn before adjusting it.
	Use simplify_replace_rtx.
	* simplify-rtx.c (simplify_replace_fn_rtx): New function, adapted from...
	(simplify_replace_rtx): ...here.  Turn into a wrapper for
	simplify_replace_fn_rtx.
	(simplify_unary_operation): Don't unwrap CONSTs.
	* var-tracking.c (check_wrap_constant): Delete.
	(vt_expand_loc_callback): Don't call it.
	(vt_expand_loc): Likewise.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2009-09-26 10:43:09.000000000 +0100
+++ gcc/rtl.h	2009-09-26 12:12:29.000000000 +0100
@@ -1765,6 +1765,8 @@ extern rtx simplify_subreg (enum machine
 			    unsigned int);
 extern rtx simplify_gen_subreg (enum machine_mode, rtx, enum machine_mode,
 				unsigned int);
+extern rtx simplify_replace_fn_rtx (rtx, const_rtx,
+				    rtx (*fn) (void *), void *);
 extern rtx simplify_replace_rtx (rtx, const_rtx, rtx);
 extern rtx simplify_rtx (const_rtx);
 extern rtx avoid_constant_pool_reference (rtx);
@@ -2404,8 +2406,6 @@ extern void invert_br_probabilities (rtx
 extern bool expensive_function_p (int);
 /* In cfgexpand.c */
 extern void add_reg_br_prob_note (rtx last, int probability);
-extern rtx wrap_constant (enum machine_mode, rtx);
-extern rtx unwrap_constant (rtx);
 
 /* In var-tracking.c */
 extern unsigned int variable_tracking_main (void);
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	2009-09-26 10:43:09.000000000 +0100
+++ gcc/cfgexpand.c	2009-09-26 12:12:29.000000000 +0100
@@ -2194,46 +2194,6 @@ round_udiv_adjust (enum machine_mode mod
      const1_rtx, const0_rtx);
 }
 
-/* Wrap modeless constants in CONST:MODE.  */
-rtx
-wrap_constant (enum machine_mode mode, rtx x)
-{
-  if (GET_MODE (x) != VOIDmode)
-    return x;
-
-  if (CONST_INT_P (x)
-      || GET_CODE (x) == CONST_FIXED
-      || GET_CODE (x) == CONST_DOUBLE
-      || GET_CODE (x) == LABEL_REF)
-    {
-      gcc_assert (mode != VOIDmode);
-
-      x = gen_rtx_CONST (mode, x);
-    }
-
-  return x;
-}
-
-/* Remove CONST wrapper added by wrap_constant().  */
-rtx
-unwrap_constant (rtx x)
-{
-  rtx ret = x;
-
-  if (GET_CODE (x) != CONST)
-    return x;
-
-  x = XEXP (x, 0);
-
-  if (CONST_INT_P (x)
-      || GET_CODE (x) == CONST_FIXED
-      || GET_CODE (x) == CONST_DOUBLE
-      || GET_CODE (x) == LABEL_REF)
-    ret = x;
-
-  return ret;
-}
-
 /* Convert X to MODE, that must be Pmode or ptr_mode, without emitting
    any rtl.  */
 
@@ -2353,9 +2313,7 @@ expand_debug_expr (tree exp)
     case COMPLEX_CST:
       gcc_assert (COMPLEX_MODE_P (mode));
       op0 = expand_debug_expr (TREE_REALPART (exp));
-      op0 = wrap_constant (GET_MODE_INNER (mode), op0);
       op1 = expand_debug_expr (TREE_IMAGPART (exp));
-      op1 = wrap_constant (GET_MODE_INNER (mode), op1);
       return gen_rtx_CONCAT (mode, op0, op1);
 
     case VAR_DECL:
Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2009-09-26 10:43:09.000000000 +0100
+++ gcc/combine.c	2009-09-26 12:12:29.000000000 +0100
@@ -2257,68 +2257,33 @@ cleanup_auto_inc_dec (rtx src, bool afte
 
   return x;
 }
-#endif
 
 /* Auxiliary data structure for propagate_for_debug_stmt.  */
 
 struct rtx_subst_pair
 {
-  rtx from, to;
-  bool changed;
-#ifdef AUTO_INC_DEC
+  rtx to;
   bool adjusted;
   bool after;
-#endif
 };
 
-/* Clean up any auto-updates in PAIR->to the first time it is called
-   for a PAIR.  PAIR->adjusted is used to tell whether we've cleaned
-   up before.  */
+/* DATA points to an rtx_subst_pair.  Return the value that should be
+   substituted.  */
 
-static void
-auto_adjust_pair (struct rtx_subst_pair *pair ATTRIBUTE_UNUSED)
+static rtx
+propagate_for_debug_subst (void *data)
 {
-#ifdef AUTO_INC_DEC
+  struct rtx_subst_pair *pair = (struct rtx_subst_pair *)data;
+
   if (!pair->adjusted)
     {
       pair->adjusted = true;
       pair->to = cleanup_auto_inc_dec (pair->to, pair->after, VOIDmode);
+      return pair->to;
     }
-#endif
-}
-
-/* If *LOC is the same as FROM in the struct rtx_subst_pair passed as
-   DATA, replace it with a copy of TO.  Handle SUBREGs of *LOC as
-   well.  */
-
-static int
-propagate_for_debug_subst (rtx *loc, void *data)
-{
-  struct rtx_subst_pair *pair = (struct rtx_subst_pair *)data;
-  rtx from = pair->from, to = pair->to;
-  rtx x = *loc, s = x;
-
-  if (rtx_equal_p (x, from)
-      || (GET_CODE (x) == SUBREG && rtx_equal_p ((s = SUBREG_REG (x)), from)))
-    {
-      auto_adjust_pair (pair);
-      if (pair->to != to)
-	to = pair->to;
-      else
-	to = copy_rtx (to);
-      if (s != x)
-	{
-	  gcc_assert (GET_CODE (x) == SUBREG && SUBREG_REG (x) == s);
-	  to = simplify_gen_subreg (GET_MODE (x), to,
-				    GET_MODE (from), SUBREG_BYTE (x));
-	}
-      *loc = wrap_constant (GET_MODE (x), to);
-      pair->changed = true;
-      return -1;
-    }
-
-  return 0;
+  return copy_rtx (pair->to);
 }
+#endif
 
 /* Replace occurrences of DEST with SRC in DEBUG_INSNs between INSN
    and LAST.  If MOVE holds, debug insns must also be moved past
@@ -2327,14 +2292,11 @@ propagate_for_debug_subst (rtx *loc, voi
 static void
 propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src, bool move)
 {
-  struct rtx_subst_pair p;
-  rtx next, move_pos = move ? last : NULL_RTX;
-
-  p.from = dest;
-  p.to = src;
-  p.changed = false;
+  rtx next, move_pos = move ? last : NULL_RTX, loc;
 
 #ifdef AUTO_INC_DEC
+  struct rtx_subst_pair p;
+  p.to = src;
   p.adjusted = false;
   p.after = move;
 #endif
@@ -2346,11 +2308,15 @@ propagate_for_debug (rtx insn, rtx last,
       next = NEXT_INSN (insn);
       if (DEBUG_INSN_P (insn))
 	{
-	  for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
-			propagate_for_debug_subst, &p);
-	  if (!p.changed)
+#ifdef AUTO_INC_DEC
+	  loc = simplify_replace_fn_rtx (INSN_VAR_LOCATION_LOC (insn),
+					 dest, propagate_for_debug_subst, &p);
+#else
+	  loc = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn), dest, src);
+#endif
+	  if (loc == INSN_VAR_LOCATION_LOC (insn))
 	    continue;
-	  p.changed = false;
+	  INSN_VAR_LOCATION_LOC (insn) = loc;
 	  if (move_pos)
 	    {
 	      remove_insn (insn);
Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c	2009-09-26 10:43:09.000000000 +0100
+++ gcc/cselib.c	2009-09-26 12:12:29.000000000 +0100
@@ -661,6 +661,19 @@ rtx_equal_for_cselib_p (rtx x, rtx y)
   return 1;
 }
 
+/* We need to pass down the mode of constants through the hash table
+   functions.  For that purpose, wrap them in a CONST of the appropriate
+   mode.  */
+static rtx
+wrap_constant (enum machine_mode mode, rtx x)
+{
+  if (!CONST_INT_P (x) && GET_CODE (x) != CONST_FIXED
+      && (GET_CODE (x) != CONST_DOUBLE || GET_MODE (x) != VOIDmode))
+    return x;
+  gcc_assert (mode != VOIDmode);
+  return gen_rtx_CONST (mode, x);
+}
+
 /* Hash an rtx.  Return 0 if we couldn't hash the rtx.
    For registers and memory locations, we look up their cselib_val structure
    and return its VALUE element.
@@ -1327,21 +1340,9 @@ cselib_expand_value_rtx_1 (rtx orig, str
     default:
       break;
     }
-  if (scopy == NULL_RTX)
-    {
-      XEXP (copy, 0)
-	= gen_rtx_CONST (GET_MODE (XEXP (orig, 0)), XEXP (copy, 0));
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	fprintf (dump_file, "  wrapping const_int result in const to preserve mode %s\n",
-		 GET_MODE_NAME (GET_MODE (XEXP (copy, 0))));
-    }
   scopy = simplify_rtx (copy);
   if (scopy)
-    {
-      if (GET_MODE (copy) != GET_MODE (scopy))
-	scopy = wrap_constant (GET_MODE (copy), scopy);
-      return scopy;
-    }
+    return scopy;
   return copy;
 }
 
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	2009-09-26 10:43:09.000000000 +0100
+++ gcc/reload1.c	2009-09-26 12:12:29.000000000 +0100
@@ -1257,36 +1257,25 @@ reload (rtx first, int global)
 
 	  for (use = DF_REG_USE_CHAIN (i); use; use = next)
 	    {
-	      rtx *loc = DF_REF_LOC (use);
-	      rtx x = *loc;
-
 	      insn = DF_REF_INSN (use);
+
+	      /* Make sure the next ref is for a different instruction,
+		 so that we're not affected by the rescan.  */
 	      next = DF_REF_NEXT_REG (use);
+	      while (next && DF_REF_INSN (next) == insn)
+		next = DF_REF_NEXT_REG (next);
 
 	      if (DEBUG_INSN_P (insn))
 		{
-		  gcc_assert (x == reg
-			      || (GET_CODE (x) == SUBREG
-				  && SUBREG_REG (x) == reg));
-
 		  if (!equiv)
 		    {
 		      INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
 		      df_insn_rescan_debug_internal (insn);
 		    }
 		  else
-		    {
-		      if (x == reg)
-			*loc = copy_rtx (equiv);
-		      else if (GET_CODE (x) == SUBREG
-			       && SUBREG_REG (x) == reg)
-			*loc = simplify_gen_subreg (GET_MODE (x), equiv,
-						    GET_MODE (reg),
-						    SUBREG_BYTE (x));
-		      else
-			gcc_unreachable ();
-		    *loc = wrap_constant (GET_MODE (x), *loc);
-		    }
+		    INSN_VAR_LOCATION_LOC (insn)
+		      = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn),
+					      reg, equiv);
 		}
 	    }
 	}
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2009-09-26 10:43:09.000000000 +0100
+++ gcc/simplify-rtx.c	2009-09-26 13:17:07.000000000 +0100
@@ -350,38 +350,45 @@ simplify_gen_relational (enum rtx_code c
   return gen_rtx_fmt_ee (code, mode, op0, op1);
 }
 \f
-/* Replace all occurrences of OLD_RTX in X with NEW_RTX and try to simplify the
-   resulting RTX.  Return a new RTX which is as simplified as possible.  */
+/* Replace all occurrences of OLD_RTX in X with FN (DATA).  Treat FN as
+   copy_rtx if it is null.  Canonicalize and simplify the result.  */
 
 rtx
-simplify_replace_rtx (rtx x, const_rtx old_rtx, rtx new_rtx)
+simplify_replace_fn_rtx (rtx x, const_rtx old_rtx,
+			 rtx (*fn) (void *), void *data)
 {
   enum rtx_code code = GET_CODE (x);
   enum machine_mode mode = GET_MODE (x);
   enum machine_mode op_mode;
   rtx op0, op1, op2;
 
-  /* If X is OLD_RTX, return NEW_RTX.  Otherwise, if this is an expression, try
-     to build a new expression substituting recursively.  If we can't do
-     anything, return our input.  */
+  /* If X is OLD_RTX, return FN (DATA), with a null FN equating to
+     copy_rtx.  Otherwise, if this is an expression, try to build a
+     new expression, substituting recursively.  If we can't do anything,
+     return our input.  */
 
-  if (x == old_rtx)
-    return new_rtx;
+  if (rtx_equal_p (x, old_rtx))
+    {
+      if (fn)
+	return fn (data);
+      else
+	return copy_rtx ((rtx) data);
+    }
 
   switch (GET_RTX_CLASS (code))
     {
     case RTX_UNARY:
       op0 = XEXP (x, 0);
       op_mode = GET_MODE (op0);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
       if (op0 == XEXP (x, 0))
 	return x;
       return simplify_gen_unary (code, mode, op0, op_mode);
 
     case RTX_BIN_ARITH:
     case RTX_COMM_ARITH:
-      op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1))
 	return x;
       return simplify_gen_binary (code, mode, op0, op1);
@@ -391,8 +398,8 @@ simplify_replace_rtx (rtx x, const_rtx o
       op0 = XEXP (x, 0);
       op1 = XEXP (x, 1);
       op_mode = GET_MODE (op0) != VOIDmode ? GET_MODE (op0) : GET_MODE (op1);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (op1, old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (op1, old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1))
 	return x;
       return simplify_gen_relational (code, mode, op_mode, op0, op1);
@@ -401,9 +408,9 @@ simplify_replace_rtx (rtx x, const_rtx o
     case RTX_BITFIELD_OPS:
       op0 = XEXP (x, 0);
       op_mode = GET_MODE (op0);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
-      op2 = simplify_replace_rtx (XEXP (x, 2), old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
+      op2 = simplify_replace_fn_rtx (XEXP (x, 2), old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1) && op2 == XEXP (x, 2))
 	return x;
       if (op_mode == VOIDmode)
@@ -414,7 +421,7 @@ simplify_replace_rtx (rtx x, const_rtx o
       /* The only case we try to handle is a SUBREG.  */
       if (code == SUBREG)
 	{
-	  op0 = simplify_replace_rtx (SUBREG_REG (x), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (SUBREG_REG (x), old_rtx, fn, data);
 	  if (op0 == SUBREG_REG (x))
 	    return x;
 	  op0 = simplify_gen_subreg (GET_MODE (x), op0,
@@ -427,15 +434,15 @@ simplify_replace_rtx (rtx x, const_rtx o
     case RTX_OBJ:
       if (code == MEM)
 	{
-	  op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
 	  if (op0 == XEXP (x, 0))
 	    return x;
 	  return replace_equiv_address_nv (x, op0);
 	}
       else if (code == LO_SUM)
 	{
-	  op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
-	  op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
+	  op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
 
 	  /* (lo_sum (high x) x) -> x  */
 	  if (GET_CODE (op0) == HIGH && rtx_equal_p (XEXP (op0, 0), op1))
@@ -445,11 +452,6 @@ simplify_replace_rtx (rtx x, const_rtx o
 	    return x;
 	  return gen_rtx_LO_SUM (mode, op0, op1);
 	}
-      else if (code == REG)
-	{
-	  if (rtx_equal_p (x, old_rtx))
-	    return new_rtx;
-	}
       break;
 
     default:
@@ -457,6 +459,15 @@ simplify_replace_rtx (rtx x, const_rtx o
     }
   return x;
 }
+
+/* Replace all occurrences of OLD_RTX in X with NEW_RTX and try to simplify the
+   resulting RTX.  Return a new RTX which is as simplified as possible.  */
+
+rtx
+simplify_replace_rtx (rtx x, const_rtx old_rtx, rtx new_rtx)
+{
+  return simplify_replace_fn_rtx (x, old_rtx, 0, new_rtx);
+}
 \f
 /* Try to simplify a unary operation CODE whose output mode is to be
    MODE with input operand OP whose mode was originally OP_MODE.
@@ -467,9 +478,6 @@ simplify_unary_operation (enum rtx_code 
 {
   rtx trueop, tem;
 
-  if (GET_CODE (op) == CONST)
-    op = XEXP (op, 0);
-
   trueop = avoid_constant_pool_reference (op);
 
   tem = simplify_const_unary_operation (code, mode, trueop, op_mode);
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2009-09-26 10:43:09.000000000 +0100
+++ gcc/var-tracking.c	2009-09-26 12:12:29.000000000 +0100
@@ -6242,24 +6242,6 @@ delete_variable_part (dataflow_set *set,
   slot = delete_slot_part (set, loc, slot, offset);
 }
 
-/* Wrap result in CONST:MODE if needed to preserve the mode.  */
-
-static rtx
-check_wrap_constant (enum machine_mode mode, rtx result)
-{
-  if (!result || GET_MODE (result) == mode)
-    return result;
-
-  if (dump_file && (dump_flags & TDF_DETAILS))
-    fprintf (dump_file, "  wrapping result in const to preserve mode %s\n",
-	     GET_MODE_NAME (mode));
-
-  result = wrap_constant (mode, result);
-  gcc_assert (GET_MODE (result) == mode);
-
-  return result;
-}
-
 /* Callback for cselib_expand_value, that looks for expressions
    holding the value in the var-tracking hash tables.  Return X for
    standard processing, anything else is to be used as-is.  */
@@ -6323,7 +6305,6 @@ vt_expand_loc_callback (rtx x, bitmap re
     {
       result = cselib_expand_value_rtx_cb (loc->loc, regs, max_depth,
 					   vt_expand_loc_callback, vars);
-      result = check_wrap_constant (GET_MODE (loc->loc), result);
       if (result)
 	break;
     }
@@ -6341,14 +6322,11 @@ vt_expand_loc_callback (rtx x, bitmap re
 static rtx
 vt_expand_loc (rtx loc, htab_t vars)
 {
-  rtx newloc;
-
   if (!MAY_HAVE_DEBUG_INSNS)
     return loc;
 
-  newloc = cselib_expand_value_rtx_cb (loc, scratch_regs, 5,
-				       vt_expand_loc_callback, vars);
-  loc = check_wrap_constant (GET_MODE (loc), newloc);
+  loc = cselib_expand_value_rtx_cb (loc, scratch_regs, 5,
+				    vt_expand_loc_callback, vars);
 
   if (loc && MEM_P (loc))
     loc = targetm.delegitimize_address (loc);

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
>   - It now copies the "to" rtx for each replacement.  All but one caller
>     seemed to pass an already-used rtx as the "to" value, so current
>     substitutions could create invalid sharing.  I've removed the
>     now-redundant copy_rtx from the one caller that used it.

Except I messed up the quilt, so that gcse.c change wasn't part of the
patch I posted.  Here's what I meant to post (and actually tested).

Richard


gcc/
	* rtl.h (simplify_replace_fn_rtx): Declare.
	(wrap_constant, unwrap_constant): Delete.
	* cfgexpand.c (unwrap_constant, wrap_constant): Delete.
	(expand_debug_expr): Don't call wrap_constant.
	* combine.c (rtx_subst_pair): Only define for AUTO_INC_DEC.
	(auto_adjust_pair): Fold into...
	(propagate_for_debug_subst): ...here.  Only define for AUTO_INC_DEC.
	Just return a new value.
	(propagate_for_debug): Use simplify_replace_fn_rtx for AUTO_INC_DEC,
	otherwise use simplify_replace_rtx.
	* cselib.c (wrap_constant): Reinstate old definition.
	(cselib_expand_value_rtx_1): Don't wrap constants.
	* gcse.c (try_replace_reg): Don't use copy_rtx in the call to
	simplify_replace_rtx.
	(bypass_block): Fix formatting in calls to simplify_replace_rtx.
	* reload1.c (reload): Skip all uses for an insn before adjusting it.
	Use simplify_replace_rtx.
	* simplify-rtx.c (simplify_replace_fn_rtx): New function, adapted from...
	(simplify_replace_rtx): ...here.  Turn into a wrapper for
	simplify_replace_fn_rtx.
	(simplify_unary_operation): Don't unwrap CONSTs.
	* var-tracking.c (check_wrap_constant): Delete.
	(vt_expand_loc_callback): Don't call it.
	(vt_expand_loc): Likewise.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2009-09-26 14:23:39.000000000 +0100
+++ gcc/rtl.h	2009-09-26 21:59:22.000000000 +0100
@@ -1765,6 +1765,8 @@ extern rtx simplify_subreg (enum machine
 			    unsigned int);
 extern rtx simplify_gen_subreg (enum machine_mode, rtx, enum machine_mode,
 				unsigned int);
+extern rtx simplify_replace_fn_rtx (rtx, const_rtx,
+				    rtx (*fn) (void *), void *);
 extern rtx simplify_replace_rtx (rtx, const_rtx, rtx);
 extern rtx simplify_rtx (const_rtx);
 extern rtx avoid_constant_pool_reference (rtx);
@@ -2404,8 +2406,6 @@ extern void invert_br_probabilities (rtx
 extern bool expensive_function_p (int);
 /* In cfgexpand.c */
 extern void add_reg_br_prob_note (rtx last, int probability);
-extern rtx wrap_constant (enum machine_mode, rtx);
-extern rtx unwrap_constant (rtx);
 
 /* In var-tracking.c */
 extern unsigned int variable_tracking_main (void);
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	2009-09-26 14:23:39.000000000 +0100
+++ gcc/cfgexpand.c	2009-09-26 21:59:22.000000000 +0100
@@ -2194,46 +2194,6 @@ round_udiv_adjust (enum machine_mode mod
      const1_rtx, const0_rtx);
 }
 
-/* Wrap modeless constants in CONST:MODE.  */
-rtx
-wrap_constant (enum machine_mode mode, rtx x)
-{
-  if (GET_MODE (x) != VOIDmode)
-    return x;
-
-  if (CONST_INT_P (x)
-      || GET_CODE (x) == CONST_FIXED
-      || GET_CODE (x) == CONST_DOUBLE
-      || GET_CODE (x) == LABEL_REF)
-    {
-      gcc_assert (mode != VOIDmode);
-
-      x = gen_rtx_CONST (mode, x);
-    }
-
-  return x;
-}
-
-/* Remove CONST wrapper added by wrap_constant().  */
-rtx
-unwrap_constant (rtx x)
-{
-  rtx ret = x;
-
-  if (GET_CODE (x) != CONST)
-    return x;
-
-  x = XEXP (x, 0);
-
-  if (CONST_INT_P (x)
-      || GET_CODE (x) == CONST_FIXED
-      || GET_CODE (x) == CONST_DOUBLE
-      || GET_CODE (x) == LABEL_REF)
-    ret = x;
-
-  return ret;
-}
-
 /* Convert X to MODE, that must be Pmode or ptr_mode, without emitting
    any rtl.  */
 
@@ -2353,9 +2313,7 @@ expand_debug_expr (tree exp)
     case COMPLEX_CST:
       gcc_assert (COMPLEX_MODE_P (mode));
       op0 = expand_debug_expr (TREE_REALPART (exp));
-      op0 = wrap_constant (GET_MODE_INNER (mode), op0);
       op1 = expand_debug_expr (TREE_IMAGPART (exp));
-      op1 = wrap_constant (GET_MODE_INNER (mode), op1);
       return gen_rtx_CONCAT (mode, op0, op1);
 
     case VAR_DECL:
Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2009-09-26 14:23:39.000000000 +0100
+++ gcc/combine.c	2009-09-26 21:59:22.000000000 +0100
@@ -2257,68 +2257,33 @@ cleanup_auto_inc_dec (rtx src, bool afte
 
   return x;
 }
-#endif
 
 /* Auxiliary data structure for propagate_for_debug_stmt.  */
 
 struct rtx_subst_pair
 {
-  rtx from, to;
-  bool changed;
-#ifdef AUTO_INC_DEC
+  rtx to;
   bool adjusted;
   bool after;
-#endif
 };
 
-/* Clean up any auto-updates in PAIR->to the first time it is called
-   for a PAIR.  PAIR->adjusted is used to tell whether we've cleaned
-   up before.  */
+/* DATA points to an rtx_subst_pair.  Return the value that should be
+   substituted.  */
 
-static void
-auto_adjust_pair (struct rtx_subst_pair *pair ATTRIBUTE_UNUSED)
+static rtx
+propagate_for_debug_subst (void *data)
 {
-#ifdef AUTO_INC_DEC
+  struct rtx_subst_pair *pair = (struct rtx_subst_pair *)data;
+
   if (!pair->adjusted)
     {
       pair->adjusted = true;
       pair->to = cleanup_auto_inc_dec (pair->to, pair->after, VOIDmode);
+      return pair->to;
     }
-#endif
-}
-
-/* If *LOC is the same as FROM in the struct rtx_subst_pair passed as
-   DATA, replace it with a copy of TO.  Handle SUBREGs of *LOC as
-   well.  */
-
-static int
-propagate_for_debug_subst (rtx *loc, void *data)
-{
-  struct rtx_subst_pair *pair = (struct rtx_subst_pair *)data;
-  rtx from = pair->from, to = pair->to;
-  rtx x = *loc, s = x;
-
-  if (rtx_equal_p (x, from)
-      || (GET_CODE (x) == SUBREG && rtx_equal_p ((s = SUBREG_REG (x)), from)))
-    {
-      auto_adjust_pair (pair);
-      if (pair->to != to)
-	to = pair->to;
-      else
-	to = copy_rtx (to);
-      if (s != x)
-	{
-	  gcc_assert (GET_CODE (x) == SUBREG && SUBREG_REG (x) == s);
-	  to = simplify_gen_subreg (GET_MODE (x), to,
-				    GET_MODE (from), SUBREG_BYTE (x));
-	}
-      *loc = wrap_constant (GET_MODE (x), to);
-      pair->changed = true;
-      return -1;
-    }
-
-  return 0;
+  return copy_rtx (pair->to);
 }
+#endif
 
 /* Replace occurrences of DEST with SRC in DEBUG_INSNs between INSN
    and LAST.  If MOVE holds, debug insns must also be moved past
@@ -2327,14 +2292,11 @@ propagate_for_debug_subst (rtx *loc, voi
 static void
 propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src, bool move)
 {
-  struct rtx_subst_pair p;
-  rtx next, move_pos = move ? last : NULL_RTX;
-
-  p.from = dest;
-  p.to = src;
-  p.changed = false;
+  rtx next, move_pos = move ? last : NULL_RTX, loc;
 
 #ifdef AUTO_INC_DEC
+  struct rtx_subst_pair p;
+  p.to = src;
   p.adjusted = false;
   p.after = move;
 #endif
@@ -2346,11 +2308,15 @@ propagate_for_debug (rtx insn, rtx last,
       next = NEXT_INSN (insn);
       if (DEBUG_INSN_P (insn))
 	{
-	  for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
-			propagate_for_debug_subst, &p);
-	  if (!p.changed)
+#ifdef AUTO_INC_DEC
+	  loc = simplify_replace_fn_rtx (INSN_VAR_LOCATION_LOC (insn),
+					 dest, propagate_for_debug_subst, &p);
+#else
+	  loc = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn), dest, src);
+#endif
+	  if (loc == INSN_VAR_LOCATION_LOC (insn))
 	    continue;
-	  p.changed = false;
+	  INSN_VAR_LOCATION_LOC (insn) = loc;
 	  if (move_pos)
 	    {
 	      remove_insn (insn);
Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c	2009-09-26 14:23:39.000000000 +0100
+++ gcc/cselib.c	2009-09-26 21:59:22.000000000 +0100
@@ -661,6 +661,19 @@ rtx_equal_for_cselib_p (rtx x, rtx y)
   return 1;
 }
 
+/* We need to pass down the mode of constants through the hash table
+   functions.  For that purpose, wrap them in a CONST of the appropriate
+   mode.  */
+static rtx
+wrap_constant (enum machine_mode mode, rtx x)
+{
+  if (!CONST_INT_P (x) && GET_CODE (x) != CONST_FIXED
+      && (GET_CODE (x) != CONST_DOUBLE || GET_MODE (x) != VOIDmode))
+    return x;
+  gcc_assert (mode != VOIDmode);
+  return gen_rtx_CONST (mode, x);
+}
+
 /* Hash an rtx.  Return 0 if we couldn't hash the rtx.
    For registers and memory locations, we look up their cselib_val structure
    and return its VALUE element.
@@ -1327,21 +1340,9 @@ cselib_expand_value_rtx_1 (rtx orig, str
     default:
       break;
     }
-  if (scopy == NULL_RTX)
-    {
-      XEXP (copy, 0)
-	= gen_rtx_CONST (GET_MODE (XEXP (orig, 0)), XEXP (copy, 0));
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	fprintf (dump_file, "  wrapping const_int result in const to preserve mode %s\n",
-		 GET_MODE_NAME (GET_MODE (XEXP (copy, 0))));
-    }
   scopy = simplify_rtx (copy);
   if (scopy)
-    {
-      if (GET_MODE (copy) != GET_MODE (scopy))
-	scopy = wrap_constant (GET_MODE (copy), scopy);
-      return scopy;
-    }
+    return scopy;
   return copy;
 }
 
Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c	2009-09-26 21:59:30.000000000 +0100
+++ gcc/gcse.c	2009-09-26 21:59:34.000000000 +0100
@@ -2276,8 +2276,7 @@ try_replace_reg (rtx from, rtx to, rtx i
      with our replacement.  */
   if (note != 0 && REG_NOTE_KIND (note) == REG_EQUAL)
     set_unique_reg_note (insn, REG_EQUAL,
-			 simplify_replace_rtx (XEXP (note, 0), from,
-			 copy_rtx (to)));
+			 simplify_replace_rtx (XEXP (note, 0), from, to));
   if (!success && set && reg_mentioned_p (from, SET_SRC (set)))
     {
       /* If above failed and this is a single set, try to simplify the source of
@@ -3038,12 +3037,12 @@ bypass_block (basic_block bb, rtx setcc,
 	  src = SET_SRC (pc_set (jump));
 
 	  if (setcc != NULL)
-	      src = simplify_replace_rtx (src,
-					  SET_DEST (PATTERN (setcc)),
-					  SET_SRC (PATTERN (setcc)));
+	    src = simplify_replace_rtx (src,
+					SET_DEST (PATTERN (setcc)),
+					SET_SRC (PATTERN (setcc)));
 
 	  new_rtx = simplify_replace_rtx (src, reg_used->reg_rtx,
-				      SET_SRC (set->expr));
+					  SET_SRC (set->expr));
 
 	  /* Jump bypassing may have already placed instructions on
 	     edges of the CFG.  We can't bypass an outgoing edge that
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	2009-09-26 14:23:39.000000000 +0100
+++ gcc/reload1.c	2009-09-26 21:59:22.000000000 +0100
@@ -1257,36 +1257,25 @@ reload (rtx first, int global)
 
 	  for (use = DF_REG_USE_CHAIN (i); use; use = next)
 	    {
-	      rtx *loc = DF_REF_LOC (use);
-	      rtx x = *loc;
-
 	      insn = DF_REF_INSN (use);
+
+	      /* Make sure the next ref is for a different instruction,
+		 so that we're not affected by the rescan.  */
 	      next = DF_REF_NEXT_REG (use);
+	      while (next && DF_REF_INSN (next) == insn)
+		next = DF_REF_NEXT_REG (next);
 
 	      if (DEBUG_INSN_P (insn))
 		{
-		  gcc_assert (x == reg
-			      || (GET_CODE (x) == SUBREG
-				  && SUBREG_REG (x) == reg));
-
 		  if (!equiv)
 		    {
 		      INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
 		      df_insn_rescan_debug_internal (insn);
 		    }
 		  else
-		    {
-		      if (x == reg)
-			*loc = copy_rtx (equiv);
-		      else if (GET_CODE (x) == SUBREG
-			       && SUBREG_REG (x) == reg)
-			*loc = simplify_gen_subreg (GET_MODE (x), equiv,
-						    GET_MODE (reg),
-						    SUBREG_BYTE (x));
-		      else
-			gcc_unreachable ();
-		    *loc = wrap_constant (GET_MODE (x), *loc);
-		    }
+		    INSN_VAR_LOCATION_LOC (insn)
+		      = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn),
+					      reg, equiv);
 		}
 	    }
 	}
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2009-09-26 14:23:39.000000000 +0100
+++ gcc/simplify-rtx.c	2009-09-26 21:59:22.000000000 +0100
@@ -350,38 +350,45 @@ simplify_gen_relational (enum rtx_code c
   return gen_rtx_fmt_ee (code, mode, op0, op1);
 }
 \f
-/* Replace all occurrences of OLD_RTX in X with NEW_RTX and try to simplify the
-   resulting RTX.  Return a new RTX which is as simplified as possible.  */
+/* Replace all occurrences of OLD_RTX in X with FN (DATA).  Treat FN as
+   copy_rtx if it is null.  Canonicalize and simplify the result.  */
 
 rtx
-simplify_replace_rtx (rtx x, const_rtx old_rtx, rtx new_rtx)
+simplify_replace_fn_rtx (rtx x, const_rtx old_rtx,
+			 rtx (*fn) (void *), void *data)
 {
   enum rtx_code code = GET_CODE (x);
   enum machine_mode mode = GET_MODE (x);
   enum machine_mode op_mode;
   rtx op0, op1, op2;
 
-  /* If X is OLD_RTX, return NEW_RTX.  Otherwise, if this is an expression, try
-     to build a new expression substituting recursively.  If we can't do
-     anything, return our input.  */
+  /* If X is OLD_RTX, return FN (DATA), with a null FN equating to
+     copy_rtx.  Otherwise, if this is an expression, try to build a
+     new expression, substituting recursively.  If we can't do anything,
+     return our input.  */
 
-  if (x == old_rtx)
-    return new_rtx;
+  if (rtx_equal_p (x, old_rtx))
+    {
+      if (fn)
+	return fn (data);
+      else
+	return copy_rtx ((rtx) data);
+    }
 
   switch (GET_RTX_CLASS (code))
     {
     case RTX_UNARY:
       op0 = XEXP (x, 0);
       op_mode = GET_MODE (op0);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
       if (op0 == XEXP (x, 0))
 	return x;
       return simplify_gen_unary (code, mode, op0, op_mode);
 
     case RTX_BIN_ARITH:
     case RTX_COMM_ARITH:
-      op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1))
 	return x;
       return simplify_gen_binary (code, mode, op0, op1);
@@ -391,8 +398,8 @@ simplify_replace_rtx (rtx x, const_rtx o
       op0 = XEXP (x, 0);
       op1 = XEXP (x, 1);
       op_mode = GET_MODE (op0) != VOIDmode ? GET_MODE (op0) : GET_MODE (op1);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (op1, old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (op1, old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1))
 	return x;
       return simplify_gen_relational (code, mode, op_mode, op0, op1);
@@ -401,9 +408,9 @@ simplify_replace_rtx (rtx x, const_rtx o
     case RTX_BITFIELD_OPS:
       op0 = XEXP (x, 0);
       op_mode = GET_MODE (op0);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
-      op2 = simplify_replace_rtx (XEXP (x, 2), old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
+      op2 = simplify_replace_fn_rtx (XEXP (x, 2), old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1) && op2 == XEXP (x, 2))
 	return x;
       if (op_mode == VOIDmode)
@@ -414,7 +421,7 @@ simplify_replace_rtx (rtx x, const_rtx o
       /* The only case we try to handle is a SUBREG.  */
       if (code == SUBREG)
 	{
-	  op0 = simplify_replace_rtx (SUBREG_REG (x), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (SUBREG_REG (x), old_rtx, fn, data);
 	  if (op0 == SUBREG_REG (x))
 	    return x;
 	  op0 = simplify_gen_subreg (GET_MODE (x), op0,
@@ -427,15 +434,15 @@ simplify_replace_rtx (rtx x, const_rtx o
     case RTX_OBJ:
       if (code == MEM)
 	{
-	  op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
 	  if (op0 == XEXP (x, 0))
 	    return x;
 	  return replace_equiv_address_nv (x, op0);
 	}
       else if (code == LO_SUM)
 	{
-	  op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
-	  op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
+	  op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
 
 	  /* (lo_sum (high x) x) -> x  */
 	  if (GET_CODE (op0) == HIGH && rtx_equal_p (XEXP (op0, 0), op1))
@@ -445,11 +452,6 @@ simplify_replace_rtx (rtx x, const_rtx o
 	    return x;
 	  return gen_rtx_LO_SUM (mode, op0, op1);
 	}
-      else if (code == REG)
-	{
-	  if (rtx_equal_p (x, old_rtx))
-	    return new_rtx;
-	}
       break;
 
     default:
@@ -457,6 +459,15 @@ simplify_replace_rtx (rtx x, const_rtx o
     }
   return x;
 }
+
+/* Replace all occurrences of OLD_RTX in X with NEW_RTX and try to simplify the
+   resulting RTX.  Return a new RTX which is as simplified as possible.  */
+
+rtx
+simplify_replace_rtx (rtx x, const_rtx old_rtx, rtx new_rtx)
+{
+  return simplify_replace_fn_rtx (x, old_rtx, 0, new_rtx);
+}
 \f
 /* Try to simplify a unary operation CODE whose output mode is to be
    MODE with input operand OP whose mode was originally OP_MODE.
@@ -467,9 +478,6 @@ simplify_unary_operation (enum rtx_code 
 {
   rtx trueop, tem;
 
-  if (GET_CODE (op) == CONST)
-    op = XEXP (op, 0);
-
   trueop = avoid_constant_pool_reference (op);
 
   tem = simplify_const_unary_operation (code, mode, trueop, op_mode);
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2009-09-26 14:23:39.000000000 +0100
+++ gcc/var-tracking.c	2009-09-26 21:59:22.000000000 +0100
@@ -6242,24 +6242,6 @@ delete_variable_part (dataflow_set *set,
   slot = delete_slot_part (set, loc, slot, offset);
 }
 
-/* Wrap result in CONST:MODE if needed to preserve the mode.  */
-
-static rtx
-check_wrap_constant (enum machine_mode mode, rtx result)
-{
-  if (!result || GET_MODE (result) == mode)
-    return result;
-
-  if (dump_file && (dump_flags & TDF_DETAILS))
-    fprintf (dump_file, "  wrapping result in const to preserve mode %s\n",
-	     GET_MODE_NAME (mode));
-
-  result = wrap_constant (mode, result);
-  gcc_assert (GET_MODE (result) == mode);
-
-  return result;
-}
-
 /* Callback for cselib_expand_value, that looks for expressions
    holding the value in the var-tracking hash tables.  Return X for
    standard processing, anything else is to be used as-is.  */
@@ -6323,7 +6305,6 @@ vt_expand_loc_callback (rtx x, bitmap re
     {
       result = cselib_expand_value_rtx_cb (loc->loc, regs, max_depth,
 					   vt_expand_loc_callback, vars);
-      result = check_wrap_constant (GET_MODE (loc->loc), result);
       if (result)
 	break;
     }
@@ -6341,14 +6322,11 @@ vt_expand_loc_callback (rtx x, bitmap re
 static rtx
 vt_expand_loc (rtx loc, htab_t vars)
 {
-  rtx newloc;
-
   if (!MAY_HAVE_DEBUG_INSNS)
     return loc;
 
-  newloc = cselib_expand_value_rtx_cb (loc, scratch_regs, 5,
-				       vt_expand_loc_callback, vars);
-  loc = check_wrap_constant (GET_MODE (loc), newloc);
+  loc = cselib_expand_value_rtx_cb (loc, scratch_regs, 5,
+				    vt_expand_loc_callback, vars);
 
   if (loc && MEM_P (loc))
     loc = targetm.delegitimize_address (loc);

Re: Use simplify_replace_rtx rather than wrap_constant

[Alexandre Oliva]

On Sep 26, 2009, Richard Sandiford <rdsandiford@googlemail.com> wrote:

> I wouldn't be entirely surprised if this patch introduces some post-VTA
> regressions that weren't picked up by the usual testing.  But if it does,
> I think that's actually a useful thing, because we'll then be able to
> pinpoint the places that are doing invalid substitutions, losing the mode,
> or whatever.  And if it doesn't introduce any regressions, well, even better!

I agree.  I like the (followup) patch.  In hindsight, the approach of
naively substituting RTX and adding CONSTs to preserve modes where
needed was quite poor.  Thanks for cleaning this up.

The one bit I'm concerned about is the potential loss of modes in
CONST_INTs in VALUE equivalence tables and in the expansion of locs.
I'd expect some assertions to fail if this patch actually broke anything
in this regard, though.  We'll see.

As far as I'm concerned, you can check it in, but I'm not entitled to
approve it.

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

Re: Use simplify_replace_rtx rather than wrap_constant

[Bernd Schmidt]

Richard Sandiford wrote:
> I made a couple of other changes to simplify_replace_rtx too:
> 
>   - It now copies the "to" rtx for each replacement.  All but one caller
>     seemed to pass an already-used rtx as the "to" value, so current
>     substitutions could create invalid sharing.  I've removed the
>     now-redundant copy_rtx from the one caller that used it.
> 
>     (Doing the copy lazily seems better, and copes with cases where the
>     value is used twice within the same expression.)
> 
>   - It now uses rtx_equal_p to check for equivalence.  Some callers seem
>     to pass MEM rtxes, which can't be shared, so I think using rtx_equal_p
>     is better than relying on pointer equivalence for everything except REGs.
>     I scanned the current callers, and none of them seemed to be using
>     the function in a way that genuinely required strict pointer
>     equivalence.
> 
>     rtx_equal_p isn't cheap of course.  We could reduce the overhead
>     by turning it into an inline wrapper that first checks whether the
>     code and mode are the same (or maybe just the code) and only then
>     calls an out-of-line function.

Can you split these out?  They seem to be unrelated to the problem at
hand and I'm not sure they are good changes.  For the
simplify_replace_rtx cases in loop_iv.c, for example, I'm quite sure
invalid sharing is not an issue.

Based on Alex Oliva's OK I'll approve the rest of the changes.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

Re: Use simplify_replace_rtx rather than wrap_constant

[Paolo Bonzini]

On 09/26/2009 04:44 PM, Richard Sandiford wrote:
> +/* Replace all occurrences of OLD_RTX in X with FN (DATA).  Treat FN as
> +   copy_rtx if it is null.  Canonicalize and simplify the result.  */

Richard, this is a great patch.  The only objection is, I would pass X 
to FN too.  This way, if we have to build something out of X we can 
avoid copying it unconditionally.

Paolo

Re: Use simplify_replace_rtx rather than wrap_constant

[Paolo Bonzini]

On 09/26/2009 04:44 PM, Richard Sandiford wrote:
> +/* Replace all occurrences of OLD_RTX in X with FN (DATA).  Treat FN as
> +   copy_rtx if it is null.  Canonicalize and simplify the result.  */

Richard, this is a great patch.  The only objection is, I would pass X 
to FN too.  This way, if we have to build something out of X we can 
avoid copying it unconditionally.

Paolo

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Bernd Schmidt <bernds_cb1@t-online.de> writes:
> Richard Sandiford wrote:
>> I made a couple of other changes to simplify_replace_rtx too:
>> 
>>   - It now copies the "to" rtx for each replacement.  All but one caller
>>     seemed to pass an already-used rtx as the "to" value, so current
>>     substitutions could create invalid sharing.  I've removed the
>>     now-redundant copy_rtx from the one caller that used it.
>> 
>>     (Doing the copy lazily seems better, and copes with cases where the
>>     value is used twice within the same expression.)
>> 
>>   - It now uses rtx_equal_p to check for equivalence.  Some callers seem
>>     to pass MEM rtxes, which can't be shared, so I think using rtx_equal_p
>>     is better than relying on pointer equivalence for everything except REGs.
>>     I scanned the current callers, and none of them seemed to be using
>>     the function in a way that genuinely required strict pointer
>>     equivalence.
>> 
>>     rtx_equal_p isn't cheap of course.  We could reduce the overhead
>>     by turning it into an inline wrapper that first checks whether the
>>     code and mode are the same (or maybe just the code) and only then
>>     calls an out-of-line function.
>
> Can you split these out?  They seem to be unrelated to the problem at
> hand and I'm not sure they are good changes.  For the
> simplify_replace_rtx cases in loop_iv.c, for example, I'm quite sure
> invalid sharing is not an issue.

The combine.c code isn't right without these two changes.
The current combine code (rightly IMO) uses rtx_equal_p to
check for equivalence between the rtx and the "from" value,
and it uses copy_rtx to make sure that each "to" value is unique.

Why don't you think the changes are correct?  If the "from"
value occurs twice in the expression then we'll end up with
two copies of the "to" value.  We ought to make sure that
the two copies are only shared if copy_rtx says that they
_can_ be shared.  Callers in general don't know whether the
the "from" expression only occurs once, or whether the "to"
value can be shared, so I don't think the onus should be
on them.

Likewise with rtx_equal_p.  The reason we prohibit sharing of MEMs is
(AIUI) because we want to be able to perform substitutions on one MEM's
address without it having a knock-on effect in some unrelated situation.
The different addresses don't carry any semantic information.  So I
think using rtx_equal_p should be the default position, and we should
only switch to pointer equivalence if there is s specific need to, or if
it can be guaranteed correct for all callers.  It doesn't seem
appropriate for such a general function as simplify_replace_rtx.

I think any _reliance_ on pointer equivalence (i.e. any deliberate
_rejection_ of rtx_equal_p for correctness reasons) is special enough
that it ought to be flagged up by something in the function name
(and ideally a comment too).  As I mentioned above, I tried to check
that each current caller of simplify_replace_rtx does not rely on
pointer equivalence in this way.

Do you think the changes are unsafe, or are you worried about the
performance impact?  If the latter, then what about my suggestion
of making rtx_equal_p an inline wrapper to an out-of-line function?
I think that is a separate change, but I'm happy to do it if you
think it's worthwhile (or even necessary for the main patch to
be acceptable).

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Paolo Bonzini <bonzini@gnu.org> writes:
> On 09/26/2009 04:44 PM, Richard Sandiford wrote:
>> +/* Replace all occurrences of OLD_RTX in X with FN (DATA).  Treat FN as
>> +   copy_rtx if it is null.  Canonicalize and simplify the result.  */
>
> Richard, this is a great patch.  The only objection is, I would pass X 
> to FN too.  This way, if we have to build something out of X we can 
> avoid copying it unconditionally.

OK, I can do that.  I'll wait for Bernd's reply before submitting
an updated patch though.

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Hi Alex,

Thanks for the feedback.

Alexandre Oliva <aoliva@redhat.com> writes:
> The one bit I'm concerned about is the potential loss of modes in
> CONST_INTs in VALUE equivalence tables and in the expansion of locs.
> I'd expect some assertions to fail if this patch actually broke anything
> in this regard, though.  We'll see.

The mode of a VALUE entry is taken from the VALUE rtx itself, so if
we're looking up a value in the table, we always have the table entry's
mode to hand.  The problem is the mode of the value we're trying to
find, which is what wrap_constant was originally there to deal with.
The patch leaves that part intact, although like I say, I'd prefer to
pass an (rtx, mode) pair instead a throw-away CONST.

When replacing VALUEs with CONST_INTs, the principle is the same:
the CONST_INT in the table should already be canonical for the
VALUE's mode, and anything that wants to know the mode of the
CONST_INT should either (a) get it from the VALUE (if it's still
to hand) or (b) be given the information up-front, such as in a
separate function parameter.  The principle's the same as the
result of simplify_binary_operation, or whatever, since it too
could return a CONST_INT for a non-CONST_INT input.

(I realise I'm probably not saying anything new here.
Just doing it for avoidance of doubt.)

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Bernd Schmidt]

Richard Sandiford wrote:

> Do you think the changes are unsafe, or are you worried about the
> performance impact?

The latter (performance and memory usage), and I don't like the idea of
having this change buried in a larger patch.  So, if the other changes
depend on it, split it off and make sure it gets in first.

I'd also like more information about which existing places you found
that made you think this change is needed (wrt. MEMs or possibly invalid
sharing).

You could leave the decision whether to copy or not to the caller,
simply by avoiding the copy if fn is NULL, and making the caller pass in
copy_rtx as the fn otherwise.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

Re: Use simplify_replace_rtx rather than wrap_constant

[Eric Botcazou]

> Do you think the changes are unsafe, or are you worried about the
> performance impact?  If the latter, then what about my suggestion
> of making rtx_equal_p an inline wrapper to an out-of-line function?
> I think that is a separate change, but I'm happy to do it if you
> think it's worthwhile (or even necessary for the main patch to
> be acceptable).

FWIW I share Bernd's concerns, in particular I'd be cautious about unsharing 
everything by default; the recog.c approach might be better, with the two 
variants validate_change and validate_unshare_change

But I agree that making rtx_equal_p an inline wrapper would be a good idea to 
speed things up; there are already a few manual attemps, e.g.

  /* X matches FROM if it is the same rtx or they are both referring to the
     same register in the same mode.  Avoid calling rtx_equal_p unless the
     operands look similar.  */

  if (x == from
      || (REG_P (x) && REG_P (from)
	  && GET_MODE (x) == GET_MODE (from)
	  && REGNO (x) == REGNO (from))
      || (GET_CODE (x) == GET_CODE (from) && GET_MODE (x) == GET_MODE (from)
	  && rtx_equal_p (x, from)))
    {
      validate_unshare_change (object, loc, to, 1);
      return;
    }

-- 
Eric Botcazou

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Bernd Schmidt <bernds_cb1@t-online.de> writes:
> Richard Sandiford wrote:
>
>> Do you think the changes are unsafe, or are you worried about the
>> performance impact?
>
> The latter (performance and memory usage), and I don't like the idea of
> having this change buried in a larger patch.  So, if the other changes
> depend on it, split it off and make sure it gets in first.
>
> I'd also like more information about which existing places you found
> that made you think this change is needed (wrt. MEMs or possibly invalid
> sharing).

OK, well, let's go through each caller in turn:

fwprop.c:forward_propagate_and_simplify()

    Here we're replacing a MEM with a constant.  The extra rtx_equal_p()
    is safe but redundant.  The copy_rtx() will be a no-op for sharable
    constants, but strictly speaking, it might be needed for unsharable
    constants.

gcse.c:try_replace_reg()

    Here's we're replacing a register with either a new register or
    a gcse_constant_p().  The extra rtx_equal_p() is an operational
    no-op because simplify_replace_rtx() already uses rtx_equal_p()
    for registers.  The copy_rtx() will be a no-op for registers.

    The gcse_constant_p() case is interesting because we have:

      /* Determine whether the rtx X should be treated as a constant for
         the purposes of GCSE's constant propagation.  */

      static bool
      gcse_constant_p (const_rtx x)
      {
        /* Consider a COMPARE of two integers constant.  */
        if (GET_CODE (x) == COMPARE
            && CONST_INT_P (XEXP (x, 0))
            && CONST_INT_P (XEXP (x, 1)))
          return true;

        /* Consider a COMPARE of the same registers is a constant
           if they are not floating point registers.  */
        if (GET_CODE(x) == COMPARE
            && REG_P (XEXP (x, 0)) && REG_P (XEXP (x, 1))
            && REGNO (XEXP (x, 0)) == REGNO (XEXP (x, 1))
            && ! FLOAT_MODE_P (GET_MODE (XEXP (x, 0)))
            && ! FLOAT_MODE_P (GET_MODE (XEXP (x, 1))))
          return true;

        /* Since X might be inserted more than once we have to take care that it
           is sharable.  */
        return CONSTANT_P (x) && (GET_CODE (x) != CONST || shared_const_p (x));
      }

    The first two cases look a little hackish, but strictly speaking,
    those COMPAREs aren't shareable.  We would need the copy_rtx()
    for them.  (Or I suppose we could just say "well, it shouldn't
    matter if we share those particular unsharable expressions,
    because they'll probably be reduced or rejected".  But that isn't
    being correct by design.)

    Also note that the comment above try_replace_reg() says:

      /* Try to replace all non-SET_DEST occurrences of FROM in INSN with TO.
         Returns nonzero is successful.  */

    so there's no guarantee that FROM occurs only once.

    In the usual case, we only consider sharable constants to be
    gcse_constant_p(), so the copy_rtx() will be a no-op.  But
    try_simplify_rtx() nevertheless has:

      /* Usually we substitute easy stuff, so we won't copy everything.
         We however need to take care to not duplicate non-trivial CONST
         expressions.  */
      to = copy_rtx (to);

    TBH, I think this is exactly the sort of thing that happens when you
    leave it up to the caller rather than the callee to handle sharing
    (which is after all a correctness decision).  Optimisation decisions
    like gcse_constant_p() shouldn't be affected by such an internal
    decision as sharability.

gcse.c:cprop_jump()

    The second call replaces a register with a gcse_constant_p(),
    so this case is the same as try_replace_reg().

    The first call replaces a setcc destination with a setcc source
    pattern, or with a REG_EQUAL note, if present.  This can create
    invalid sharing in cases where no REG_EQUAL note is present.
    There's also no guarantee that the register is used only once in the
    jump, although that is of course the usual case.  (Or again we could
    just say "well, it shouldn't matter if we share those particular
    expressions, because they'll probably be reduced or rejected".
    But that again isn't being correct by design.)

gcse.c:bypass_block()

    Like cprop_jump(), the first call replaces a setcc destination
    with a setcc source, while the second call replaces a register
    with a gcse_constant_p().  The same analysis applies.

loop-iv.c:replace_single_def_regs()

    This replaces a register with a function_invariant_p().
    These invariants include (plus (fp|ap) (const_int ...)), which isn't
    sharable, so a copy_rtx() would be needed here for each use of
    the register.  (Or each use minus 1 if we can guarantee that the
    original value is no longer needed.)  Again, there is no guarantee
    that the "from" register is used only once in the argument to
    simplify_replace_rtx(), so the decision shouldn't be left up
    to the caller.

loop-iv.c:replace_in_expr()

    This replaces a register with a simple_rhs_p().  These simple rhses
    include various kinds of unsharable binary operation, so the same
    copying requirements apply as for replace_single_def_regs().
    
loop-iv.c:implies_p()

    This function uses simplify_replace_rtx() to see whether something
    simplifies to "true".  Sharing isn't a concern here, but the
    function can create plenty of throw-away rtx when used in
    this way, regardless of whether we use copy_rtx().

loop-iv.c:simplify_using_condition()

    This function replaces a REG with a CONSTANT_P.  Some CONSTANT_Ps
    aren't sharable, so the copy here could be needed.  If the users
    of the returned value are mindful that the returned value could
    have invalid sharing, then I think there should be a comment
    to say so.  Otherwise the same "correctness by design" concerns
    apply as before.

In short, I think implies_p() is the only case where both the following apply:

  (a) copy_rtx() might create new rtl
  (b) the creation of that new rtl is always unnecessary

And like I say, the creation of _any_ new rtl in the implies_p() case
is unnecessary, so I don't think it's a good example.

Really, I think any assumption along the lines of "this sharing isn't
strictly correct, but it shouldn't matter in this case" is as bad as
the problem I'm trying to fix.  It feels like premature optimisation.

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Bernd Schmidt]

Richard Sandiford wrote:
> 
> loop-iv.c:replace_single_def_regs()
> loop-iv.c:replace_in_expr()
> loop-iv.c:implies_p()
> loop-iv.c:simplify_using_condition()

I think all of these are ok right now since we don't put the rtl we
create back into an insn - all of this code is designed to simplify an
expression to a constant, and we won't use the result otherwise.

I guess I can live with the change; thanks for posting the cases you
found.  OK, but please do install this change as a separate commit.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Bernd Schmidt <bernds_cb1@t-online.de> writes:
> Richard Sandiford wrote:
>> 
>> loop-iv.c:replace_single_def_regs()
>> loop-iv.c:replace_in_expr()
>> loop-iv.c:implies_p()
>> loop-iv.c:simplify_using_condition()
>
> I think all of these are ok right now since we don't put the rtl we
> create back into an insn - all of this code is designed to simplify an
> expression to a constant, and we won't use the result otherwise.

I agree they're probably OK, but it's the probably that worries me.
I think it's dangerous to have shared subrtxes floating around
in this way.  E.g. simplify_using_initial_values can apply a
substitution to one expression while still holding the result
of the same substitution on another expression, so the routines
we call during that period mustn't assume valid sharing of the
"to" value.

It might be better to make the sharing rules apply across the board, not
just to rtxes in the insn stream, since that way we could destructively
modify rtxes more often (and thus create less garbage rtl).  But that's
obviously only for the brave.

> I guess I can live with the change; thanks for posting the cases you
> found.  OK, but please do install this change as a separate commit.

Thanks.  Here's what I installed after bootstrapping and
regression-testing on x86_64-linux-gnu.  I'll revise the main
patch with the change Paolo suggested and repost.

Richard


gcc/
	* simplify-rtx.c (simplify_replace_rtx): Use rtx_equal_p for
	all OLD_RTXes, not just REGs.  Use copy_rtx to create the
	replacement value.

Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2009-10-11 17:12:45.000000000 +0100
+++ gcc/simplify-rtx.c	2009-10-11 17:13:09.000000000 +0100
@@ -365,8 +365,8 @@ simplify_replace_rtx (rtx x, const_rtx o
      to build a new expression substituting recursively.  If we can't do
      anything, return our input.  */
 
-  if (x == old_rtx)
-    return new_rtx;
+  if (rtx_equal_p (x, old_rtx))
+    return copy_rtx (new_rtx);
 
   switch (GET_RTX_CLASS (code))
     {
@@ -445,11 +445,6 @@ simplify_replace_rtx (rtx x, const_rtx o
 	    return x;
 	  return gen_rtx_LO_SUM (mode, op0, op1);
 	}
-      else if (code == REG)
-	{
-	  if (rtx_equal_p (x, old_rtx))
-	    return new_rtx;
-	}
       break;
 
     default:

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Thanks.  Here's what I installed after bootstrapping and
> regression-testing on x86_64-linux-gnu.  I'll revise the main
> patch with the change Paolo suggested and repost.

And here's the revised main patch.  Bootstrapped & regression-tested
on x86_64-linux-gnu, and regression-tested on arm-eabi.  OK to install?

Richard


gcc/
	* rtl.h (simplify_replace_fn_rtx): Declare.
	(wrap_constant, unwrap_constant): Delete.
	* cfgexpand.c (unwrap_constant, wrap_constant): Delete.
	(expand_debug_expr): Don't call wrap_constant.
	* combine.c (rtx_subst_pair): Only define for AUTO_INC_DEC.
	(auto_adjust_pair): Fold into...
	(propagate_for_debug_subst): ...here.  Only define for AUTO_INC_DEC.
	Just return a new value.
	(propagate_for_debug): Use simplify_replace_fn_rtx for AUTO_INC_DEC,
	otherwise use simplify_replace_rtx.
	* cselib.c (wrap_constant): Reinstate old definition.
	(cselib_expand_value_rtx_1): Don't wrap constants.
	* gcse.c (try_replace_reg): Don't use copy_rtx in the call to
	simplify_replace_rtx.
	(bypass_block): Fix formatting in calls to simplify_replace_rtx.
	* reload1.c (reload): Skip all uses for an insn before adjusting it.
	Use simplify_replace_rtx.
	* simplify-rtx.c (simplify_replace_fn_rtx): New function,
	adapted from...
	(simplify_replace_rtx): ...here.  Turn into a wrapper for
	simplify_replace_fn_rtx.
	(simplify_unary_operation): Don't unwrap CONSTs.
	* var-tracking.c (check_wrap_constant): Delete.
	(vt_expand_loc_callback): Don't call it.
	(vt_expand_loc): Likewise.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2009-10-05 20:08:12.000000000 +0100
+++ gcc/rtl.h	2009-10-11 20:13:39.000000000 +0100
@@ -1765,6 +1765,8 @@ extern rtx simplify_subreg (enum machine
 			    unsigned int);
 extern rtx simplify_gen_subreg (enum machine_mode, rtx, enum machine_mode,
 				unsigned int);
+extern rtx simplify_replace_fn_rtx (rtx, const_rtx,
+				    rtx (*fn) (rtx, void *), void *);
 extern rtx simplify_replace_rtx (rtx, const_rtx, rtx);
 extern rtx simplify_rtx (const_rtx);
 extern rtx avoid_constant_pool_reference (rtx);
@@ -2404,8 +2406,6 @@ extern void invert_br_probabilities (rtx
 extern bool expensive_function_p (int);
 /* In cfgexpand.c */
 extern void add_reg_br_prob_note (rtx last, int probability);
-extern rtx wrap_constant (enum machine_mode, rtx);
-extern rtx unwrap_constant (rtx);
 
 /* In var-tracking.c */
 extern unsigned int variable_tracking_main (void);
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	2009-10-05 20:08:12.000000000 +0100
+++ gcc/cfgexpand.c	2009-10-11 20:09:12.000000000 +0100
@@ -2194,46 +2194,6 @@ round_udiv_adjust (enum machine_mode mod
      const1_rtx, const0_rtx);
 }
 
-/* Wrap modeless constants in CONST:MODE.  */
-rtx
-wrap_constant (enum machine_mode mode, rtx x)
-{
-  if (GET_MODE (x) != VOIDmode)
-    return x;
-
-  if (CONST_INT_P (x)
-      || GET_CODE (x) == CONST_FIXED
-      || GET_CODE (x) == CONST_DOUBLE
-      || GET_CODE (x) == LABEL_REF)
-    {
-      gcc_assert (mode != VOIDmode);
-
-      x = gen_rtx_CONST (mode, x);
-    }
-
-  return x;
-}
-
-/* Remove CONST wrapper added by wrap_constant().  */
-rtx
-unwrap_constant (rtx x)
-{
-  rtx ret = x;
-
-  if (GET_CODE (x) != CONST)
-    return x;
-
-  x = XEXP (x, 0);
-
-  if (CONST_INT_P (x)
-      || GET_CODE (x) == CONST_FIXED
-      || GET_CODE (x) == CONST_DOUBLE
-      || GET_CODE (x) == LABEL_REF)
-    ret = x;
-
-  return ret;
-}
-
 /* Convert X to MODE, that must be Pmode or ptr_mode, without emitting
    any rtl.  */
 
@@ -2356,9 +2316,7 @@ expand_debug_expr (tree exp)
     case COMPLEX_CST:
       gcc_assert (COMPLEX_MODE_P (mode));
       op0 = expand_debug_expr (TREE_REALPART (exp));
-      op0 = wrap_constant (GET_MODE_INNER (mode), op0);
       op1 = expand_debug_expr (TREE_IMAGPART (exp));
-      op1 = wrap_constant (GET_MODE_INNER (mode), op1);
       return gen_rtx_CONCAT (mode, op0, op1);
 
     case VAR_DECL:
Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2009-10-10 09:16:48.000000000 +0100
+++ gcc/combine.c	2009-10-11 20:14:04.000000000 +0100
@@ -2264,68 +2264,33 @@ cleanup_auto_inc_dec (rtx src, bool afte
 
   return x;
 }
-#endif
 
 /* Auxiliary data structure for propagate_for_debug_stmt.  */
 
 struct rtx_subst_pair
 {
-  rtx from, to;
-  bool changed;
-#ifdef AUTO_INC_DEC
+  rtx to;
   bool adjusted;
   bool after;
-#endif
 };
 
-/* Clean up any auto-updates in PAIR->to the first time it is called
-   for a PAIR.  PAIR->adjusted is used to tell whether we've cleaned
-   up before.  */
+/* DATA points to an rtx_subst_pair.  Return the value that should be
+   substituted.  */
 
-static void
-auto_adjust_pair (struct rtx_subst_pair *pair ATTRIBUTE_UNUSED)
+static rtx
+propagate_for_debug_subst (rtx from ATTRIBUTE_UNUSED, void *data)
 {
-#ifdef AUTO_INC_DEC
+  struct rtx_subst_pair *pair = (struct rtx_subst_pair *)data;
+
   if (!pair->adjusted)
     {
       pair->adjusted = true;
       pair->to = cleanup_auto_inc_dec (pair->to, pair->after, VOIDmode);
+      return pair->to;
     }
-#endif
-}
-
-/* If *LOC is the same as FROM in the struct rtx_subst_pair passed as
-   DATA, replace it with a copy of TO.  Handle SUBREGs of *LOC as
-   well.  */
-
-static int
-propagate_for_debug_subst (rtx *loc, void *data)
-{
-  struct rtx_subst_pair *pair = (struct rtx_subst_pair *)data;
-  rtx from = pair->from, to = pair->to;
-  rtx x = *loc, s = x;
-
-  if (rtx_equal_p (x, from)
-      || (GET_CODE (x) == SUBREG && rtx_equal_p ((s = SUBREG_REG (x)), from)))
-    {
-      auto_adjust_pair (pair);
-      if (pair->to != to)
-	to = pair->to;
-      else
-	to = copy_rtx (to);
-      if (s != x)
-	{
-	  gcc_assert (GET_CODE (x) == SUBREG && SUBREG_REG (x) == s);
-	  to = simplify_gen_subreg (GET_MODE (x), to,
-				    GET_MODE (from), SUBREG_BYTE (x));
-	}
-      *loc = wrap_constant (GET_MODE (x), to);
-      pair->changed = true;
-      return -1;
-    }
-
-  return 0;
+  return copy_rtx (pair->to);
 }
+#endif
 
 /* Replace occurrences of DEST with SRC in DEBUG_INSNs between INSN
    and LAST.  If MOVE holds, debug insns must also be moved past
@@ -2334,14 +2299,11 @@ propagate_for_debug_subst (rtx *loc, voi
 static void
 propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src, bool move)
 {
-  struct rtx_subst_pair p;
-  rtx next, move_pos = move ? last : NULL_RTX;
-
-  p.from = dest;
-  p.to = src;
-  p.changed = false;
+  rtx next, move_pos = move ? last : NULL_RTX, loc;
 
 #ifdef AUTO_INC_DEC
+  struct rtx_subst_pair p;
+  p.to = src;
   p.adjusted = false;
   p.after = move;
 #endif
@@ -2353,11 +2315,15 @@ propagate_for_debug (rtx insn, rtx last,
       next = NEXT_INSN (insn);
       if (DEBUG_INSN_P (insn))
 	{
-	  for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
-			propagate_for_debug_subst, &p);
-	  if (!p.changed)
+#ifdef AUTO_INC_DEC
+	  loc = simplify_replace_fn_rtx (INSN_VAR_LOCATION_LOC (insn),
+					 dest, propagate_for_debug_subst, &p);
+#else
+	  loc = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn), dest, src);
+#endif
+	  if (loc == INSN_VAR_LOCATION_LOC (insn))
 	    continue;
-	  p.changed = false;
+	  INSN_VAR_LOCATION_LOC (insn) = loc;
 	  if (move_pos)
 	    {
 	      remove_insn (insn);
Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c	2009-09-27 10:05:24.000000000 +0100
+++ gcc/cselib.c	2009-10-11 20:09:12.000000000 +0100
@@ -661,6 +661,19 @@ rtx_equal_for_cselib_p (rtx x, rtx y)
   return 1;
 }
 
+/* We need to pass down the mode of constants through the hash table
+   functions.  For that purpose, wrap them in a CONST of the appropriate
+   mode.  */
+static rtx
+wrap_constant (enum machine_mode mode, rtx x)
+{
+  if (!CONST_INT_P (x) && GET_CODE (x) != CONST_FIXED
+      && (GET_CODE (x) != CONST_DOUBLE || GET_MODE (x) != VOIDmode))
+    return x;
+  gcc_assert (mode != VOIDmode);
+  return gen_rtx_CONST (mode, x);
+}
+
 /* Hash an rtx.  Return 0 if we couldn't hash the rtx.
    For registers and memory locations, we look up their cselib_val structure
    and return its VALUE element.
@@ -1327,21 +1340,9 @@ cselib_expand_value_rtx_1 (rtx orig, str
     default:
       break;
     }
-  if (scopy == NULL_RTX)
-    {
-      XEXP (copy, 0)
-	= gen_rtx_CONST (GET_MODE (XEXP (orig, 0)), XEXP (copy, 0));
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	fprintf (dump_file, "  wrapping const_int result in const to preserve mode %s\n",
-		 GET_MODE_NAME (GET_MODE (XEXP (copy, 0))));
-    }
   scopy = simplify_rtx (copy);
   if (scopy)
-    {
-      if (GET_MODE (copy) != GET_MODE (scopy))
-	scopy = wrap_constant (GET_MODE (copy), scopy);
-      return scopy;
-    }
+    return scopy;
   return copy;
 }
 
Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c	2009-09-27 10:05:31.000000000 +0100
+++ gcc/gcse.c	2009-10-11 20:09:12.000000000 +0100
@@ -2276,8 +2276,7 @@ try_replace_reg (rtx from, rtx to, rtx i
      with our replacement.  */
   if (note != 0 && REG_NOTE_KIND (note) == REG_EQUAL)
     set_unique_reg_note (insn, REG_EQUAL,
-			 simplify_replace_rtx (XEXP (note, 0), from,
-			 copy_rtx (to)));
+			 simplify_replace_rtx (XEXP (note, 0), from, to));
   if (!success && set && reg_mentioned_p (from, SET_SRC (set)))
     {
       /* If above failed and this is a single set, try to simplify the source of
@@ -3038,12 +3037,12 @@ bypass_block (basic_block bb, rtx setcc,
 	  src = SET_SRC (pc_set (jump));
 
 	  if (setcc != NULL)
-	      src = simplify_replace_rtx (src,
-					  SET_DEST (PATTERN (setcc)),
-					  SET_SRC (PATTERN (setcc)));
+	    src = simplify_replace_rtx (src,
+					SET_DEST (PATTERN (setcc)),
+					SET_SRC (PATTERN (setcc)));
 
 	  new_rtx = simplify_replace_rtx (src, reg_used->reg_rtx,
-				      SET_SRC (set->expr));
+					  SET_SRC (set->expr));
 
 	  /* Jump bypassing may have already placed instructions on
 	     edges of the CFG.  We can't bypass an outgoing edge that
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	2009-09-27 10:05:24.000000000 +0100
+++ gcc/reload1.c	2009-10-11 20:09:12.000000000 +0100
@@ -1257,36 +1257,25 @@ reload (rtx first, int global)
 
 	  for (use = DF_REG_USE_CHAIN (i); use; use = next)
 	    {
-	      rtx *loc = DF_REF_LOC (use);
-	      rtx x = *loc;
-
 	      insn = DF_REF_INSN (use);
+
+	      /* Make sure the next ref is for a different instruction,
+		 so that we're not affected by the rescan.  */
 	      next = DF_REF_NEXT_REG (use);
+	      while (next && DF_REF_INSN (next) == insn)
+		next = DF_REF_NEXT_REG (next);
 
 	      if (DEBUG_INSN_P (insn))
 		{
-		  gcc_assert (x == reg
-			      || (GET_CODE (x) == SUBREG
-				  && SUBREG_REG (x) == reg));
-
 		  if (!equiv)
 		    {
 		      INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
 		      df_insn_rescan_debug_internal (insn);
 		    }
 		  else
-		    {
-		      if (x == reg)
-			*loc = copy_rtx (equiv);
-		      else if (GET_CODE (x) == SUBREG
-			       && SUBREG_REG (x) == reg)
-			*loc = simplify_gen_subreg (GET_MODE (x), equiv,
-						    GET_MODE (reg),
-						    SUBREG_BYTE (x));
-		      else
-			gcc_unreachable ();
-		    *loc = wrap_constant (GET_MODE (x), *loc);
-		    }
+		    INSN_VAR_LOCATION_LOC (insn)
+		      = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn),
+					      reg, equiv);
 		}
 	    }
 	}
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2009-10-11 20:06:38.000000000 +0100
+++ gcc/simplify-rtx.c	2009-10-11 20:13:15.000000000 +0100
@@ -350,38 +350,47 @@ simplify_gen_relational (enum rtx_code c
   return gen_rtx_fmt_ee (code, mode, op0, op1);
 }
 \f
-/* Replace all occurrences of OLD_RTX in X with NEW_RTX and try to simplify the
-   resulting RTX.  Return a new RTX which is as simplified as possible.  */
+/* Replace all occurrences of OLD_RTX in X with FN (X', DATA), where X'
+   is an expression in X that is equal to OLD_RTX.  Canonicalize and
+   simplify the result.
+
+   If FN is null, assume FN (X', DATA) == copy_rtx (DATA).  */
 
 rtx
-simplify_replace_rtx (rtx x, const_rtx old_rtx, rtx new_rtx)
+simplify_replace_fn_rtx (rtx x, const_rtx old_rtx,
+			 rtx (*fn) (rtx, void *), void *data)
 {
   enum rtx_code code = GET_CODE (x);
   enum machine_mode mode = GET_MODE (x);
   enum machine_mode op_mode;
   rtx op0, op1, op2;
 
-  /* If X is OLD_RTX, return NEW_RTX.  Otherwise, if this is an expression, try
-     to build a new expression substituting recursively.  If we can't do
-     anything, return our input.  */
+  /* If X is OLD_RTX, return FN (X, DATA), with a null FN.  Otherwise,
+     if this is an expression, try to build a new expression, substituting
+     recursively.  If we can't do anything, return our input.  */
 
   if (rtx_equal_p (x, old_rtx))
-    return copy_rtx (new_rtx);
+    {
+      if (fn)
+	return fn (x, data);
+      else
+	return copy_rtx ((rtx) data);
+    }
 
   switch (GET_RTX_CLASS (code))
     {
     case RTX_UNARY:
       op0 = XEXP (x, 0);
       op_mode = GET_MODE (op0);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
       if (op0 == XEXP (x, 0))
 	return x;
       return simplify_gen_unary (code, mode, op0, op_mode);
 
     case RTX_BIN_ARITH:
     case RTX_COMM_ARITH:
-      op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1))
 	return x;
       return simplify_gen_binary (code, mode, op0, op1);
@@ -391,8 +400,8 @@ simplify_replace_rtx (rtx x, const_rtx o
       op0 = XEXP (x, 0);
       op1 = XEXP (x, 1);
       op_mode = GET_MODE (op0) != VOIDmode ? GET_MODE (op0) : GET_MODE (op1);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (op1, old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (op1, old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1))
 	return x;
       return simplify_gen_relational (code, mode, op_mode, op0, op1);
@@ -401,9 +410,9 @@ simplify_replace_rtx (rtx x, const_rtx o
     case RTX_BITFIELD_OPS:
       op0 = XEXP (x, 0);
       op_mode = GET_MODE (op0);
-      op0 = simplify_replace_rtx (op0, old_rtx, new_rtx);
-      op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
-      op2 = simplify_replace_rtx (XEXP (x, 2), old_rtx, new_rtx);
+      op0 = simplify_replace_fn_rtx (op0, old_rtx, fn, data);
+      op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
+      op2 = simplify_replace_fn_rtx (XEXP (x, 2), old_rtx, fn, data);
       if (op0 == XEXP (x, 0) && op1 == XEXP (x, 1) && op2 == XEXP (x, 2))
 	return x;
       if (op_mode == VOIDmode)
@@ -414,7 +423,7 @@ simplify_replace_rtx (rtx x, const_rtx o
       /* The only case we try to handle is a SUBREG.  */
       if (code == SUBREG)
 	{
-	  op0 = simplify_replace_rtx (SUBREG_REG (x), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (SUBREG_REG (x), old_rtx, fn, data);
 	  if (op0 == SUBREG_REG (x))
 	    return x;
 	  op0 = simplify_gen_subreg (GET_MODE (x), op0,
@@ -427,15 +436,15 @@ simplify_replace_rtx (rtx x, const_rtx o
     case RTX_OBJ:
       if (code == MEM)
 	{
-	  op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
 	  if (op0 == XEXP (x, 0))
 	    return x;
 	  return replace_equiv_address_nv (x, op0);
 	}
       else if (code == LO_SUM)
 	{
-	  op0 = simplify_replace_rtx (XEXP (x, 0), old_rtx, new_rtx);
-	  op1 = simplify_replace_rtx (XEXP (x, 1), old_rtx, new_rtx);
+	  op0 = simplify_replace_fn_rtx (XEXP (x, 0), old_rtx, fn, data);
+	  op1 = simplify_replace_fn_rtx (XEXP (x, 1), old_rtx, fn, data);
 
 	  /* (lo_sum (high x) x) -> x  */
 	  if (GET_CODE (op0) == HIGH && rtx_equal_p (XEXP (op0, 0), op1))
@@ -452,6 +461,15 @@ simplify_replace_rtx (rtx x, const_rtx o
     }
   return x;
 }
+
+/* Replace all occurrences of OLD_RTX in X with NEW_RTX and try to simplify the
+   resulting RTX.  Return a new RTX which is as simplified as possible.  */
+
+rtx
+simplify_replace_rtx (rtx x, const_rtx old_rtx, rtx new_rtx)
+{
+  return simplify_replace_fn_rtx (x, old_rtx, 0, new_rtx);
+}
 \f
 /* Try to simplify a unary operation CODE whose output mode is to be
    MODE with input operand OP whose mode was originally OP_MODE.
@@ -462,9 +480,6 @@ simplify_unary_operation (enum rtx_code 
 {
   rtx trueop, tem;
 
-  if (GET_CODE (op) == CONST)
-    op = XEXP (op, 0);
-
   trueop = avoid_constant_pool_reference (op);
 
   tem = simplify_const_unary_operation (code, mode, trueop, op_mode);
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2009-09-27 10:05:24.000000000 +0100
+++ gcc/var-tracking.c	2009-10-11 20:09:12.000000000 +0100
@@ -6242,24 +6242,6 @@ delete_variable_part (dataflow_set *set,
   slot = delete_slot_part (set, loc, slot, offset);
 }
 
-/* Wrap result in CONST:MODE if needed to preserve the mode.  */
-
-static rtx
-check_wrap_constant (enum machine_mode mode, rtx result)
-{
-  if (!result || GET_MODE (result) == mode)
-    return result;
-
-  if (dump_file && (dump_flags & TDF_DETAILS))
-    fprintf (dump_file, "  wrapping result in const to preserve mode %s\n",
-	     GET_MODE_NAME (mode));
-
-  result = wrap_constant (mode, result);
-  gcc_assert (GET_MODE (result) == mode);
-
-  return result;
-}
-
 /* Callback for cselib_expand_value, that looks for expressions
    holding the value in the var-tracking hash tables.  Return X for
    standard processing, anything else is to be used as-is.  */
@@ -6323,7 +6305,6 @@ vt_expand_loc_callback (rtx x, bitmap re
     {
       result = cselib_expand_value_rtx_cb (loc->loc, regs, max_depth,
 					   vt_expand_loc_callback, vars);
-      result = check_wrap_constant (GET_MODE (loc->loc), result);
       if (result)
 	break;
     }
@@ -6341,14 +6322,11 @@ vt_expand_loc_callback (rtx x, bitmap re
 static rtx
 vt_expand_loc (rtx loc, htab_t vars)
 {
-  rtx newloc;
-
   if (!MAY_HAVE_DEBUG_INSNS)
     return loc;
 
-  newloc = cselib_expand_value_rtx_cb (loc, scratch_regs, 5,
-				       vt_expand_loc_callback, vars);
-  loc = check_wrap_constant (GET_MODE (loc), newloc);
+  loc = cselib_expand_value_rtx_cb (loc, scratch_regs, 5,
+				    vt_expand_loc_callback, vars);
 
   if (loc && MEM_P (loc))
     loc = targetm.delegitimize_address (loc);

Re: Use simplify_replace_rtx rather than wrap_constant

[Bernd Schmidt]

Richard Sandiford wrote:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>> Thanks.  Here's what I installed after bootstrapping and
>> regression-testing on x86_64-linux-gnu.  I'll revise the main
>> patch with the change Paolo suggested and repost.
> 
> And here's the revised main patch.  Bootstrapped & regression-tested
> on x86_64-linux-gnu, and regression-tested on arm-eabi.  OK to install?

OK.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

Re: Use simplify_replace_rtx rather than wrap_constant

[Jakub Jelinek]

On Tue, Oct 20, 2009 at 01:20:06AM +0100, Bernd Schmidt wrote:
> Richard Sandiford wrote:
> > Richard Sandiford <rdsandiford@googlemail.com> writes:
> >> Thanks.  Here's what I installed after bootstrapping and
> >> regression-testing on x86_64-linux-gnu.  I'll revise the main
> >> patch with the change Paolo suggested and repost.
> > 
> > And here's the revised main patch.  Bootstrapped & regression-tested
> > on x86_64-linux-gnu, and regression-tested on arm-eabi.  OK to install?
> 
> OK.

Unfortunately this patch regressed a bunch of guality tests, diff on
testsuite_summary shows on x86_64-linux:
 XPASS: gcc.dg/guality/guality.c  -O3 -g  execution test
 XPASS: gcc.dg/guality/guality.c  -Os  execution test
 XPASS: gcc.dg/guality/inline-params.c  -O0  execution test
-XPASS: gcc.dg/guality/inline-params.c  -O1  execution test
-XPASS: gcc.dg/guality/inline-params.c  -O2  execution test
-XPASS: gcc.dg/guality/inline-params.c  -O3 -fomit-frame-pointer  execution test
-XPASS: gcc.dg/guality/inline-params.c  -O3 -g  execution test
-XPASS: gcc.dg/guality/inline-params.c  -Os  execution test
 XPASS: gcc.dg/guality/pr41353-1.c  -O0  line 28 j == 28 + 37
 XPASS: gcc.dg/guality/pr41447-1.c  -O0  execution test
 XPASS: gcc.dg/guality/pr41616-1.c  -O0  execution test
-XPASS: gcc.dg/guality/pr41616-1.c  -O1  execution test
-XPASS: gcc.dg/guality/pr41616-1.c  -O2  execution test
-XPASS: gcc.dg/guality/pr41616-1.c  -Os  execution test
 XPASS: gcc.dg/struct/wo_prof_array_through_pointer.c scan-ipa-dump ipa_struct_reorg "Number of structures to transform is 1"
and similarly for i686-linux.

For inline-params.c the detailed new state is:
FAIL: b is optimized away, expected 3003
PASS: ab is 6296384
PASS: ac is 6296388
PASS: msg is 4197837
FAIL: 3 PASS, 1 FAIL, 0 UNRESOLVED
XFAIL: gcc.dg/guality/inline-params.c  -O1  execution test
where it used to be:
PASS: b is 3003
PASS: ab is 6296384
PASS: ac is 6296388
PASS: msg is 4197837
PASS: 4 PASS, 0 FAIL, 0 UNRESOLVED
XPASS: gcc.dg/guality/inline-params.c  -O1  execution test
and for pr41616-1.c newly:
FAIL: b is optimized away, expected -1
PASS: b is 1
FAIL: 1 PASS, 1 FAIL, 0 UNRESOLVED
XFAIL: gcc.dg/guality/pr41616-1.c  -O1  execution test
from:
PASS: b is -1
PASS: b is 1
PASS: 2 PASS, 0 FAIL, 0 UNRESOLVED
XPASS: gcc.dg/guality/pr41616-1.c  -O1  execution test

	Jakub

Re: Use simplify_replace_rtx rather than wrap_constant

[Jakub Jelinek]

Hi!

Even trivial testcases like -g -O2:
void foo (void)
{
  int var = -1;
}
regress, so IMHO it is very important regression.
The dumps are identical until the last pass before vartrack, so it is just
vartrack where things broke.

	Jakub

[PATCH] Re: Use simplify_replace_rtx rather than wrap_constant

[Jakub Jelinek]

On Wed, Oct 21, 2009 at 06:36:23PM +0200, Jakub Jelinek wrote:
> Even trivial testcases like -g -O2:
> void foo (void)
> {
>   int var = -1;
> }
> regress, so IMHO it is very important regression.
> The dumps are identical until the last pass before vartrack, so it is just
> vartrack where things broke.

Here is a fix for that I'm going to bootstrap/regtest.  So far just tested
that it fixes this little testcase and all guality.exp regressions.
The second hunk is just a little optimization, it is wasteful to call the
sometimes expensive function when we know we'll ignore what it returns
anyway.

2009-10-21  Jakub Jelinek  <jakub@redhat.com>

	* var-tracking.c (emit_note_insn_var_location): If vt_expand_loc
	returned VOIDmode rtx, get mode from var->var_part[i].loc_chain->loc.
	Don't call the second vt_expand_loc unnecessarily when location is not
	a register nor memory.

--- gcc/var-tracking.c.jj	2009-10-21 18:41:33.000000000 +0200
+++ gcc/var-tracking.c	2009-10-21 18:54:55.000000000 +0200
@@ -6403,6 +6403,8 @@ emit_note_insn_var_location (void **varp
 	}
       loc[n_var_parts] = loc2;
       mode = GET_MODE (loc[n_var_parts]);
+      if (mode == VOIDmode)
+	mode = GET_MODE (var->var_part[i].loc_chain->loc);
       initialized = var->var_part[i].loc_chain->init;
       last_limit = offsets[n_var_parts] + GET_MODE_SIZE (mode);
 
@@ -6413,6 +6415,7 @@ emit_note_insn_var_location (void **varp
 	  break;
       if (j < var->n_var_parts
 	  && wider_mode != VOIDmode
+	  && (REG_P (loc[n_var_parts]) || MEM_P (loc[n_var_parts]))
 	  && (loc2 = vt_expand_loc (var->var_part[j].loc_chain->loc, vars))
 	  && GET_CODE (loc[n_var_parts]) == GET_CODE (loc2)
 	  && mode == GET_MODE (loc2)


	Jakub

Re: Use simplify_replace_rtx rather than wrap_constant

[Ulrich Weigand]

Richard Sandiford wrote:

> Index: gcc/reload1.c
> ===================================================================
> --- gcc/reload1.c	2009-09-27 10:05:24.000000000 +0100
> +++ gcc/reload1.c	2009-10-11 20:09:12.000000000 +0100

> -		    {
> -		      if (x == reg)
> -			*loc = copy_rtx (equiv);
> -		      else if (GET_CODE (x) == SUBREG
> -			       && SUBREG_REG (x) == reg)
> -			*loc = simplify_gen_subreg (GET_MODE (x), equiv,
> -						    GET_MODE (reg),
> -						    SUBREG_BYTE (x));
> -		      else
> -			gcc_unreachable ();
> -		    *loc = wrap_constant (GET_MODE (x), *loc);
> -		    }
> +		    INSN_VAR_LOCATION_LOC (insn)
> +		      = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn),
> +					      reg, equiv);

It seems this change causes an ICE while building libstdc++ on spu-elf.

The problem is that simplify_replace_rtx does not in fact replace all
occurrances of the register in all cases; for example, it does not look
into UNSPEC expressions at all.

In the test case on the SPU, we're seeing a debug_insn like this
before reload:

(debug_insn 1294 1937 1819 136 (var_location:QI __c (unspec:QI [
            (reg:QI 1151)
            (reg:QI 1152)
            (subreg:QI (reg:HI 1129) 1)
        ] 17)) -1 (nil))

Pseudos 1151 and 1152 are eliminated and replaced by constants.
However, because simplify_replace_rtx ignores UNSPECs, the pseudos
survive until after reload:

(debug_insn 1294 1937 2433 136 (var_location:QI __c (unspec:QI [
            (reg:QI 1151)
            (reg:QI 1152)
            (subreg:QI (reg:HI 4 $4 [1129]) 1)
        ] 17)) -1 (nil))

causing an ICE in reload_combine_note_use (postreload.c:1097).


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Jakub Jelinek <jakub@redhat.com> writes:
> On Tue, Oct 20, 2009 at 01:20:06AM +0100, Bernd Schmidt wrote:
>> Richard Sandiford wrote:
>> > Richard Sandiford <rdsandiford@googlemail.com> writes:
>> >> Thanks.  Here's what I installed after bootstrapping and
>> >> regression-testing on x86_64-linux-gnu.  I'll revise the main
>> >> patch with the change Paolo suggested and repost.
>> > 
>> > And here's the revised main patch.  Bootstrapped & regression-tested
>> > on x86_64-linux-gnu, and regression-tested on arm-eabi.  OK to install?
>> 
>> OK.
>
> Unfortunately this patch regressed a bunch of guality tests, diff on
> testsuite_summary shows on x86_64-linux:
>  XPASS: gcc.dg/guality/guality.c  -O3 -g  execution test
>  XPASS: gcc.dg/guality/guality.c  -Os  execution test
>  XPASS: gcc.dg/guality/inline-params.c  -O0  execution test
> -XPASS: gcc.dg/guality/inline-params.c  -O1  execution test
> -XPASS: gcc.dg/guality/inline-params.c  -O2  execution test
> -XPASS: gcc.dg/guality/inline-params.c  -O3 -fomit-frame-pointer  execution test
> -XPASS: gcc.dg/guality/inline-params.c  -O3 -g  execution test
> -XPASS: gcc.dg/guality/inline-params.c  -Os  execution test
>  XPASS: gcc.dg/guality/pr41353-1.c  -O0  line 28 j == 28 + 37
>  XPASS: gcc.dg/guality/pr41447-1.c  -O0  execution test
>  XPASS: gcc.dg/guality/pr41616-1.c  -O0  execution test
> -XPASS: gcc.dg/guality/pr41616-1.c  -O1  execution test
> -XPASS: gcc.dg/guality/pr41616-1.c  -O2  execution test
> -XPASS: gcc.dg/guality/pr41616-1.c  -Os  execution test
>  XPASS: gcc.dg/struct/wo_prof_array_through_pointer.c scan-ipa-dump ipa_struct_reorg "Number of structures to transform is 1"
> and similarly for i686-linux.

Sorry, I hadn't realised these were actually expected passes rather than
unexpected passes.  I'll update my script.

It's the kind of fall-out we were predicting upthread: var-tracking
is using the mode of a simplified result, whereas the "real" mode is
still to hand elsewhere.  The patch below seems to fix the guality.exp
regressions; I'll give it a full test tomorrow.

As far as the second hunk goes, checking the mode first seems faster too.
I hope I'm not missing some subtle reason why the checks have to be
in their current order...

Richard


gcc/
	* var-tracking.c (emit_note_insn_var_location): Get the mode of
	a variable part from its REG, MEM or VALUE.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2009-10-21 20:59:55.000000000 +0100
+++ gcc/var-tracking.c	2009-10-21 21:00:01.000000000 +0100
@@ -6402,7 +6402,7 @@ emit_note_insn_var_location (void **varp
 	  continue;
 	}
       loc[n_var_parts] = loc2;
-      mode = GET_MODE (loc[n_var_parts]);
+      mode = GET_MODE (var->var_part[i].loc_chain->loc);
       initialized = var->var_part[i].loc_chain->init;
       last_limit = offsets[n_var_parts] + GET_MODE_SIZE (mode);
 
@@ -6413,9 +6413,9 @@ emit_note_insn_var_location (void **varp
 	  break;
       if (j < var->n_var_parts
 	  && wider_mode != VOIDmode
+	  && mode == GET_MODE (var->var_part[j].loc_chain->loc)
 	  && (loc2 = vt_expand_loc (var->var_part[j].loc_chain->loc, vars))
 	  && GET_CODE (loc[n_var_parts]) == GET_CODE (loc2)
-	  && mode == GET_MODE (loc2)
 	  && last_limit == var->var_part[j].offset)
 	{
 	  rtx new_loc = NULL;
Index: gcc/config/m32r/m32r.c
===================================================================
--- gcc/config/m32r/m32r.c	2009-10-21 20:59:55.000000000 +0100
+++ gcc/config/m32r/m32r.c	2009-10-21 21:00:01.000000000 +0100
@@ -1212,6 +1212,7 @@ m32r_setup_incoming_varargs (CUMULATIVE_
 m32r_is_insn (rtx insn)
 {
   return (NONDEBUG_INSN_P (insn)
+	  && !DEBUG_INSN_P (insn)
 	  && GET_CODE (PATTERN (insn)) != USE
 	  && GET_CODE (PATTERN (insn)) != CLOBBER
 	  && GET_CODE (PATTERN (insn)) != ADDR_VEC);
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2009-10-21 20:59:55.000000000 +0100
+++ gcc/config/mips/mips.c	2009-10-21 21:00:01.000000000 +0100
@@ -9533,7 +9533,10 @@ mips_restore_gp_from_cprestore_slot (rtx
   gcc_assert (TARGET_ABICALLS && TARGET_OLDABI && epilogue_completed);
 
   if (!cfun->machine->must_restore_gp_when_clobbered_p)
-    return;
+    {
+      emit_note (NOTE_INSN_DELETED);
+      return;
+    }
 
   if (TARGET_MIPS16)
     {
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp	2009-10-21 20:59:55.000000000 +0100
+++ gcc/testsuite/lib/target-supports.exp	2009-10-21 21:00:01.000000000 +0100
@@ -1568,6 +1568,7 @@ proc check_effective_target_arm_thumb2_o
 # otherwise.  Cache the result.
 
 proc check_effective_target_arm_neon_hw { } {
+    return 0
     return [check_runtime arm_neon_hw_available {
 	int
 	main (void)

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Richard Sandiford wrote:
>
>> Index: gcc/reload1.c
>> ===================================================================
>> --- gcc/reload1.c	2009-09-27 10:05:24.000000000 +0100
>> +++ gcc/reload1.c	2009-10-11 20:09:12.000000000 +0100
>
>> -		    {
>> -		      if (x == reg)
>> -			*loc = copy_rtx (equiv);
>> -		      else if (GET_CODE (x) == SUBREG
>> -			       && SUBREG_REG (x) == reg)
>> -			*loc = simplify_gen_subreg (GET_MODE (x), equiv,
>> -						    GET_MODE (reg),
>> -						    SUBREG_BYTE (x));
>> -		      else
>> -			gcc_unreachable ();
>> -		    *loc = wrap_constant (GET_MODE (x), *loc);
>> -		    }
>> +		    INSN_VAR_LOCATION_LOC (insn)
>> +		      = simplify_replace_rtx (INSN_VAR_LOCATION_LOC (insn),
>> +					      reg, equiv);
>
> It seems this change causes an ICE while building libstdc++ on spu-elf.
>
> The problem is that simplify_replace_rtx does not in fact replace all
> occurrances of the register in all cases; for example, it does not look
> into UNSPEC expressions at all.
>
> In the test case on the SPU, we're seeing a debug_insn like this
> before reload:
>
> (debug_insn 1294 1937 1819 136 (var_location:QI __c (unspec:QI [
>             (reg:QI 1151)
>             (reg:QI 1152)
>             (subreg:QI (reg:HI 1129) 1)
>         ] 17)) -1 (nil))
>
> Pseudos 1151 and 1152 are eliminated and replaced by constants.
> However, because simplify_replace_rtx ignores UNSPECs, the pseudos
> survive until after reload:
>
> (debug_insn 1294 1937 2433 136 (var_location:QI __c (unspec:QI [
>             (reg:QI 1151)
>             (reg:QI 1152)
>             (subreg:QI (reg:HI 4 $4 [1129]) 1)
>         ] 17)) -1 (nil))
>
> causing an ICE in reload_combine_note_use (postreload.c:1097).

Sorry for the breakage.  I think most callers of simplify_rtx
will require all references to be replaced, so I think handling
UNSPEC and UNSPEC_VOLATILE would be the right fix.  Do you have
a preprocessed testcase I could try on a cross-compiler?
Alternatively, could you give the patch below a spin?
I'll bootstrap it on x86_64-linux-gnu in combination with
the patch I sent Jakub.

Richard

PS. I notice there are still some copying concerns in this function.


gcc/
	* simplify-rtx.c (simplify_replace_fn_rtx): Handle UNSPECs
	and UNSPEC_VOLATILEs too.

Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2009-10-21 21:07:46.000000000 +0100
+++ gcc/simplify-rtx.c	2009-10-21 21:07:49.000000000 +0100
@@ -420,7 +420,6 @@ simplify_replace_fn_rtx (rtx x, const_rt
       return simplify_gen_ternary (code, mode, op_mode, op0, op1, op2);
 
     case RTX_EXTRA:
-      /* The only case we try to handle is a SUBREG.  */
       if (code == SUBREG)
 	{
 	  op0 = simplify_replace_fn_rtx (SUBREG_REG (x), old_rtx, fn, data);
@@ -431,6 +430,36 @@ simplify_replace_fn_rtx (rtx x, const_rt
 				     SUBREG_BYTE (x));
 	  return op0 ? op0 : x;
 	}
+      else if (code == UNSPEC || code == UNSPEC_VOLATILE)
+	{
+	  rtx newx, op;
+	  int i, j;
+
+	  newx = x;
+	  for (i = 0; i < XVECLEN (x, 0); i++)
+	    {
+	      op = simplify_replace_fn_rtx (XVECEXP (x, 0, i),
+					    old_rtx, fn, data);
+	      /* See if this is the first operand that differs.
+		 Create a copy of everything up to index I if so.  */
+	      if (op != XVECEXP (x, 0, i) && newx == x)
+		{
+		  newx = shallow_copy_rtx (x);
+		  XVEC (newx, 0) = rtvec_alloc (XVECLEN (newx, 0));
+		  for (j = 0; j < i; j++)
+		    XVECEXP (newx, 0, j) = copy_rtx (XVECEXP (x, 0, j));
+		}
+	      /* If we're building a new vector, initialize index I.  */
+	      if (newx != x)
+		{
+		  if (op != XVECEXP (x, 0, i))
+		    XVECEXP (newx, 0, i) = op;
+		  else
+		    XVECEXP (newx, 0, i) = copy_rtx (op);
+		}
+	    }
+	  return newx;
+	}
       break;
 
     case RTX_OBJ:

Re: Use simplify_replace_rtx rather than wrap_constant

[Jakub Jelinek]

On Wed, Oct 21, 2009 at 09:12:50PM +0100, Richard Sandiford wrote:
> 	* simplify-rtx.c (simplify_replace_fn_rtx): Handle UNSPECs
> 	and UNSPEC_VOLATILEs too.

I think we shouldn't special case UNSPEC*.  There might be something else
too (e.g. CONCAT, CONCATN, CONST, PRE_DEC, ...).
If this routine is to be used, it IMHO should just in the case that isn't
already handled go through the format string and for any 'e's there
simplify_replace_fn_rtx the operand, if it returns something different
shallow_copy_rtx x unless already done.  Similarly for 'E'.

In your handling of UNSPEC, I don't understand the copy_rtx, I think
currently simplify_replace_rtx doesn't guarantee 100% unsharing of
everything, so I think you should unshare only the operands where recursive
call actually changed something.

	Jakub

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, Oct 21, 2009 at 09:12:50PM +0100, Richard Sandiford wrote:
>> 	* simplify-rtx.c (simplify_replace_fn_rtx): Handle UNSPECs
>> 	and UNSPEC_VOLATILEs too.
>
> I think we shouldn't special case UNSPEC*.  There might be something else
> too (e.g. CONCAT, CONCATN, CONST, PRE_DEC, ...).
> If this routine is to be used, it IMHO should just in the case that isn't
> already handled go through the format string and for any 'e's there
> simplify_replace_fn_rtx the operand, if it returns something different
> shallow_copy_rtx x unless already done.  Similarly for 'E'.

Yeah, good point.

> In your handling of UNSPEC, I don't understand the copy_rtx, I think
> currently simplify_replace_rtx doesn't guarantee 100% unsharing of
> everything, so I think you should unshare only the operands where recursive
> call actually changed something.

That was the reason for my PS comment.  I think the inconsistent
sharing is actually a bug, but I don't want to lump too many changes
together.

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Jakub Jelinek <jakub@redhat.com> writes:
>> In your handling of UNSPEC, I don't understand the copy_rtx, I think
>> currently simplify_replace_rtx doesn't guarantee 100% unsharing of
>> everything, so I think you should unshare only the operands where recursive
>> call actually changed something.
>
> That was the reason for my PS comment.  I think the inconsistent
> sharing is actually a bug, but I don't want to lump too many changes
> together.

OTOH, I guess the other simplify_* functions are really only suitable
for cases where you either keep the original expression or the new one,
not both.  Sorry for the noise. ;(

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Index: gcc/config/m32r/m32r.c
> ===================================================================
> --- gcc/config/m32r/m32r.c	2009-10-21 20:59:55.000000000 +0100
> +++ gcc/config/m32r/m32r.c	2009-10-21 21:00:01.000000000 +0100
> @@ -1212,6 +1212,7 @@ m32r_setup_incoming_varargs (CUMULATIVE_
>  m32r_is_insn (rtx insn)
>  {
>    return (NONDEBUG_INSN_P (insn)
> +	  && !DEBUG_INSN_P (insn)
>  	  && GET_CODE (PATTERN (insn)) != USE
>  	  && GET_CODE (PATTERN (insn)) != CLOBBER
>  	  && GET_CODE (PATTERN (insn)) != ADDR_VEC);
> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c	2009-10-21 20:59:55.000000000 +0100
> +++ gcc/config/mips/mips.c	2009-10-21 21:00:01.000000000 +0100
> @@ -9533,7 +9533,10 @@ mips_restore_gp_from_cprestore_slot (rtx
>    gcc_assert (TARGET_ABICALLS && TARGET_OLDABI && epilogue_completed);
>  
>    if (!cfun->machine->must_restore_gp_when_clobbered_p)
> -    return;
> +    {
> +      emit_note (NOTE_INSN_DELETED);
> +      return;
> +    }
>  
>    if (TARGET_MIPS16)
>      {
> Index: gcc/testsuite/lib/target-supports.exp
> ===================================================================
> --- gcc/testsuite/lib/target-supports.exp	2009-10-21 20:59:55.000000000 +0100
> +++ gcc/testsuite/lib/target-supports.exp	2009-10-21 21:00:01.000000000 +0100
> @@ -1568,6 +1568,7 @@ proc check_effective_target_arm_thumb2_o
>  # otherwise.  Cache the result.
>  
>  proc check_effective_target_arm_neon_hw { } {
> +    return 0
>      return [check_runtime arm_neon_hw_available {
>  	int
>  	main (void)

And as you will have guessed, this lot wasn't supposed to be there.
Tonight really hasn't been my night.

Richard

Re: Use simplify_replace_rtx rather than wrap_constant

[Ulrich Weigand]

Richard Sandiford wrote:

> Sorry for the breakage.  I think most callers of simplify_rtx
> will require all references to be replaced, so I think handling
> UNSPEC and UNSPEC_VOLATILE would be the right fix.  Do you have
> a preprocessed testcase I could try on a cross-compiler?
> Alternatively, could you give the patch below a spin?

Thanks for the quick fix!  The patch does indeed fix the ICE
when building libstdc++ for me.

(If you want, I can send also you a preprocessed test case offline;
as usual for reload issues I haven't been able to reduce it much.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: Use simplify_replace_rtx rather than wrap_constant

[H.J. Lu]

On Sat, Sep 26, 2009 at 1:01 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>   - It now copies the "to" rtx for each replacement.  All but one caller
>>     seemed to pass an already-used rtx as the "to" value, so current
>>     substitutions could create invalid sharing.  I've removed the
>>     now-redundant copy_rtx from the one caller that used it.
>
> Except I messed up the quilt, so that gcse.c change wasn't part of the
> patch I posted.  Here's what I meant to post (and actually tested).
>
> Richard
>
>
> gcc/
>        * rtl.h (simplify_replace_fn_rtx): Declare.
>        (wrap_constant, unwrap_constant): Delete.
>        * cfgexpand.c (unwrap_constant, wrap_constant): Delete.
>        (expand_debug_expr): Don't call wrap_constant.
>        * combine.c (rtx_subst_pair): Only define for AUTO_INC_DEC.
>        (auto_adjust_pair): Fold into...
>        (propagate_for_debug_subst): ...here.  Only define for AUTO_INC_DEC.
>        Just return a new value.
>        (propagate_for_debug): Use simplify_replace_fn_rtx for AUTO_INC_DEC,
>        otherwise use simplify_replace_rtx.
>        * cselib.c (wrap_constant): Reinstate old definition.
>        (cselib_expand_value_rtx_1): Don't wrap constants.
>        * gcse.c (try_replace_reg): Don't use copy_rtx in the call to
>        simplify_replace_rtx.
>        (bypass_block): Fix formatting in calls to simplify_replace_rtx.
>        * reload1.c (reload): Skip all uses for an insn before adjusting it.
>        Use simplify_replace_rtx.
>        * simplify-rtx.c (simplify_replace_fn_rtx): New function, adapted from...
>        (simplify_replace_rtx): ...here.  Turn into a wrapper for
>        simplify_replace_fn_rtx.
>        (simplify_unary_operation): Don't unwrap CONSTs.
>        * var-tracking.c (check_wrap_constant): Delete.
>        (vt_expand_loc_callback): Don't call it.
>        (vt_expand_loc): Likewise.
>

This patch caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42662


H.J.