[PATCH] Fix reload bugs w.r.t. optional address operand reloads

[Ulrich Weigand <weigand at immd1 dot informatik dot uni-erlangen dot de>]

[Mail re-sent because it apparently didn't get through ...]

Hello,

this fixes two reload bugs that caused an ICE on s390.  For an initial
discussion of the bugs see the thread:
  http://gcc.gnu.org/ml/gcc-patches/2002-11/msg01371.html

As pointed out by Joern Rennecke, the test case triggered a combination
of two bugs in reload.  This patch fixes both of them (either bug, when
fixed alone, lets the test case pass).

The first problem is that in some cases, there can be optional reloads 
generated for address operands *after* the initial find_reloads_address 
pass.  In those cases, reload would use the operand mode specified in 
the insn pattern as reload mode.  This is incorrect for address operands,
where the operand mode has a different meaning.  I had already fixed
that case for the EXTRA_ADDRESS_OPERAND constraints, but it can also
occur for the regular 'p' constraint.  This patch uses the original 
operand mode in all cases as the reload mode for address operands.

The second problem is that in most of those cases, it was actually
incorrect to generate the optional reload, because we in fact had
already pushed a mandatory reload from inside find_reloads_address
for the whole operand.  Joern Rennecke suggested to detect this
situation using find_replacement.  However, this does not work as
find_replacement assumes the actual reload registers have already
been filled in, which is not the case during find_reloads.  Thus,
I've added a new routine have_replacement_p, which simply checks
whether there is any replacement scheduled at a specified location.
For every operand where this holds true, no optional reloads are
generated.

Even this new function can only work on the final pass through
find_reloads, because the initial passes do not remember the 
replacement locations at all.  However, as far as I can see,
the initial passes should not need to care about optional
reloads in the first place; the only thing calculate_needs_all_insn
does with optional reloads is to ignore them.  Thus, I've changed
find_reloads to simply not generate optional reloads at all 
unless the REPLACE parameter is true.

Bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux,
both on mainline and the 3.3 branch.

OK to commit to mainline and 3.3?



ChangeLog:

gcc/
	* reload.c (find_reloads): Do not use the mode specified in the insn
	pattern as reload mode for address operands.  Do not generate optional 
	reloads for operands where a mandatory reload was already pushed.
	Generate optional reloads only in the final pass though find_reloads.
	(have_replacement_p): New function.

gcc/testsuite/
	* gcc.dg/20030129-1.c: New test.


Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.199.2.1
diff -c -p -r1.199.2.1 reload.c
*** gcc/reload.c	25 Jan 2003 23:59:25 -0000	1.199.2.1
--- gcc/reload.c	27 Jan 2003 17:31:23 -0000
*************** static void find_reloads_address_part PA
*** 272,277 ****
--- 272,278 ----
  static rtx find_reloads_subreg_address PARAMS ((rtx, int, int,
  						enum reload_type, int, rtx));
  static void copy_replacements_1 PARAMS ((rtx *, rtx *, int));
+ static bool have_replacement_p	PARAMS ((rtx *));
  static int find_inc_amount	PARAMS ((rtx, rtx));
  
  #ifdef HAVE_SECONDARY_RELOADS
*************** find_reloads (insn, replace, ind_levels,
*** 2685,2690 ****
--- 2686,2695 ----
  
  	  recog_data.operand[i] = *recog_data.operand_loc[i];
  	  substed_operand[i] = recog_data.operand[i];
+ 
+ 	  /* Address operands are reloaded in their existing mode,
+ 	     no matter what is specified in the machine description.  */
+ 	  operand_mode[i] = GET_MODE (recog_data.operand[i]);
  	}
        else if (code == MEM)
  	{
*************** find_reloads (insn, replace, ind_levels,
*** 3284,3293 ****
  			   the address into a base register.  */
  			this_alternative[i] = (int) MODE_BASE_REG_CLASS (VOIDmode);
  			badop = 0;
- 
- 			/* Address constraints are reloaded in Pmode, no matter
- 			   what mode is given in the machine description.  */
- 			operand_mode[i] = Pmode;
  			break;
  		      }
  
--- 3289,3294 ----
*************** find_reloads (insn, replace, ind_levels,
*** 3855,3863 ****
  	    return 0;
  	  }
        }
      else if (goal_alternative_matched[i] < 0
! 	     && goal_alternative_matches[i] < 0
! 	     && optimize)
        {
  	/* For each non-matching operand that's a MEM or a pseudo-register
  	   that didn't get a hard register, make an optional reload.
--- 3856,3875 ----
  	    return 0;
  	  }
        }
+ 
+     /* Generate optional reloads only when optimizing, and only
+        on the last pass through reload.  Also, make sure we do not
+        make an optional reload where we already have a mandatory
+        one; this can happen in the case of address operands.
+ 
+        To check for mandatory reloads, we use have_replacement_p.
+        Note that this works only on the last pass through reload.  */
+     else if (!optimize || !replace 
+ 	     || have_replacement_p (recog_data.operand_loc[i]))
+       ; /* Do nothing.  */
+ 
      else if (goal_alternative_matched[i] < 0
! 	     && goal_alternative_matches[i] < 0)
        {
  	/* For each non-matching operand that's a MEM or a pseudo-register
  	   that didn't get a hard register, make an optional reload.
*************** find_reloads (insn, replace, ind_levels,
*** 3907,3917 ****
  	   reload, check if this is actually a pseudo register reference;
  	   we then need to emit a USE and/or a CLOBBER so that reload
  	   inheritance will do the right thing.  */
! 	else if (replace
! 		 && (GET_CODE (operand) == MEM
! 		     || (GET_CODE (operand) == REG
! 			 && REGNO (operand) >= FIRST_PSEUDO_REGISTER
! 			 && reg_renumber [REGNO (operand)] < 0)))
  	  {
  	    operand = *recog_data.operand_loc[i];
  
--- 3919,3928 ----
  	   reload, check if this is actually a pseudo register reference;
  	   we then need to emit a USE and/or a CLOBBER so that reload
  	   inheritance will do the right thing.  */
! 	else if (GET_CODE (operand) == MEM
! 		 || (GET_CODE (operand) == REG
! 		     && REGNO (operand) >= FIRST_PSEUDO_REGISTER
! 		     && reg_renumber [REGNO (operand)] < 0))
  	  {
  	    operand = *recog_data.operand_loc[i];
  
*************** find_reloads (insn, replace, ind_levels,
*** 3934,3941 ****
  	     && goal_alternative_win[goal_alternative_matches[i]]
  	     && modified[i] == RELOAD_READ
  	     && modified[goal_alternative_matches[i]] == RELOAD_WRITE
! 	     && ! no_input_reloads && ! no_output_reloads
! 	     && optimize)
        {
  	/* Similarly, make an optional reload for a pair of matching
  	   objects that are in MEM or a pseudo that didn't get a hard reg.  */
--- 3945,3951 ----
  	     && goal_alternative_win[goal_alternative_matches[i]]
  	     && modified[i] == RELOAD_READ
  	     && modified[goal_alternative_matches[i]] == RELOAD_WRITE
! 	     && ! no_input_reloads && ! no_output_reloads)
        {
  	/* Similarly, make an optional reload for a pair of matching
  	   objects that are in MEM or a pseudo that didn't get a hard reg.  */
*************** find_replacement (loc)
*** 6107,6112 ****
--- 6117,6137 ----
      }
  
    return *loc;
+ }
+ 
+ /* Return true if some replacement was scheduled at LOC.  */
+ 
+ static bool
+ have_replacement_p (loc)
+      rtx *loc;
+ {
+   struct replacement *r;
+ 
+   for (r = &replacements[0]; r < &replacements[n_replacements]; r++)
+     if (r->where == loc)
+       return true;
+ 
+   return false;
  }
  
  /* Return nonzero if register in range [REGNO, ENDREGNO)
*** /dev/null	Thu Sep 19 11:33:10 2002
--- gcc/testsuite/gcc.dg/20030129-1.c	Wed Jan 29 14:52:45 2003
***************
*** 0 ****
--- 1,37 ----
+ /* This used to ICE due to a reload bug on s390*.  */
+ 
+ /* { dg-do compile { target s390*-*-* } } */
+ /* { dg-options "-O2" } */
+ 
+ int f (unsigned int);
+ void g (void *);
+ 
+ void test (void *p, void *dummy)
+ {
+   unsigned int flags = 0;
+ 
+   if (dummy)
+     g (dummy);
+ 
+   if (p)
+     flags |= 0x80000000;
+ 
+   asm volatile ("" : : : "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12");
+ 
+   if (dummy)
+     g (dummy);
+ 
+   if (p) 
+     {
+       flags |= 0x20000000|0x80000000;
+ 
+       if (!f (0))
+         flags &= ~0x80000000;
+     }
+ 
+   f (flags);
+ 
+   if (dummy)
+     g (dummy);
+ }
+ 

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de