RE: [PATCH RFC] MIPS add support for MIPS SIMD ARCHITECTURE V1.07

[Matthew Fortune <Matthew dot Fortune at imgtec dot com>]

Hi Richard,

I'd like to get some more of your thoughts on the ABI implications of MSA. Generally, any thoughts you have (or anyone else) on the current state of MIPS ABIs would be welcome. As an example I'm curious whether you see variations of O32 as being new ABIs or extensions of O32, it can be seen as a fine line.

The MSA patch as submitted is another variation of O32 ABI which could be described as O32+FP64+MSA(+nan2008) and would be link incompatible with both O32 and O32+FP64(+/-nan2008). The same of course applies to N32/N64 being link incompatible with N32/N64+FP64. This may make it difficult to have a small part of an application optimised with MSA but the rest of the code ignorant of MSA, in your experience do you think that will be an issue?

We (MIPS) have several discussions ongoing regarding ABIs on a variety of mailing lists so I'm trying to collect as much input as possible to inform future plans. 

One part of the patch that I don't believe you commented on is the change of stack alignment for both existing O32+FP64 and new O32+FP64+MSA. Did this seem OK? You did however comment on a related change to MIPS_STACK_ALIGN which has similar implications. I've also pasted the other parts of the patch that are ABI related along with your original comments below.

> -#define STACK_BOUNDARY (TARGET_NEWABI ? 128 : 64)
> +/* Because we want to allow MSA functions and non-MSA functions to
> +   call + each other and MSA always requires -mfp64, we set stack
> +   boundary to + 128 bits when newabi or -mfp64.  */
> +#define STACK_BOUNDARY \
> +  ((TARGET_NEWABI || TARGET_FLOAT64) ? 128 : 64)

Thoughts and opinions welcome from all...

Regards,
Matthew

> > @@ -419,11 +423,17 @@
> >    /* The number of words passed in registers, rounded up.  */
> >    unsigned int reg_words;
> >
> > -  /* For EABI, the offset of the first register from GP_ARG_FIRST or
> > -     FP_ARG_FIRST.  For other ABIs, the offset of the first register
> > from
> > -     the start of the ABI's argument structure (see the
> > CUMULATIVE_ARGS
> > -     comment for details).
> > +  /* If msa_reg_p is true, the offset of the first register from
> > +     MSA_ARG_FIRST.
> >
> > +     The value is MAX_ARGS_IN_MSA_REGISTERS if the argument is passed
> > +entirely
> > +     on the stack.
> > +
> > +     If msa_reg_p is false, for EABI, the offset of the first
> > +register from
> > +     GP_ARG_FIRST or FP_ARG_FIRST.  For other ABIs, the offset of the
> > +first
> > +     register from the start of the ABI's argument structure
> > +     (see the CUMULATIVE_ARGS comment for details).
> > +
> >       The value is MAX_ARGS_IN_REGISTERS if the argument is passed
> >entirely
> >       on the stack.  */
> >    unsigned int reg_offset;
> 
> So the MSA arguments operate outside the normal "argument structure"
> for o32, n32 and n64?  That's OK with me but please write up what the new
> ABI is.
> 
> > @@ -6445,6 +6979,233 @@
> >      }
> >  }
> >
> > +/* Write a stub for the current function.  This stub is used
> > +   for functions from FR0 to FR1 or from FR1 to FR0.
> > +   If FR0_TO_FR1_P is true, we need to set the FR mode to 1,
> > +   set up floating-point arguments, align the stack pointer to 16
> > +bytes,
> > +   copy the arguments on the stack, call the function, set up the
> > +returned
> > +   floating-point value, and restore the FR mode to 0.
> > +   If FR0_TO_FR1_P is false, the stub will be similar but we won't
> > +need to
> > +   align the stack pointer to 16 bytes and copy the arguments.  */
> 
> Hmm, what's all this about?
> 
> 
> > @@ -1410,8 +1436,11 @@
> >  /* 8 is observed right on a DECstation and on riscos 4.02.  */
> >  #define STRUCTURE_SIZE_BOUNDARY 8
> >
> > -/* There is no point aligning anything to a rounder boundary than
> > this.  */ -#define BIGGEST_ALIGNMENT LONG_DOUBLE_TYPE_SIZE
> > +/* There is no point aligning anything to a rounder boundary than
> > +   LONG_DOUBLE_TYPE_SIZE, unless under MSA the bigggest alignment is
> > +   BITS_PER_MSA_REG.  */
> > +#define BIGGEST_ALIGNMENT \
> > +  (TARGET_MSA ? BITS_PER_MSA_REG : LONG_DOUBLE_TYPE_SIZE)
> 
> I'm not sure about the ABI implications of this -- will have to think about
> it.  Please integrate MSA into the comment a bit more rather than tacking it
> onto the end.
> 
> > @@ -2293,8 +2372,8 @@
> >  /* Treat LOC as a byte offset from the stack pointer and round it up
> >     to the next fully-aligned offset.  */
> >  #define MIPS_STACK_ALIGN(LOC) \
> > -  (TARGET_NEWABI ? ((LOC) + 15) & -16 : ((LOC) + 7) & -8)
> > -
> > +  ((TARGET_NEWABI || TARGET_FLOAT64) ? \
> > +   ((LOC) + 15) & -16 : ((LOC) + 7) & -8)
> 
> What's this about -- again seems dangerous ABIwise.