This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Unreviewed patch


Roger Sayle wrote:

> Would it be OK for ask you to bootstrap and regression test this patch
> on at least one other platform?  May be x86 and/or powerpc?  As you'll
> appreciate accepting patches to reload is a risky business, even before
> stage3.

Sure.  I don't have ready access to powerpc, but I'll test the patch
on x86 and ia64, if that's OK with you.  (Testing will take at least
until tomorrow, though.)

> With the exception of setting goal_alternative_win, this hunk
> looks like a obvious extension of the non-PLUS case, but I'm not
> confident enough to believe it's totally safe without some additional
> testing.

Setting goal_alternative_win would be wrong: if we have *just* a constant,
and this gets forced to the pool, and the insn directly accepts a memory
operand, we need no reload at all any more, so goal_alternative_win can
be set to true.  In the *new* case, however, even after we forced the
constant, we still have to compute the PLUS, so we still require a reload.

I found another place where simply copying the constant case was wrong:
in the 'just a constant case', it makes sense to force the constant 
even though it would be OK, if we cannot allow input reloads (that's
the || no_input_reload case).  In the PLUS case, we require the reload
in any case, so no_input_reload must not be true anyway, and the test
is redundant and probably misleading, so I've removed it now.

The following patch shows this modified version of the patch, together
with the s390-specific parts that remove the workarounds no longer
required after reload it fixed.  I'm currently in the progress of
bootstrapping and regression testing that patch on s390-ibm-linux,
s390x-ibm-linux, i686-pc-linux, and ia64-unknown-linux.

OK if testing is successful everywhere?

Bye,
Ulrich


Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.256
diff -c -p -r1.256 reload.c
*** gcc/reload.c	12 Oct 2004 19:28:55 -0000	1.256
--- gcc/reload.c	13 Oct 2004 19:14:27 -0000
*************** find_reloads (rtx insn, int replace, int
*** 3778,3783 ****
--- 3778,3804 ----
  	  goal_alternative_win[i] = 1;
        }
  
+   /* Likewise any invalid constants appearing as operand of a PLUS
+      that is to be reloaded.  */
+   for (i = 0; i < noperands; i++)
+     if (! goal_alternative_win[i]
+ 	&& GET_CODE (recog_data.operand[i]) == PLUS
+ 	&& CONST_POOL_OK_P (XEXP (recog_data.operand[i], 1))
+ 	&& (PREFERRED_RELOAD_CLASS (XEXP (recog_data.operand[i], 1),
+ 				    (enum reg_class) goal_alternative[i])
+ 	     == NO_REGS)
+ 	&& operand_mode[i] != VOIDmode)
+       {
+ 	rtx tem = force_const_mem (operand_mode[i],
+ 				   XEXP (recog_data.operand[i], 1));
+ 	tem = gen_rtx_PLUS (operand_mode[i],
+ 			    XEXP (recog_data.operand[i], 0), tem);
+ 
+ 	substed_operand[i] = recog_data.operand[i]
+ 	  = find_reloads_toplev (tem, i, address_type[i],
+ 				 ind_levels, 0, insn, NULL);
+       }
+ 
    /* Record the values of the earlyclobber operands for the caller.  */
    if (goal_earlyclobber)
      for (i = 0; i < noperands; i++)
Index: gcc/config/s390/s390.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.c,v
retrieving revision 1.193
diff -c -p -r1.193 s390.c
*** gcc/config/s390/s390.c	12 Oct 2004 20:37:15 -0000	1.193
--- gcc/config/s390/s390.c	13 Oct 2004 19:14:28 -0000
*************** s390_secondary_input_reload_class (enum 
*** 2274,2290 ****
  				   enum machine_mode mode, rtx in)
  {
    if (s390_plus_operand (in, mode))
!     {
!       /* ??? Reload sometimes pushes a PLUS reload with a too-large constant.
! 	 Until reload is fixed, we need to force_const_mem while emitting the
! 	 secondary reload insn -- thus we need to make sure here that we do
! 	 have a literal pool for the current function.  */
!       if (CONSTANT_P (XEXP (in, 1))
! 	  && !legitimate_reload_constant_p (XEXP (in, 1)))
! 	current_function_uses_const_pool = true;
! 
!       return ADDR_REGS;
!     }
  
    return NO_REGS;
  }
--- 2274,2280 ----
  				   enum machine_mode mode, rtx in)
  {
    if (s390_plus_operand (in, mode))
!     return ADDR_REGS;
  
    return NO_REGS;
  }
*************** s390_expand_plus_operand (register rtx t
*** 2366,2375 ****
  	}
        if (true_regnum (sum2) < 1 || true_regnum (sum2) > 15)
  	{
- 	  /* ??? See comment in s390_secondary_input_reload_class.  */
- 	  if (CONSTANT_P (sum2) && !legitimate_reload_constant_p (sum2))
- 	    sum2 = force_const_mem (Pmode, sum2);
- 
  	  emit_move_insn (scratch, sum2);
  	  sum2 = scratch;
  	}
--- 2356,2361 ----


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]