[PATCH] Fix target/15286 (reload of paradoxical subregs)

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

Hello,

PR target/15286 is caused by several bugs related to processing
pardoxical SUBREGs during reload.

The situation is triggered by RTL like

 (set (subreg:DI (reg/v:SI 279 [ clock_start ]) 0)
      (fix:DI (reg:DF 294)))

where the insn (on rs6000) requires a floating point register
as output (i.e. to hold the DImode value).

If the target pseudo does not get assigned a hard register,
reload needs to generate an output reload for this insn.
Usually, this output reload would be performed in SImode;
but in this case, since CANNOT_CHANGE_MODE_CLASS forbids
DImode -> SImode subregs for floating point registers,
find_reloads correctly decides the reload needs to be
performed in DImode.

Generally speaking, performing a reload on a stack pseudo
using a wider mode *is* supported; if this happens, reload
makes sure the stack slot assigned to the pseudo is wide
enough and properly aligned for the widest paradoxical
subreg used for such purposes (see scan_paradoxical_subregs).

However, as paradoxical subreg reload can usually be avoided,
it would appear the related code paths have fallen into disuse
and no longer work quite as expected.  We've found three
distinct bugs that need to be fixed to get the test case to
work properly.

First of all, there is code in gen_reload that tries to avoid
paradoxical subreg reloads at the last minute.  While this is
generally fine, it must not be done at the expense of introducing
subregs forbidden by CANNOT_CHANGE_MODE_CLASS.

Now, gen_reload uses the return value of gen_lowpart_common to
decide whether the transformed subreg is valid.  However, that
function calls simplify_gen_subreg which in turn calls simplify_subreg.
This last function has code to verify using CANNOT_CHANGE_MODE_CLASS
that no invalid mode change is being encoded into a SUBREG.  However,
in *simplify_gen_subreg*, that decision is subsequently simply ignored:

  newx = simplify_subreg (outermode, op, innermode, byte);
  if (newx)
    return newx;

  if (GET_CODE (op) == SUBREG || GET_MODE (op) == VOIDmode)
    return NULL_RTX;

  return gen_rtx_SUBREG (outermode, op, byte);

I'm proposing to fix this by having simplify_gen_subreg fail if
simplify_subreg fails when called for a hard register, similarly
to how it already fails if simplify_subreg fails when called for
another SUBREG.  In both cases, if simplify_subreg fails it means
that the requested mode change is invalid, and thus we shouldn't
IMO generate an explicit SUBREG either.


The second problem is that now with the first problem fixed,
gen_reload does generate an insn accessing a paradoxical SUBREG
of a stack slot.  Now this is supposed to be cleaned up to a
normal MEM with adjusted address by cleanup_subreg_operands.
However, that function must first recognize the insn, which 
fails because general_operand categorically rejects paradoxical
subregs of memory operands (at least if INSN_SCHEDULING).

To fix this, I propose to relax that check after reload to allow
cleanup_subreg_operands to work; this is just like what is done
in the immediately subsequent check in general_operand.


The third problem is that once the insn is accepted, alter_subreg
actually handles parodoxical subregs on big-endian machines
incorrectly: in this case the required address offset is *not*
equal to SUBREG_BYTE; see the comments in simplify_subreg addressing
that issue.

This is fixed simply by using the same code to determine proper
address offsets as is used in simplify_subreg.


With those three problems fixes, the test case passes on Darwin.
The patch was bootstrapped and regtested on apple-ppc-darwin
by Fariborz Jahanian.

OK for mainline?

Bye,
Ulrich

ChangeLog:

	PR target/15286
	* final.c (alter_subreg): Compute correct offset to use with
	paradoxical SUBREGs of memory operands.
	* recog.c (general_operand): Allow paradoxical SUBREGs of
	memory operands after reload.
	* simplify-rtx.c (simplify_gen_subreg): Fail if simplify_subreg
	has failed when passed a hard register.

Index: gcc/final.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/final.c,v
retrieving revision 1.339
diff -c -p -r1.339 final.c
*** gcc/final.c	22 Oct 2004 17:05:05 -0000	1.339
--- gcc/final.c	27 Oct 2004 14:17:37 -0000
*************** alter_subreg (rtx *xp)
*** 2607,2613 ****
    /* simplify_subreg does not remove subreg from volatile references.
       We are required to.  */
    if (MEM_P (y))
!     *xp = adjust_address (y, GET_MODE (x), SUBREG_BYTE (x));
    else
      {
        rtx new = simplify_subreg (GET_MODE (x), y, GET_MODE (y),
--- 2607,2630 ----
    /* simplify_subreg does not remove subreg from volatile references.
       We are required to.  */
    if (MEM_P (y))
!     {
!       int offset = SUBREG_BYTE (x);
! 
!       /* For paradoxical subregs on big-endian machines, SUBREG_BYTE
! 	 contains 0 instead of the proper offset.  See simplify_subreg.  */
!       if (offset == 0
! 	  && GET_MODE_SIZE (GET_MODE (y)) < GET_MODE_SIZE (GET_MODE (x)))
!         {
!           int difference = GET_MODE_SIZE (GET_MODE (y))
! 			   - GET_MODE_SIZE (GET_MODE (x));
!           if (WORDS_BIG_ENDIAN)
!             offset += (difference / UNITS_PER_WORD) * UNITS_PER_WORD;
!           if (BYTES_BIG_ENDIAN)
!             offset += difference % UNITS_PER_WORD;
!         }
! 
!       *xp = adjust_address (y, GET_MODE (x), offset);
!     }
    else
      {
        rtx new = simplify_subreg (GET_MODE (x), y, GET_MODE (y),
Index: gcc/recog.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.211
diff -c -p -r1.211 recog.c
*** gcc/recog.c	28 Sep 2004 07:59:48 -0000	1.211
--- gcc/recog.c	27 Oct 2004 14:17:38 -0000
*************** general_operand (rtx op, enum machine_mo
*** 936,943 ****
  
  #ifdef INSN_SCHEDULING
        /* On machines that have insn scheduling, we want all memory
! 	 reference to be explicit, so outlaw paradoxical SUBREGs.  */
!       if (MEM_P (sub)
  	  && GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (sub)))
  	return 0;
  #endif
--- 936,945 ----
  
  #ifdef INSN_SCHEDULING
        /* On machines that have insn scheduling, we want all memory
! 	 reference to be explicit, so outlaw paradoxical SUBREGs.
! 	 However, we must allow them after reload so that they can
! 	 get cleaned up by cleanup_subreg_operands.  */
!       if (!reload_completed && MEM_P (sub)
  	  && GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (sub)))
  	return 0;
  #endif
Index: gcc/simplify-rtx.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
retrieving revision 1.206
diff -c -p -r1.206 simplify-rtx.c
*** gcc/simplify-rtx.c	18 Oct 2004 18:46:06 -0000	1.206
--- gcc/simplify-rtx.c	27 Oct 2004 14:17:38 -0000
*************** simplify_gen_subreg (enum machine_mode o
*** 3789,3795 ****
    if (newx)
      return newx;
  
!   if (GET_CODE (op) == SUBREG || GET_MODE (op) == VOIDmode)
      return NULL_RTX;
  
    return gen_rtx_SUBREG (outermode, op, byte);
--- 3789,3796 ----
    if (newx)
      return newx;
  
!   if (GET_CODE (op) == SUBREG || GET_MODE (op) == VOIDmode
!       || (REG_P (op) && REGNO (op) < FIRST_PSEUDO_REGISTER))
      return NULL_RTX;
  
    return gen_rtx_SUBREG (outermode, op, byte);
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de