Re: [PATCH], Add PowerPC ISA 3.0 vector d-form addressing

[Michael Meissner <meissner at linux dot vnet dot ibm dot com>]

On Mon, May 09, 2016 at 08:11:54AM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Thu, May 05, 2016 at 02:05:19PM -0400, Michael Meissner wrote:
> > > > With this patch, I enable -mlra if the user did not specify either -mlra or
> > > > -mno-lra on the command line, and -mcpu=power9 or -mpower9-dform-vector were
> > > > used. I also enabled -mvsx-timode if LRA was used, which also is a RELOAD
> > > > issue, that works with LRA.
> > > 
> > > I don't like enabling LRA if the user didn't ask for it; it is a bit too
> > > surprising.  What do you do if there is -mno-lra explicitly?  You can just
> > > do the same if no-lra is implicit?
> > 
> > Ok.
> 
> You didn't however change this afaics?

The patch kept reload as the default. You have to explicitly enable LRA to get
vector d-forms by default with -mcpu=power9.

> 
> > > > --- gcc/config/rs6000/rs6000.opt	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000)	(revision 235831)
> > > > +++ gcc/config/rs6000/rs6000.opt	(.../gcc/config/rs6000)	(working copy)
> > > > @@ -470,8 +470,8 @@ Target RejectNegative Joined UInteger Va
> > > >  -mlong-double-<n>	Specify size of long double (64 or 128 bits).
> > > >  
> > > >  mlra
> > > > -Target Report Var(rs6000_lra_flag) Init(0) Save
> > > > -Use LRA instead of reload.
> > > > +Target Undocumented Mask(LRA) Var(rs6000_isa_flags)
> > > > +Use the LRA register allocator instead of the reload register allocator.
> > > 
> > > It wasn't "undocumented" before?  Why the change to a mask bit btw?
> > 
> > It was always meant to be undocumented, but I changed to be similar to
> > before. I am trying to change all of the random switches that set a word to be
> > an option mask, so I made that part of the change in these next patches.
> 
> I agree it should be undocumented because hopefully one day all reload
> will not exist at all anymore.  OTOH, all other archs with an -mlra
> switch do not have it hidden, so we might as well follow suit there.

I will write up some documentation.

> > I did remove setting it for -mcpu=power9.
> 
> It doesn't look like it?  Please check.
> 
> > @@ -94,6 +95,7 @@
> >  				 | OPTION_MASK_FPRND			\
> >  				 | OPTION_MASK_HTM			\
> >  				 | OPTION_MASK_ISEL			\
> > +				 | OPTION_MASK_LRA			\
> >  				 | OPTION_MASK_MFCRF			\
> >  				 | OPTION_MASK_MFPGPR			\
> >  				 | OPTION_MASK_MODULO			\

That is POWERPC_MASKS, which is the mask of all option bits that COULD be set
by -mcpu=<xxx> options. But none of the -mcpu=<xxx> set it any more (the
previous patch did set it in ISA_3_0_MASKS_SERVER, but it doesn't do it any
more. In retrospect, when I created the option masks, I should have used 2
separate words, one for things like -m32 that can never be changed, and the
other for all of the normal bits, and not need POWERPC_MASKS. But in general, I
always add things to POWERPC_MASKS unless it explictly should not be.

> > > > +mpower9-dform-scalar
> > > > +Target Report Mask(P9_DFORM_SCALAR) Var(rs6000_isa_flags)
> > > > +Use/do not use scalar register+offset memory instructions added in ISA 3.0.
> > > > +
> > > > +mpower9-dform-vector
> > > > +Target Report Mask(P9_DFORM_VECTOR) Var(rs6000_isa_flags)
> > > > +Use/do not use vector register+offset memory instructions added in ISA 3.0.
> > > > +
> > > >  mpower9-dform
> > > > -Target Undocumented Mask(P9_DFORM) Var(rs6000_isa_flags)
> > > > -Use/do not use vector and scalar instructions added in ISA 3.0.
> > > > +Target Report Var(TARGET_P9_DFORM_BOTH) Init(-1) Save
> > > > +Use/do not use register+offset memory instructions added in ISA 3.0.
> > > 
> > > These should probably all be undocumented, though (they're not something
> > > users should use).
> > 
> > I will make -mpower9-dform public (which I thought it was, but evidently I
> > missed adding the documentation for GCC 6), but I will make the -scalar and
> > -vector forms private.
> 
> You think this is something users are expected to twiddle?  Okay then.

I don't expect users to normally twiddle it, but there are times in either bug
fixing and/or extreme benchmarking where they do.

> > [gcc]
> > 2016-05-05  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	* config/rs6000/rs6000-cpus.def (ISA_3_0_MASKS_SERVER): Use
> > 	-mpower9-dform-scalar instead of -mpower9-dform. Add note not to
> > 	include -mpower9-dform-vector until we switch over to LRA.
> 
> Thanks for the better changelog, much appreciated.  Two spaces after
> a full stop though.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797