Re: [RFA] replace stub fn mips_eabi_return_value

Re: [RFA] replace stub fn mips_eabi_return_value

[Michael Snyder]

Ping?

Michael Snyder wrote:
> ... just as Fred Fish recently did for the o64 version.
> 
> 
> ------------------------------------------------------------------------
> 
> 2006-05-31  Michael Snyder  <msnyder@redhat.com>
> 
> 	* mips-tdep.c (mips_eabi_return_value): Replace stub that always
> 	returned RETURN_VALUE_STRUCT_CONVENTION with a real function.
> 
> Index: mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.393
> diff -p -r1.393 mips-tdep.c
> *** mips-tdep.c	31 May 2006 23:15:50 -0000	1.393
> --- mips-tdep.c	31 May 2006 23:52:56 -0000
> *************** mips_eabi_return_value (struct gdbarch *
> *** 2651,2664 ****
>   			struct type *type, struct regcache *regcache,
>   			gdb_byte *readbuf, const gdb_byte *writebuf)
>   {
>     if (TYPE_LENGTH (type) > 2 * mips_abi_regsize (gdbarch))
>       return RETURN_VALUE_STRUCT_CONVENTION;
> !   if (readbuf)
> !     memset (readbuf, 0, TYPE_LENGTH (type));
>     return RETURN_VALUE_REGISTER_CONVENTION;
>   }
>   
> - 
>   /* N32/N64 ABI stuff.  */
>   
>   static CORE_ADDR
> --- 2651,2712 ----
>   			struct type *type, struct regcache *regcache,
>   			gdb_byte *readbuf, const gdb_byte *writebuf)
>   {
> +   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +   int fp_return_type = 0;
> +   int offset, regnum, xfer;
> + 
>     if (TYPE_LENGTH (type) > 2 * mips_abi_regsize (gdbarch))
>       return RETURN_VALUE_STRUCT_CONVENTION;
> ! 
> !   /* Floating point type?  */
> !   if (tdep->mips_fpu_type != MIPS_FPU_NONE)
> !     {
> !       if (TYPE_CODE (type) == TYPE_CODE_FLT)
> ! 	fp_return_type = 1;
> !       /* Structs with a single field of float type 
> ! 	 are returned in a floating point register.  */
> !       if ((TYPE_CODE (type) == TYPE_CODE_STRUCT
> ! 	   || TYPE_CODE (type) == TYPE_CODE_UNION)
> ! 	  && TYPE_NFIELDS (type) == 1)
> ! 	{
> ! 	  struct type *fieldtype = TYPE_FIELD_TYPE (type, 0);
> ! 
> ! 	  if (TYPE_CODE (check_typedef (fieldtype)) == TYPE_CODE_FLT)
> ! 	    fp_return_type = 1;
> ! 	}
> !     }
> ! 
> !   if (fp_return_type)      
> !     {
> !       /* A floating-point value belongs in the least significant part
> !          of FP0/FP1.  */
> !       if (mips_debug)
> ! 	fprintf_unfiltered (gdb_stderr, "Return float in $fp0\n");
> !       regnum = mips_regnum (gdbarch)->fp0;
> !     }
> !   else 
> !     {
> !       /* An integer value goes in V0/V1.  */
> !       if (mips_debug)
> ! 	fprintf_unfiltered (gdb_stderr, "Return scalar in $v0\n");
> !       regnum = MIPS_V0_REGNUM;
> !     }
> !   for (offset = 0;
> !        offset < TYPE_LENGTH (type);
> !        /* offset += register_size (gdbarch, regnum), regnum++) */
> !        offset += mips_stack_argsize (gdbarch), regnum++)
> !     {
> !       /* xfer = register_size (gdbarch, regnum); */
> !       xfer = mips_stack_argsize (gdbarch);
> !       if (offset + xfer > TYPE_LENGTH (type))
> ! 	xfer = TYPE_LENGTH (type) - offset;
> !       mips_xfer_register (regcache, NUM_REGS + regnum, xfer,
> ! 			  TARGET_BYTE_ORDER, readbuf, writebuf, offset);
> !     }
> ! 
>     return RETURN_VALUE_REGISTER_CONVENTION;
>   }
>   
>   /* N32/N64 ABI stuff.  */
>   
>   static CORE_ADDR

Re: [RFA] replace stub fn mips_eabi_return_value

[Joel Brobecker]

> 2006-05-31  Michael Snyder  <msnyder@redhat.com>
> 
> 	* mips-tdep.c (mips_eabi_return_value): Replace stub that always
> 	returned RETURN_VALUE_STRUCT_CONVENTION with a real function.

This looks OK to me. Just make sure to remove the two commented out
lines of code in the last "for" block.

Thanks.

> !   for (offset = 0;
> !        offset < TYPE_LENGTH (type);
> !        /* offset += register_size (gdbarch, regnum), regnum++) */
> !        offset += mips_stack_argsize (gdbarch), regnum++)
> !     {
> !       /* xfer = register_size (gdbarch, regnum); */

-- 
Joel

Re: [RFA] replace stub fn mips_eabi_return_value

[Michael Snyder]

Joel Brobecker wrote:
>>2006-05-31  Michael Snyder  <msnyder@redhat.com>
>>
>>	* mips-tdep.c (mips_eabi_return_value): Replace stub that always
>>	returned RETURN_VALUE_STRUCT_CONVENTION with a real function.
> 
> 
> This looks OK to me. Just make sure to remove the two commented out
> lines of code in the last "for" block.

Will do, but now that you mention it -- that comment was sort of
a reminder to myself that I should ping Fred about this issue.

Fred, your similar code for o64 also makes use of register_size.
I don't at the moment remember what the issue was, but I did run
into problems that suggested to me that stack_argsize was a better
choice -- and the change did improve things.

Any thoughts?


>>!   for (offset = 0;
>>!        offset < TYPE_LENGTH (type);
>>!        /* offset += register_size (gdbarch, regnum), regnum++) */
>>!        offset += mips_stack_argsize (gdbarch), regnum++)
>>!     {
>>!       /* xfer = register_size (gdbarch, regnum); */
> 
> 

Re: [RFA] replace stub fn mips_eabi_return_value

[Fred Fish]

On Monday 12 June 2006 15:26, Michael Snyder wrote:
> Fred, your similar code for o64 also makes use of register_size.
> I don't at the moment remember what the issue was, but I did run
> into problems that suggested to me that stack_argsize was a better
> choice -- and the change did improve things.
> 
> Any thoughts?

Actually I think I just snagged that code from one of the other
functions and tweaked it as needed.  So I don't have any insight into
which function is better.  I can give it a try both ways in my version
and see which one works better.  :-)

-Fred

Re: [RFA] replace stub fn mips_eabi_return_value

[Fred Fish]

On Monday 12 June 2006 15:26, Michael Snyder wrote:
> Fred, your similar code for o64 also makes use of register_size.

Actually it appears that it is mips_n32n64_return_value that uses
register_size, not mips_o64_return_value.

-Fred

Re: [RFA] replace stub fn mips_eabi_return_value

[Fred Fish]

On Wednesday 31 May 2006 19:55, Michael Snyder wrote:
> ... just as Fred Fish recently did for the o64 version.

BTW, I added this patch to my source tree, built a mipsisa64-elf toolchain,
and ran it with the following multilibs:

    mips-sim-idt64/eb/hard-float
    mips-sim-idt64/eb/hard-float/-mips32
    mips-sim-idt64/eb/hard-float/-mips32r2
    mips-sim-idt64/eb/soft-float
    mips-sim-idt64/eb/soft-float/-mips32
    mips-sim-idt64/eb/soft-float/-mips32r2
    mips-sim-idt64/el/hard-float
    mips-sim-idt64/el/hard-float/-mips32
    mips-sim-idt64/el/hard-float/-mips32r2
    mips-sim-idt64/el/soft-float
    mips-sim-idt64/el/soft-float/-mips32
    mips-sim-idt64/el/soft-float/-mips32r2

The results improved dramatically from the previous run:

  < # of expected passes          101779
  < # of unexpected failures      8279
  ---
  > # of expected passes          105697
  > # of unexpected failures      4347

-Fred