Re: [PATCH] MIPS SIMD Architecture (MSA) patch

[Richard Sandiford <rdsandiford at googlemail dot com>]

Chao-Ying Fu <Chao-Ying.Fu@imgtec.com> writes:
>> I wonder whether -mmsa is the best name for the 128-bit option if we
>> know that a 256-bit form might come along.
>
>   I am ok with any names.  Or, we can add another switch to specify the
> width of MSA,
> if 256-bit MSA comes along.

OK.  I assume that if a new revision of the ASE comes along with 256-bit
support, it'll still try to be compatible with the original revision?
If so, we could end up needing to specify the width and the revision
separately.

So maybe it would be better for GCC to have a -mmsa128 option to specify
the register width.  This would be like -mgp32/-mgp64 and -mfp32/-mfp64.
Then plain -mmsa can be used to specify the revision, a bit like -mdsp
and -mdspr2.  (Similarly for ".set dsp(r2)" and ".set msa".)

The assembler would only need to know about -mmsa, so there'd be no changes
to the option handling in the rest of the patch.  It's just a question of
what the GCC option will be called, and what should be used in these error
messages.

> > > Index: include/elf/mips.h
> > > ===================================================================
> > > RCS file: /cvs/src/src/include/elf/mips.h,v
> > > retrieving revision 1.55
> > > diff -u -p -r1.55 mips.h
> > > --- include/elf/mips.h	17 Sep 2013 21:05:49 -0000	1.55
> > > +++ include/elf/mips.h	9 Oct 2013 23:42:58 -0000
> > > @@ -1135,6 +1135,9 @@ enum
> > >  
> > >    /* Floating-point ABI used by this object file.  */
> > >    Tag_GNU_MIPS_ABI_FP = 4,
> > > +
> > > +  /* MSA ABI used by this object file.  */
> > > +  Tag_GNU_MIPS_ABI_MSA = 8,
> > >  };
> > 
> > These tags are enums rather than bitfields.  Are 5-7 already taken?
> > If not, and if this isn't "in the wild" yet, it'd be good to use 5
> > instead if possible.
>
>   The selection of 8 is due to the attr types designed by bfd/elf-attrs.c.
> Ex:
> /* Determine whether a GNU object attribute tag takes an integer, a
>    string or both.  */
> static int
> gnu_obj_attrs_arg_type (int tag)
> {
>   /* Except for Tag_compatibility, for GNU attributes we follow the
>      same rule ARM ones > 32 follow: odd-numbered tags take strings
>      and even-numbered tags take integers.  In addition, tag & 2 is
>      nonzero for architecture-independent tags and zero for
>      architecture-dependent ones.  */
>   if (tag == Tag_compatibility)
>     return 3;
>   else
>     return (tag & 1) != 0 ? 2 : 1;
> }
>
> Similarly, "include/elf/ppc.h" uses 4, and 8, and 12 for object attribute tags.

Doh, hadn't realised that, sorry.

> > > @@ -513,7 +522,7 @@ const struct mips_arch_choice mips_arch_
> > >    { "mips64r2",	1, bfd_mach_mipsisa64r2, CPU_MIPS64R2,
> > >      ISA_MIPS64R2,
> > >      (ASE_MIPS3D | ASE_DSP | ASE_DSPR2 | ASE_DSP64 | ASE_EVA | ASE_MT
> > > -     | ASE_MDMX | ASE_MCU | ASE_VIRT | ASE_VIRT64),
> > > +     | ASE_MDMX | ASE_MCU | ASE_VIRT | ASE_VIRT64 | ASE_MSA | ASE_MSA64),
> > >      mips_cp0_names_mips3264r2,
> > >      mips_cp0sel_names_mips3264r2, ARRAY_SIZE (mips_cp0sel_names_mips3264r2),
> > >      mips_hwr_names_mips3264r2 },
> > > @@ -738,6 +747,18 @@ parse_mips_dis_option (const char *optio
> > >        return;
> > >      }
> > >  
> > > +  if (CONST_STRNEQ (option, "msa"))
> > > +    {
> > > +      mips_ase |= ASE_MSA;
> > > +      if ((mips_isa & INSN_ISA_MASK) == ISA_MIPS64R2)
> > > +	{
> > > +	  mips_ase |= ASE_MSA64;
> > > +	  /* Disable ASE_MDMX, because of opcode conflicts.  */
> > > +	  mips_ase &= ~ASE_MDMX;
> > > +	}
> > > +      return;
> > > +    }
> > 
> > Hmm, but you keep MDMX in the default list for mips64r2.  
> > Maybe we really
> > do need to add mips64r5 :-)
>
>   Sure.  New work to do.  :-)

Still, adding an incompatible ASE to mips64r2 just seems wrong.
We should either remove ASE_MDMX or leave ASE_MSA* out (of both
the mips32r2 and mips64r2 entries).  Since the only in-tree processor
with MDMX support is the MIPS 64r1 SB1, I'm happy with dropping MDMX
if that's easier.  There might be some testsuite fallout from doing
that though.

OK with those changes, thanks,

Richard