[PATCH] Fix find_reloads_address bug, take 2

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

I wrote originally:

>Secondly, if the condition hits, the code in question re-forms
>the address from
>  (plus (plus frame-pointer index) constant)
>to
>  (plus (plus frame-pointer constant) index)
>which is non-canonical RTL.  That would not be a problem if
>-as intended- the (plus frame-pointer constant) term got
>reloaded into a register.  However, find_reloads is called
>multiple times, and only after the last call will actual
>reloads be made.  Unfortunatly, this code segment will
>*modify* the passed-in address RTX in-place on *every*
>call of find_reloads.  This means after the return from
>the first find_reloads call out of calculate_needs_all_insns,
>the main insn stream will contain this piece of invalid RTL.

While this is correct, it actually does not really matter.
*If* the condition when to enter this block were correct,
the constant term would be invalid for an address.  This
can only happen if that constant was in fact introduced
by eliminate_regs_in_insn just before the call to find_reloads.

In that case, it does not matter that the address is
modified in place, as calculate_needs_all_insns will
throw away the insn body anway to get rid to the changes
done by eliminate_regs.

The reason why I am seeing the ICE on s390 is that this
block is entered -due to the incorrect condition- even
for addresses where the constant term is valid; such
addresses might have been in the insn stream even before
register elimination, and thus calculate_needs_all_insns
will not undo the modification ...

Thus, the only bug that needs to be fixed is in fact
the incorrect condition; the rest should be fine as is.

The following patch implements this, and also fixes a
stupid bug in the original patch: the lines 
!          && ! maybe_memory_address_p (mode, ad, &XEXP (XEXP (ad, 0), 0)))
and
!          && ! maybe_memory_address_p (mode, ad, &XEXP (XEXP (ad, 0), 1)))
need to be swapped, of course.

Also, I've included a test case that shows the ICE on s390.

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

OK to apply?  Should this be considered for 3.2 as well?


ChangeLog:

gcc/
        * reload.c (maybe_memory_address_p): New function.
        (find_reloads_address): Use it instead of memory_address_p.

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


Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.178.2.4.2.4
diff -c -p -r1.178.2.4.2.4 reload.c
*** gcc/reload.c	24 Oct 2002 08:59:49 -0000	1.178.2.4.2.4
--- gcc/reload.c	23 Jan 2003 20:21:16 -0000
*************** static int alternative_allows_memconst P
*** 257,262 ****
--- 257,263 ----
  static rtx find_reloads_toplev	PARAMS ((rtx, int, enum reload_type, int,
  					 int, rtx, int *));
  static rtx make_memloc		PARAMS ((rtx, int));
+ static int maybe_memory_address_p PARAMS ((enum machine_mode, rtx, rtx *));
  static int find_reloads_address	PARAMS ((enum machine_mode, rtx *, rtx, rtx *,
  				       int, enum reload_type, int, rtx));
  static rtx subst_reg_equivs	PARAMS ((rtx, rtx));
*************** make_memloc (ad, regno)
*** 4545,4550 ****
--- 4546,4572 ----
    return tem;
  }
  
+ /* Returns true if AD could be turned into a valid memory reference
+    to mode MODE by reloading the part pointed to by PART into a 
+    register.  */
+ 
+ static int
+ maybe_memory_address_p (mode, ad, part)
+      enum machine_mode mode;
+      rtx ad;
+      rtx *part;
+ {
+   int retv;
+   rtx tem = *part;
+   rtx reg = gen_rtx_REG (GET_MODE (tem), max_reg_num ());
+ 
+   *part = reg;
+   retv = memory_address_p (mode, ad);
+   *part = tem;
+ 
+   return retv;
+ }
+ 
  /* Record all reloads needed for handling memory address AD
     which appears in *LOC in a memory reference to mode MODE
     which itself is found in location  *MEMREFLOC.
*************** find_reloads_address (mode, memrefloc, a
*** 4844,4850 ****
  	       || XEXP (XEXP (ad, 0), 0) == arg_pointer_rtx
  #endif
  	       || XEXP (XEXP (ad, 0), 0) == stack_pointer_rtx)
! 	   && ! memory_address_p (mode, ad))
      {
        *loc = ad = gen_rtx_PLUS (GET_MODE (ad),
  				plus_constant (XEXP (XEXP (ad, 0), 0),
--- 4866,4872 ----
  	       || XEXP (XEXP (ad, 0), 0) == arg_pointer_rtx
  #endif
  	       || XEXP (XEXP (ad, 0), 0) == stack_pointer_rtx)
! 	   && ! maybe_memory_address_p (mode, ad, &XEXP (XEXP (ad, 0), 1)))
      {
        *loc = ad = gen_rtx_PLUS (GET_MODE (ad),
  				plus_constant (XEXP (XEXP (ad, 0), 0),
*************** find_reloads_address (mode, memrefloc, a
*** 4869,4875 ****
  	       || XEXP (XEXP (ad, 0), 1) == arg_pointer_rtx
  #endif
  	       || XEXP (XEXP (ad, 0), 1) == stack_pointer_rtx)
! 	   && ! memory_address_p (mode, ad))
      {
        *loc = ad = gen_rtx_PLUS (GET_MODE (ad),
  				XEXP (XEXP (ad, 0), 0),
--- 4891,4897 ----
  	       || XEXP (XEXP (ad, 0), 1) == arg_pointer_rtx
  #endif
  	       || XEXP (XEXP (ad, 0), 1) == stack_pointer_rtx)
! 	   && ! maybe_memory_address_p (mode, ad, &XEXP (XEXP (ad, 0), 0)))
      {
        *loc = ad = gen_rtx_PLUS (GET_MODE (ad),
  				XEXP (XEXP (ad, 0), 0),
*** /dev/null	Thu Sep 19 11:33:10 2002
--- gcc/testsuite/gcc.dg/20030123-1.c	Thu Jan 23 21:27:33 2003
***************
*** 0 ****
--- 1,17 ----
+ /* This used to ICE due to a reload bug on s390*.  */
+ 
+ /* { dg-do compile { target s390*-*-* } } */
+ /* { dg-options "-O2" } */
+ 
+ void func (char *p);
+ 
+ void test (void)
+ {
+    char *p = alloca (4096);
+    long idx;
+ 
+    asm ("" : "=r" (idx) : : "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12");
+ 
+    func (p + idx + 1);
+ }
+ 


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