RE: [MIPS] Update the ZC constraint for MIPSR6 and use it

[Matthew Fortune <Matthew.Fortune@imgtec.com>]

Moore, Catherine <Catherine_Moore@mentor.com> writes:
> > -----Original Message-----
> > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com]
> > Sent: Wednesday, January 14, 2015 2:54 PM
> > To: Moore, Catherine
> > Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> > Subject: RE: [MIPS] Update the ZC constraint for MIPSR6 and use it
> >
> > Moore, Catherine <Catherine_Moore@mentor.com> writes
> > > Hi Matthew,
> > >
> > > > -----Original Message-----
> > > > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com]
> > > > Sent: Tuesday, January 06, 2015 7:43 AM
> > > > To: Moore, Catherine
> > > > Cc: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)
> > > > Subject: [MIPS] Update the ZC constraint for MIPSR6 and use it
> > > >
> > > > Update the ZC constraint for MIPSR6 to allow it to be used as the
> > > > memory operand for implementations of atomic operations.  Also
> > > > switch the internal implementation of atomic operations to use ZC
> > > > instead of
> > > ZR.
> > > >
> > > > This fix accurately describes the memory constraints for the LL
> > > > and SC instructions.  An offset can therefore be used to access a
> > > > data item (ie. %lo (<var>)) rather than always having to load the
> > > > address into a register.  Tested for mips32r2, mips32r6 and
> micromips.
> > > >
> > > > gcc/
> > > >
> > > > 	* config/mips/constraints.md (ZC): Add support for R6 LL/SC
> > > > 	offsets.
> > > > 	(ZD): Update to use ISA_HAS_PREF_LL_SC_9BIT.
> > > > 	* config/mips/mips.h (ISA_HAS_PREFETCH_9BIT): Rename to...
> > > > 	(ISA_HAS_PREF_LL_SC_9BIT): ... this. New macro.
> > > > 	* config/mips/sync.md (sync_compare_and_swap<mode>): Use ZC
> > > > 	instead of ZR for the memory operand of LL/SC.
> > > > 	(compare_and_swap_12, sync_add<mode>): Likewise.
> > > > 	(sync_<optab>_12, sync_old_<optab>_12): Likewise.
> > > > 	(sync_new_<optab>_12, sync_nand_12): Likewise.
> > > > 	(sync_old_nand_12, sync_new_nand_12): Likewise.
> > > > 	(sync_sub<mode>, sync_old_add<mode>): Likewise.
> > > > 	(sync_old_sub<mode>, sync_new_add<mode>): Likewise.
> > > > 	(sync_new_sub<mode>, sync_<optab><mode>): Likewise.
> > > > 	(sync_old_<optab><mode>, sync_new_<optab><mode>"):
> > > > Likewise.
> > > > 	(sync_nand<mode>, sync_old_nand<mode>): Likewise.
> > > > 	(sync_new_nand<mode>, sync_lock_test_and_set<mode>):
> > > > Likewise.
> > > > 	(test_and_set_12, atomic_compare_and_swap<mode>): Likewise.
> > > > 	(atomic_exchange<mode>_llsc, atomic_fetch_add<mode>_llsc):
> > > > Likewise.
> > > > 	* doc/md.texi (ZC): Update description.
> > > >
> > > > OK to commit?
> > > >
> > > > diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> > > > 9dad480..b608b17 100644
> > > > --- a/gcc/config/mips/mips.h
> > > > +++ b/gcc/config/mips/mips.h
> > > > @@ -1089,8 +1089,8 @@ struct mips_cpu_info {
> > > >  				  || mips_isa_rev >= 1)			\
> > > >  				 && !TARGET_MIPS16)
> > > >
> > > > -/* ISA has data prefetch with limited 9-bit displacement.  */
> > > > -#define ISA_HAS_PREFETCH_9BIT	(mips_isa_rev >= 6)
> > > > +/* ISA has data prefetch, LL and SC with limited 9-bit
> displacement.
> > > */
> > > > +#define ISA_HAS_PREF_LL_SC_9BIT	(mips_isa_rev >= 6)
> > > >
> > > I'd like to see this described as something more general.  Say:
> > > ISA_HAS_9BIT_DISPLACEMENT.   This patch is okay with that fixup.
> >
> > I think I'm OK with changing that but it does leave us with a
> > different issue of knowing which subset of instructions should check
> for 9-bit displacement.
> > I.e. not all instructions only have a 9-bit displacement.
> 
> I'm open to a different name.  Do you have any other suggestions?   Can
> we just say >= R6?

That is pretty much what it boils down to but I do like keeping all the
isa level checks in one place and giving names to things. I'll go with
your suggestion and leave the rest to a later general improvement.

Thanks,
Matthew

> > A GCC 6 thing would be to look over all the ISA_HAS macros and perhaps
> > do some general improvement in the framework we have there. I don't
> > know exactly what I'd do but something a bit more table based seems
> sensible.
> >
> Sounds like a good idea.