Re: GCC 3.1 Prerelease

[Mark Mitchell <mark@codesourcery.com>]

--On Sunday, April 21, 2002 02:46:52 PM +0200 Toon Moene 
<toon@moene.indiv.nluug.nl> wrote:

> John David Anglin wrote:
>
>> Fortran:
>>
>> PR6138: Incorrect access of interger*1 variables on PA.

> The problem is hard to track down - already the .f.00.rtl dump is wrong
> when optimizing (another hint that it might be a bug in the frontend -
> however, that means that we have to explain how the frontend can
> generate different RTL based on optimization flags) ...
>
> Affects powerpc and hppa.

Often, the change between optimizing and non-optimizing comes up in
fixup_var_refs -- and that is the culprit here.  (When optimizing,
we try to keep things in registers, and only dump them to the stack
when necessary.)

Before fixup_var_refs, we have:

(insn 12 10 14 (set (reg/v:SI 115)
        (const_int -9 [0xfffffffffffffff7])) -1 (nil)
    (nil))

(insn 17 15 19 (set (reg/v:SI 116)
        (sign_extend:SI (subreg:QI (reg/v:SI 115) 3))) -1 (nil)
    (nil))

Insn 17 is the one that gets screwed up.  What happens is that
we replace (reg/v:SI 115) with (mem/f:HI (reg/f:SI 111 virtual-stack-vars))
because the original mode of the variable was HImode, not SImode.  (We
created an SImode register because we thought that would be more
efficient.)

So, after substitution, we have, in insn 17:

  (set (reg/v:SI 116)
       (sign_extend:SI (subreg:QI (mem/f:HI ...) 3)))

This is bogus; we no longer want byte 3 of the HI mode mem, we want
byte 1 of the HI mode mem.

Here's a patch which I think will fix the problem.  I'm going to
test this on i686-pc-linux-gnu, but I'd appreciate it if

a) someone knowledgeable would sanity check the patch, and

b) someone would bootstrap/test on a PowerPC or HPPA platform, which
   we know is susceptible to these issues.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

2002-04-21  Mark Mitchell  <mark@codesourcery.com>

	PR f/6318.
	* function.c (fixup_memory_subreg): Add promoted_mode parameter.
	(walk_fixup_memory_subreg): Likewise.
	(fixup_var_refs_insn): Adjust accordingly.
	(fixup_var_refs_1): Likewise.

Index: function.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.c,v
retrieving revision 1.347.2.4
diff -c -p -r1.347.2.4 function.c
*** function.c	12 Apr 2002 11:37:42 -0000	1.347.2.4
--- function.c	21 Apr 2002 18:36:43 -0000
*************** static void fixup_var_refs_insn PARAMS (
*** 252,259 ****
  					 int, int, rtx));
  static void fixup_var_refs_1	PARAMS ((rtx, enum machine_mode, rtx *, rtx,
  					 struct fixup_replacement **, rtx));
! static rtx fixup_memory_subreg	PARAMS ((rtx, rtx, int));
! static rtx walk_fixup_memory_subreg  PARAMS ((rtx, rtx, int));
  static rtx fixup_stack_1	PARAMS ((rtx, rtx));
  static void optimize_bit_field	PARAMS ((rtx, rtx, rtx *));
  static void instantiate_decls	PARAMS ((tree, int));
--- 252,260 ----
  					 int, int, rtx));
  static void fixup_var_refs_1	PARAMS ((rtx, enum machine_mode, rtx *, rtx,
  					 struct fixup_replacement **, rtx));
! static rtx fixup_memory_subreg	PARAMS ((rtx, rtx, enum machine_mode, 
int));
! static rtx walk_fixup_memory_subreg  PARAMS ((rtx, rtx, enum 
machine_mode,
! 					      int));
  static rtx fixup_stack_1	PARAMS ((rtx, rtx));
  static void optimize_bit_field	PARAMS ((rtx, rtx, rtx *));
  static void instantiate_decls	PARAMS ((tree, int));
*************** fixup_var_refs_insn (insn, var, promoted
*** 1868,1874 ****
  	      /* OLD might be a (subreg (mem)).  */
  	      if (GET_CODE (replacements->old) == SUBREG)
  		replacements->old
! 		  = fixup_memory_subreg (replacements->old, insn, 0);
  	      else
  		replacements->old
  		  = fixup_stack_1 (replacements->old, insn);
--- 1869,1876 ----
  	      /* OLD might be a (subreg (mem)).  */
  	      if (GET_CODE (replacements->old) == SUBREG)
  		replacements->old
! 		  = fixup_memory_subreg (replacements->old, insn,
! 					 promoted_mode, 0);
  	      else
  		replacements->old
  		  = fixup_stack_1 (replacements->old, insn);
*************** fixup_var_refs_insn (insn, var, promoted
*** 1908,1914 ****
      {
        if (GET_CODE (note) != INSN_LIST)
  	XEXP (note, 0)
! 	  = walk_fixup_memory_subreg (XEXP (note, 0), insn, 1);
        note = XEXP (note, 1);
      }
  }
--- 1910,1917 ----
      {
        if (GET_CODE (note) != INSN_LIST)
  	XEXP (note, 0)
! 	  = walk_fixup_memory_subreg (XEXP (note, 0), insn,
! 				      promoted_mode, 1);
        note = XEXP (note, 1);
      }
  }
*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2079,2085 ****
  		  return;
  		}
  	      else
! 		tem = fixup_memory_subreg (tem, insn, 0);
  	    }
  	  else
  	    tem = fixup_stack_1 (tem, insn);
--- 2082,2088 ----
  		  return;
  		}
  	      else
! 		tem = fixup_memory_subreg (tem, insn, promoted_mode, 0);
  	    }
  	  else
  	    tem = fixup_stack_1 (tem, insn);
*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2194,2200 ****
  	      return;
  	    }

! 	  replacement->new = *loc = fixup_memory_subreg (x, insn, 0);

  	  INSN_CODE (insn) = -1;
  	  if (! flag_force_mem && recog_memoized (insn) >= 0)
--- 2197,2204 ----
  	      return;
  	    }

! 	  replacement->new = *loc = fixup_memory_subreg (x, insn,
! 							 promoted_mode, 0);

  	  INSN_CODE (insn) = -1;
  	  if (! flag_force_mem && recog_memoized (insn) >= 0)
*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2285,2291 ****
  	       This was legitimate when the MEM was a REG.  */
  	    if (GET_CODE (tem) == SUBREG
  		&& SUBREG_REG (tem) == var)
! 	      tem = fixup_memory_subreg (tem, insn, 0);
  	    else
  	      tem = fixup_stack_1 (tem, insn);

--- 2289,2295 ----
  	       This was legitimate when the MEM was a REG.  */
  	    if (GET_CODE (tem) == SUBREG
  		&& SUBREG_REG (tem) == var)
! 	      tem = fixup_memory_subreg (tem, insn, promoted_mode, 0);
  	    else
  	      tem = fixup_stack_1 (tem, insn);

*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2387,2393 ****
  		  SET_SRC (x) = replacement->new;
  		else if (GET_CODE (SET_SRC (x)) == SUBREG)
  		  SET_SRC (x) = replacement->new
! 		    = fixup_memory_subreg (SET_SRC (x), insn, 0);
  		else
  		  SET_SRC (x) = replacement->new
  		    = fixup_stack_1 (SET_SRC (x), insn);
--- 2391,2398 ----
  		  SET_SRC (x) = replacement->new;
  		else if (GET_CODE (SET_SRC (x)) == SUBREG)
  		  SET_SRC (x) = replacement->new
! 		    = fixup_memory_subreg (SET_SRC (x), insn, promoted_mode,
! 					   0);
  		else
  		  SET_SRC (x) = replacement->new
  		    = fixup_stack_1 (SET_SRC (x), insn);
*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2440,2446 ****
  	    rtx pat, last;

  	    if (GET_CODE (SET_DEST (x)) == SUBREG)
! 	      SET_DEST (x) = fixup_memory_subreg (SET_DEST (x), insn, 0);
  	    else
  	      SET_DEST (x) = fixup_stack_1 (SET_DEST (x), insn);

--- 2445,2452 ----
  	    rtx pat, last;

  	    if (GET_CODE (SET_DEST (x)) == SUBREG)
! 	      SET_DEST (x) = fixup_memory_subreg (SET_DEST (x), insn,
! 						  promoted_mode, 0);
  	    else
  	      SET_DEST (x) = fixup_stack_1 (SET_DEST (x), insn);

*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2492,2498 ****
  	    /* Convert (SUBREG (MEM)) to a MEM in a changed mode.  */
  	    if (GET_CODE (fixeddest) == SUBREG)
  	      {
! 		fixeddest = fixup_memory_subreg (fixeddest, insn, 0);
  		promoted_mode = GET_MODE (fixeddest);
  	      }
  	    else
--- 2498,2505 ----
  	    /* Convert (SUBREG (MEM)) to a MEM in a changed mode.  */
  	    if (GET_CODE (fixeddest) == SUBREG)
  	      {
! 		fixeddest = fixup_memory_subreg (fixeddest, insn,
! 						 promoted_mode, 0);
  		promoted_mode = GET_MODE (fixeddest);
  	      }
  	    else
*************** fixup_var_refs_1 (var, promoted_mode, lo
*** 2531,2554 ****
      }
  }
  \f
! /* Given X, an rtx of the form (SUBREG:m1 (MEM:m2 addr)),
!    return an rtx (MEM:m1 newaddr) which is equivalent.
!    If any insns must be emitted to compute NEWADDR, put them before INSN.

     UNCRITICAL nonzero means accept paradoxical subregs.
     This is used for subregs found inside REG_NOTES.  */

  static rtx
! fixup_memory_subreg (x, insn, uncritical)
       rtx x;
       rtx insn;
       int uncritical;
  {
!   int offset = SUBREG_BYTE (x);
    rtx addr = XEXP (SUBREG_REG (x), 0);
    enum machine_mode mode = GET_MODE (x);
    rtx result;

    /* Paradoxical SUBREGs are usually invalid during RTL generation.  */
    if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))
        && ! uncritical)
--- 2538,2569 ----
      }
  }
  \f
! /* Previously, X had the form (SUBREG:m1 (REG:PROMOTED_MODE ...)).
!    The REG  was placed on the stack, so X now has the form (SUBREG:m1
!    (MEM:m2 ...)).
!
!    Return an rtx (MEM:m1 newaddr) which is equivalent.  If any insns
!    must be emitted to compute NEWADDR, put them before INSN.

     UNCRITICAL nonzero means accept paradoxical subregs.
     This is used for subregs found inside REG_NOTES.  */

  static rtx
! fixup_memory_subreg (x, insn, promoted_mode, uncritical)
       rtx x;
       rtx insn;
+      enum machine_mode promoted_mode;
       int uncritical;
  {
!   int offset;
    rtx addr = XEXP (SUBREG_REG (x), 0);
    enum machine_mode mode = GET_MODE (x);
    rtx result;

+   offset = (SUBREG_BYTE (x)
+ 	    - (GET_MODE_SIZE (promoted_mode)
+ 	       - GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))));
+
    /* Paradoxical SUBREGs are usually invalid during RTL generation.  */
    if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (SUBREG_REG (x)))
        && ! uncritical)
*************** fixup_memory_subreg (x, insn, uncritical
*** 2571,2584 ****
     If X itself is a (SUBREG (MEM ...) ...), return the replacement 
expression.
     Otherwise return X, with its contents possibly altered.

!    If any insns must be emitted to compute NEWADDR, put them before INSN.
!
!    UNCRITICAL is as in fixup_memory_subreg.  */

  static rtx
! walk_fixup_memory_subreg (x, insn, uncritical)
       rtx x;
       rtx insn;
       int uncritical;
  {
    enum rtx_code code;
--- 2586,2599 ----
     If X itself is a (SUBREG (MEM ...) ...), return the replacement 
expression.
     Otherwise return X, with its contents possibly altered.

!    INSN, PROMOTED_MODE and UNCRITICAL are as for
!    fixup_memory_subreg.  */

  static rtx
! walk_fixup_memory_subreg (x, insn, promoted_mode, uncritical)
       rtx x;
       rtx insn;
+      enum machine_mode promoted_mode;
       int uncritical;
  {
    enum rtx_code code;
*************** walk_fixup_memory_subreg (x, insn, uncri
*** 2591,2597 ****
    code = GET_CODE (x);

    if (code == SUBREG && GET_CODE (SUBREG_REG (x)) == MEM)
!     return fixup_memory_subreg (x, insn, uncritical);

    /* Nothing special about this RTX; fix its operands.  */

--- 2606,2612 ----
    code = GET_CODE (x);

    if (code == SUBREG && GET_CODE (SUBREG_REG (x)) == MEM)
!     return fixup_memory_subreg (x, insn, promoted_mode, uncritical);

    /* Nothing special about this RTX; fix its operands.  */

*************** walk_fixup_memory_subreg (x, insn, uncri
*** 2599,2611 ****
    for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
      {
        if (fmt[i] == 'e')
! 	XEXP (x, i) = walk_fixup_memory_subreg (XEXP (x, i), insn, uncritical);
        else if (fmt[i] == 'E')
  	{
  	  int j;
  	  for (j = 0; j < XVECLEN (x, i); j++)
  	    XVECEXP (x, i, j)
! 	      = walk_fixup_memory_subreg (XVECEXP (x, i, j), insn, uncritical);
  	}
      }
    return x;
--- 2614,2628 ----
    for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
      {
        if (fmt[i] == 'e')
! 	XEXP (x, i) = walk_fixup_memory_subreg (XEXP (x, i), insn,
! 						promoted_mode, uncritical);
        else if (fmt[i] == 'E')
  	{
  	  int j;
  	  for (j = 0; j < XVECLEN (x, i); j++)
  	    XVECEXP (x, i, j)
! 	      = walk_fixup_memory_subreg (XVECEXP (x, i, j), insn,
! 					  promoted_mode, uncritical);
  	}
      }
    return x;