Re: setjmp on 32b MIPS with 64b FPRs

[Yossi Kreinin <yossi dot kreinin at gmail dot com>]

Hi,

I'll talk to the person maintaining the newlib build in our company
and we'll hopefully apply and test the patch together ASAP. In the
meanwhile, a couple of questions:

1. #ifdef __mips_fpr=64 - did you mean #if __mips_fpr == 64?

2. I'm guessing the definition of jmp_buf in setjmp.h would need
fixing as well ("breaking" the ABI - in the sense that currently
setjmp doesn't save doubles on 32b MIPSes with 64b floats, but perhaps
that's good enough to work for some apps; and once jmp_buf is
redefined to have enough space for saved 64b register values, those
previously working apps would break because they were compiled with
smaller jmp_bufs and now they're dynamically linked against a libc
that expects larger jmp_bufs, overwriting whatever it was that those
apps happened to place right after their jmp_bufs.) What's normally
done about ABI-breaking fixes in MIPS's libc I don't know - I'm just
guessing it might be a problem.

The relevant snippet of setjmp.h that appears to need an added #if
__mips_fpr==64 clause is:

#ifdef __mips__
#ifdef __mips64
#define _JBTYPE long long
#endif
#ifdef __mips_soft_float
#define _JBLEN 11
#else
#define _JBLEN 23
#endif
#endif

On Fri, Aug 23, 2013 at 11:20 PM, Steve Ellcey <sellcey@mips.com> wrote:
> On Thu, 2013-08-22 at 12:50 +0300, Yossi Kreinin wrote:
>> Hi,
>>
>> Apparently setjmp.S does not support 32b MIPS with 64b float registers
>> - rather, the code has a path for 32b MIPS, which assumes 32b GPRs and
>> FPRs, and a path for 64b MIPS, which assumes 64b GPRs and FPRs.
>> However, there exist 32b MIPS processors where each $f register can
>> keep a 64b double-precision number.
>>
>> The code below is based on setjmp.S and appears to work for me on 32b
>> MIPS with 64b FPRs. It is not an actual patch that can be applied to
>> setjmp.S because I'm unfamiliar with newlib and its build system and
>> so I don't know which #ifdefs to use to detect which MIPS we're
>> compiling for.
>>
>> Hope this helps,
>> -- Yossi
>
> Yossi,  Here is a patch with the proper ifdef's that should take care of
> the problem.  It seems to work fine for me, could you test it too and if
> it works for you and if someone approves it I can check it in.
>
> Steve Ellcey
> sellcey@mips.com
>
>
>
> 2013-08-23  Steve Ellcey  <sellcey@mips.com>
>
>         * libc/machine/mips/setjmp.S (FP_BYTES_PER_WORD): New.
>         (FPOFF): Modify to use FP_BYTES_PER_WORD.
>
>
>
>
> diff --git a/newlib/libc/machine/mips/setjmp.S
> b/newlib/libc/machine/mips/setjmp.S
> index 268cb18..0ebea59 100644
> --- a/newlib/libc/machine/mips/setjmp.S
> +++ b/newlib/libc/machine/mips/setjmp.S
> @@ -42,20 +42,32 @@
>
>  #ifdef __mips64
>  #define BYTES_PER_WORD 8
> +#define FP_BYTES_PER_WORD 8
>  #define LOAD_GPR ld
> -#define LOAD_FPR ldc1
>  #define STORE_GPR sd
> +#define LOAD_FPR ldc1
>  #define STORE_FPR sdc1
>  #else
>  #define BYTES_PER_WORD 4
>  #define LOAD_GPR lw
> -#define LOAD_FPR lwc1
>  #define STORE_GPR sw
> +#ifdef __mips_fpr=64
> +#define FP_BYTES_PER_WORD 8
> +#define LOAD_FPR ldc1
> +#define STORE_FPR sdc1
> +#else
> +#define FP_BYTES_PER_WORD 4
> +#define LOAD_FPR lwc1
>  #define STORE_FPR swc1
>  #endif
> +#endif
>
>  #define GPOFF(INDEX) (INDEX * BYTES_PER_WORD)
> -#define FPOFF(INDEX) ((INDEX + NUM_GPRS_SAVED) * BYTES_PER_WORD)
> +/* Make sure we get the right alignment for FP regs.  */
> +#define FPOFF(INDEX) ((((NUM_GPRS_SAVED * BYTES_PER_WORD) \
> +                       + FP_BYTES_PER_WORD - 1) \
> +                      / FP_BYTES_PER_WORD) * FP_BYTES_PER_WORD) \
> +                    + (INDEX * FP_BYTES_PER_WORD)
>
>  /* int setjmp (jmp_buf);  */
>         .globl  setjmp
>
>
>
>