[PATCH] Fix LEGITIMIZE_RELOAD_ADDRESS vs. EXTRA_MEMORY_CONSTRAINT problem

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

Hello,

when trying to add LEGITIMIZE_RELOAD_ADDRESS for s390 I noticed that
this actually don't work correctly for targets with multiple addressing
modes.

The problem is the following:  the routine find_reloads_address returns
an integer specifying whether it replaced or reloaded the address
*as a whole*, i.e. at the top level.  This information is used in 
different places in find_reloads: 

- for predecrement/postincrement operands: '<'/'>' constraints match
  a PRE_INC/POST_DEC pattern only if the address was *not* replaced
  at top level

- for non-standard (i.e. offsettable or EXTRA_MEMORY_CONTRAINT)
  addresses: if the address *was* already reloaded at the top level,
  then it certainly matches the non-standard memory constraint

- for address operands: if they were *not* already reloaded, we need
  to make an optional reload or mark the operand otherwise available
  for reload inheritance via a USE


Now, one of the cases in find_reloads_address calls the target macro
LEGITIMIZE_RELOAD_ADDRESS to attempt to fix up the address in a 
target-specific way.   However, if that macro succeeds, the common
code does not actually *know* whether the target replaced the address
at the top level (in many cases it will only reload a subexpression).
So the question is, what to return from find_reloads_address?

The current behaviour is to return always 1, apparently intended
as conservative guess.  However, this is really only the correct
conservative behaviour for the PRE_INC/POST_DEC pattern check.
For the non-"m" memory constraints, this is definitely wrong:
here the conservative assumption would be that the address is
*not* reloaded at top level.  I'm not completly sure what the
conservative behaviour for the optional reloads would be, but it
looks like the current behaviour is incorrect (better we have an
optional reload and a regular reload than we have nothing and
screw up reload inheritance).


The question is now, how to fix this.  One way would be to actually
find out the correct answer to the question whether the top level
was reloaded even for LEGITIMIZE_RELOAD_ADDRESS.  However, the
only way I can see to do that would be to add a corresponding
argument to the macro and update all targets that use it ...

As a simpler solution I propose to make the return value of
find_reloads_address tri-state: definitely reloaded at toplevel,
definitely *not* reloaded at toplevel, and maybe reloaded at
toplevel.  Then every user of that flag can choose the 
conservative behaviour in the 'maybe' case that appropriate
for that particular situation.

The following patch implements this, together with an actual
implementation of LEGITIMIZE_RELOAD_ADDRESS for s390(x).

Bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux.
OK?

Bye,
Ulrich

ChangeLog:

	* reload.c (find_reloads_address): Make return value tri-state.
	Return -1 if LEGITIMIZE_RELOAD_ADDRESS succeeded.
	(find_reloads): Assume that reloaded addresses match 'o' or
	EXTRA_MEMORY_CONSTRAINT constraints only if find_reloads_address
	returned 1 (not -1).  Omit optional reloads for address operands
	only if find_reloads_address returned 1 (not -1).

	* config/s390/s390.c (legitimize_reload_address): New function.
	* config/s390/s390-protos.h (legitimize_reload_address): Declare.
	* config/s390/s390.h (LEGITIMIZE_RELOAD_ADDRESS): Define.  Call
	legitimize_reload_address.


Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.246
diff -c -p -r1.246 reload.c
*** gcc/reload.c	9 Jul 2004 03:29:34 -0000	1.246
--- gcc/reload.c	15 Jul 2004 19:10:18 -0000
*************** find_reloads (rtx insn, int replace, int
*** 2485,2493 ****
       a register.  */
    enum reg_class preferred_class[MAX_RECOG_OPERANDS];
    char pref_or_nothing[MAX_RECOG_OPERANDS];
!   /* Nonzero for a MEM operand whose entire address needs a reload.  */
    int address_reloaded[MAX_RECOG_OPERANDS];
!   /* Nonzero for an address operand that needs to be completely reloaded.  */
    int address_operand_reloaded[MAX_RECOG_OPERANDS];
    /* Value of enum reload_type to use for operand.  */
    enum reload_type operand_type[MAX_RECOG_OPERANDS];
--- 2485,2495 ----
       a register.  */
    enum reg_class preferred_class[MAX_RECOG_OPERANDS];
    char pref_or_nothing[MAX_RECOG_OPERANDS];
!   /* Nonzero for a MEM operand whose entire address needs a reload. 
!      May be -1 to indicate the entire address may or may not need a reload.  */
    int address_reloaded[MAX_RECOG_OPERANDS];
!   /* Nonzero for an address operand that needs to be completely reloaded.
!      May be -1 to indicate the entire operand may or may not need a reload.  */
    int address_operand_reloaded[MAX_RECOG_OPERANDS];
    /* Value of enum reload_type to use for operand.  */
    enum reload_type operand_type[MAX_RECOG_OPERANDS];
*************** find_reloads (rtx insn, int replace, int
*** 3180,3186 ****
  			  : offsettable_nonstrict_memref_p (operand))
  			 /* A reloaded address is offsettable because it is now
  			    just a simple register indirect.  */
! 			 || address_reloaded[i]))
  		    || (REG_P (operand)
  			&& REGNO (operand) >= FIRST_PSEUDO_REGISTER
  			&& reg_renumber[REGNO (operand)] < 0
--- 3182,3188 ----
  			  : offsettable_nonstrict_memref_p (operand))
  			 /* A reloaded address is offsettable because it is now
  			    just a simple register indirect.  */
! 			 || address_reloaded[i] == 1))
  		    || (REG_P (operand)
  			&& REGNO (operand) >= FIRST_PSEUDO_REGISTER
  			&& reg_renumber[REGNO (operand)] < 0
*************** find_reloads (rtx insn, int replace, int
*** 3296,3302 ****
  			/* If the address was already reloaded,
  			   we win as well.  */
  			else if (MEM_P (operand)
! 				 && address_reloaded[i])
  			  win = 1;
  			/* Likewise if the address will be reloaded because
  			   reg_equiv_address is nonzero.  For reg_equiv_mem
--- 3298,3304 ----
  			/* If the address was already reloaded,
  			   we win as well.  */
  			else if (MEM_P (operand)
! 				 && address_reloaded[i] == 1)
  			  win = 1;
  			/* Likewise if the address will be reloaded because
  			   reg_equiv_address is nonzero.  For reg_equiv_mem
*************** find_reloads (rtx insn, int replace, int
*** 3896,3902 ****
        }
      else if (goal_alternative_matched[i] < 0
  	     && goal_alternative_matches[i] < 0
! 	     && !address_operand_reloaded[i]
  	     && optimize)
        {
  	/* For each non-matching operand that's a MEM or a pseudo-register
--- 3898,3904 ----
        }
      else if (goal_alternative_matched[i] < 0
  	     && goal_alternative_matches[i] < 0
! 	     && address_operand_reloaded[i] != 1
  	     && optimize)
        {
  	/* For each non-matching operand that's a MEM or a pseudo-register
*************** maybe_memory_address_p (enum machine_mod
*** 4636,4643 ****
     to determine if we may generate output reloads, and where to put USEs
     for pseudos that we have to replace with stack slots.
  
!    Value is nonzero if this address is reloaded or replaced as a whole.
!    This is interesting to the caller if the address is an autoincrement.
  
     Note that there is no verification that the address will be valid after
     this routine does its work.  Instead, we rely on the fact that the address
--- 4638,4646 ----
     to determine if we may generate output reloads, and where to put USEs
     for pseudos that we have to replace with stack slots.
  
!    Value is one if this address is reloaded or replaced as a whole; it is
!    zero if the top level of this address was not reloaded or replaced, and
!    it is -1 if it may or may not have been reloaded or replaced.
  
     Note that there is no verification that the address will be valid after
     this routine does its work.  Instead, we rely on the fact that the address
*************** find_reloads_address (enum machine_mode 
*** 4775,4781 ****
        *memrefloc = copy_rtx (*memrefloc);
        XEXP (*memrefloc, 0) = ad;
        move_replacements (&ad, &XEXP (*memrefloc, 0));
!       return 1;
      }
    while (0);
  #endif
--- 4778,4784 ----
        *memrefloc = copy_rtx (*memrefloc);
        XEXP (*memrefloc, 0) = ad;
        move_replacements (&ad, &XEXP (*memrefloc, 0));
!       return -1;
      }
    while (0);
  #endif
Index: gcc/config/s390/s390-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390-protos.h,v
retrieving revision 1.51
diff -c -p -r1.51 s390-protos.h
*** gcc/config/s390/s390-protos.h	7 Jul 2004 19:24:45 -0000	1.51
--- gcc/config/s390/s390-protos.h	15 Jul 2004 19:10:19 -0000
*************** extern int legitimate_reload_constant_p 
*** 63,68 ****
--- 63,69 ----
  extern int legitimate_address_p (enum machine_mode, rtx, int);
  extern rtx legitimize_pic_address (rtx, rtx);
  extern rtx legitimize_address (rtx, rtx, enum machine_mode);
+ extern rtx legitimize_reload_address (rtx, enum machine_mode, int, int);
  extern enum reg_class s390_preferred_reload_class (rtx, enum reg_class);
  extern enum reg_class s390_secondary_input_reload_class (enum reg_class,
  							 enum machine_mode,
Index: gcc/config/s390/s390.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.c,v
retrieving revision 1.158
diff -c -p -r1.158 s390.c
*** gcc/config/s390/s390.c	13 Jul 2004 17:22:41 -0000	1.158
--- gcc/config/s390/s390.c	15 Jul 2004 19:10:19 -0000
*************** legitimize_address (register rtx x, regi
*** 3038,3043 ****
--- 3038,3089 ----
    return x;
  }
  
+ /* Try a machine-dependent way of reloading an illegitimate address AD
+    operand.  If we find one, push the reload and and return the new address.
+ 
+    MODE is the mode of the enclosing MEM.  OPNUM is the operand number
+    and TYPE is the reload type of the current reload.  */
+ 
+ rtx 
+ legitimize_reload_address (rtx ad, enum machine_mode mode ATTRIBUTE_UNUSED,
+ 			   int opnum, int type)
+ {
+   if (!optimize || TARGET_LONG_DISPLACEMENT)
+     return NULL_RTX;
+ 
+   if (GET_CODE (ad) == PLUS)
+     {
+       rtx tem = simplify_binary_operation (PLUS, Pmode,
+ 					   XEXP (ad, 0), XEXP (ad, 1));
+       if (tem)
+ 	ad = tem;
+     }
+ 
+   if (GET_CODE (ad) == PLUS
+       && GET_CODE (XEXP (ad, 0)) == REG
+       && GET_CODE (XEXP (ad, 1)) == CONST_INT
+       && !DISP_IN_RANGE (INTVAL (XEXP (ad, 1))))
+     {
+       HOST_WIDE_INT lower = INTVAL (XEXP (ad, 1)) & 0xfff;
+       HOST_WIDE_INT upper = INTVAL (XEXP (ad, 1)) ^ lower;
+       rtx cst, tem, new;
+ 
+       cst = GEN_INT (upper);
+       if (!legitimate_reload_constant_p (cst))
+ 	cst = force_const_mem (Pmode, cst);
+ 
+       tem = gen_rtx_PLUS (Pmode, XEXP (ad, 0), cst);
+       new = gen_rtx_PLUS (Pmode, tem, GEN_INT (lower));
+ 
+       push_reload (XEXP (tem, 1), 0, &XEXP (tem, 1), 0,
+ 		   BASE_REG_CLASS, Pmode, VOIDmode, 0, 0, 
+ 		   opnum, (enum reload_type) type);
+       return new;
+     }
+ 
+   return NULL_RTX;
+ }
+ 
  /* Emit code to move LEN bytes from DST to SRC.  */
  
  void
Index: gcc/config/s390/s390.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.h,v
retrieving revision 1.109
diff -c -p -r1.109 s390.h
*** gcc/config/s390/s390.h	5 Jul 2004 19:49:17 -0000	1.109
--- gcc/config/s390/s390.h	15 Jul 2004 19:10:19 -0000
*************** CUMULATIVE_ARGS;
*** 793,798 ****
--- 793,811 ----
      goto WIN;                                                           \
  }
  
+ /* Try a machine-dependent way of reloading an illegitimate address
+    operand.  If we find one, push the reload and jump to WIN.  This
+    macro is used in only one place: `find_reloads_address' in reload.c.  */
+ #define LEGITIMIZE_RELOAD_ADDRESS(AD, MODE, OPNUM, TYPE, IND, WIN)	\
+ do {									\
+   rtx new = legitimize_reload_address (AD, MODE, OPNUM, (int)(TYPE));	\
+   if (new)								\
+     {									\
+       (AD) = new;							\
+       goto WIN;								\
+     }									\
+ } while (0)
+ 
  /* Nonzero if the constant value X is a legitimate general operand.
     It is given that X satisfies CONSTANT_P or is a CONST_DOUBLE.  */
  #define LEGITIMATE_CONSTANT_P(X) \
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de