Re: [MIPS] One test failed

[Richard Sandiford <richard at codesourcery dot com>]

"Fu, Chao-Ying" <fu@mips.com> writes:
>   I got one test that failed by using the mainline GCC
> on the target of mipsisa32r2-elf.
> The handling of stack pointer or frame pointer may be broken.
> Thanks!
>
> Ex:
> # mipsisa32r2-elf-gcc -o bug126 -Tidt32.ld bug126.c
> # mipsisa32r2-elf-run bug126
> mips-core: 4 byte read to unmapped address 0x4 at 0x80020348
> program stopped with signal 10.

Doh!  Sorry.  This is caused by:

    /* MIPS16e SAVE and RESTORE instructions require the GP save area
       to be aligned at the high end with any padding at the low end,
       so do it that way all the time.  */
    offset = (total_size
              - MIPS_STACK_ALIGN (fp_reg_size)
              - GET_MODE_SIZE (gpr_mode));

I believe the idea was OK for o32 and o64, but I'd forgotten that other
ABIs need to save pretend arguments and varargs at the top of the frame.
So this bug shows up on mipsisa32r2-elf (which uses EABI32) but I didn't
see it on mipsisa32r2-elfoabi.

I've restored the original calculation for !GENERATE_MIPS16E_SAVE_RESTORE,
with a bit of extra commentary.  I've checked that it works for your
testcase, and tested it on mipsisa32r2-elfoabi (the only target for
which I have very recent results).  I'm going to install it based on
that testing because this is likely to break bootstrap on 64-bit
MIPS GNU/Linux.

I'm sorry for the problems with the save/restore patch.  For the record,
both bugs were entirely my fault, not the fault of the others named in
the changelog.

Richard


gcc/
	* config/mips/mips.c (compute_frame_size): Restore the original
	gp_sp_offset for !GENERATE_MIPS16E_SAVE_RESTORE and remove the
	fp_size term from the GENERATE_MIPS16E_SAVE_RESTORE calculation.
	Document why the difference is needed.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 126404)
+++ gcc/config/mips/mips.c	(working copy)
@@ -6781,12 +6781,16 @@ compute_frame_size (HOST_WIDE_INT size)
     {
       HOST_WIDE_INT offset;
 
-      /* MIPS16e SAVE and RESTORE instructions require the GP save area
-	 to be aligned at the high end with any padding at the low end,
-	 so do it that way all the time.  */
-      offset = (total_size
-		- MIPS_STACK_ALIGN (fp_reg_size)
-		- GET_MODE_SIZE (gpr_mode));
+      if (GENERATE_MIPS16E_SAVE_RESTORE)
+	/* MIPS16e SAVE and RESTORE instructions require the GP save area
+	   to be aligned at the high end with any padding at the low end.
+	   It is only safe to use this calculation for o32, where we never
+	   have pretend arguments, and where any varargs will be saved in
+	   the caller-allocated area rather than at the top of the frame.  */
+	offset = (total_size - GET_MODE_SIZE (gpr_mode));
+      else
+	offset = (args_size + cprestore_size + var_size
+		  + gp_reg_size - GET_MODE_SIZE (gpr_mode));
       cfun->machine->frame.gp_sp_offset = offset;
       cfun->machine->frame.gp_save_offset = offset - total_size;
     }