Re: PR middle-end/30191 (was: [PATCH] Fix PR middle-end/28690, indexed load/store performance + reload bug)

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Michael Matz wrote:

> Unfortunately that's not the whole truth, and sometimes reload does change 
> the insn structure which theoretically would require a recog.  One could 
> require for all changes imaginable by reload that the resulting insns need 
> to be recognizable (after reload) if they were before.  But that is even 
> harder to check a priori :-/  And it doesn't help you if the new insn form 
> doesn't have the same number of operands or constraint content (as without 
> rerecognition the insn code will still be the old one and hence reload 
> would look at constraint and operand places of the old insn form).  Tough.

The following patch attempts to work both in situations where we should
re-recognize in order to change the insn code, and in situations where we
cannot re-recognize because the intermediate form of the insn is not 
accepted by the target predicate.

I'm now trying the following steps when substituing the SRC of a single_set:
- Just replace the SET_SRC and validate; if OK, fine.
- Replace the full PATTERN with a standard gen_rtx_SET pattern and validate;
  if OK, fine.  This should catch cases like on powerpc, where we want to
  replace a load-with-update with unused result by a simple add.
- If neither form can be validated, simply replace the SET_SRC without
  validating and assume reload will fix it.

This should work on all platforms that work today, and fix the powerpc
problem.  (We shouldn't even need the reload_in_progress change in the
rs6000 back end any more.)

Michael, what do you think about this approach?

Peter, could you try whether it indeed fixes your original problem?

I've bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux,
and built and regtested spu-elf without regressions.

Bye,
Ulrich


Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	(revision 119844)
+++ gcc/reload1.c	(working copy)
@@ -3098,35 +3098,15 @@
 	    if (GET_CODE (XEXP (plus_cst_src, 0)) == SUBREG)
 	      to_rtx = gen_lowpart (GET_MODE (XEXP (plus_cst_src, 0)),
 				    to_rtx);
-	    if (offset == 0)
-	      {
-		int num_clobbers;
-		/* We assume here that if we need a PARALLEL with
-		   CLOBBERs for this assignment, we can do with the
-		   MATCH_SCRATCHes that add_clobbers allocates.
-		   There's not much we can do if that doesn't work.  */
-		PATTERN (insn) = gen_rtx_SET (VOIDmode,
-					      SET_DEST (old_set),
-					      to_rtx);
-		num_clobbers = 0;
-		INSN_CODE (insn) = recog (PATTERN (insn), insn, &num_clobbers);
-		if (num_clobbers)
-		  {
-		    rtvec vec = rtvec_alloc (num_clobbers + 1);
-
-		    vec->elem[0] = PATTERN (insn);
-		    PATTERN (insn) = gen_rtx_PARALLEL (VOIDmode, vec);
-		    add_clobbers (PATTERN (insn), INSN_CODE (insn));
-		  }
-		gcc_assert (INSN_CODE (insn) >= 0);
-	      }
 	    /* If we have a nonzero offset, and the source is already
 	       a simple REG, the following transformation would
 	       increase the cost of the insn by replacing a simple REG
 	       with (plus (reg sp) CST).  So try only when we already
 	       had a PLUS before.  */
-	    else if (plus_src)
+	    if (offset == 0 || plus_src)
 	      {
+		rtx new_src = plus_constant (to_rtx, offset);
+
 		new_body = old_body;
 		if (! replace)
 		  {
@@ -3137,8 +3117,20 @@
 		PATTERN (insn) = new_body;
 		old_set = single_set (insn);
 
-		XEXP (SET_SRC (old_set), 0) = to_rtx;
-		XEXP (SET_SRC (old_set), 1) = GEN_INT (offset);
+		/* First see if this insn remains valid when we make the
+		   change.  If not, try to replace the whole pattern with
+		   a simple set (this may help if the original insn was a
+		   PARALLEL that was only recognized as single_set due to 
+		   REG_UNUSED notes).  If this isn't valid either, keep
+		   the INSN_CODE the same and let reload fix it up.  */
+		if (!validate_change (insn, &SET_SRC (old_set), new_src, 0))
+		  {
+		    rtx new_pat = gen_rtx_SET (VOIDmode,
+					       SET_DEST (old_set), new_src);
+
+		    if (!validate_change (insn, &PATTERN (insn), new_pat, 0))
+		      SET_SRC (old_set) = new_src;
+		  }
 	      }
 	    else
 	      break;


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