Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Richard Biener <rguenther@suse.de>]

On Wed, 26 Jul 2017, Jakub Jelinek wrote:

> On Tue, Jul 25, 2017 at 03:52:56PM -0500, Segher Boessenkool wrote:
> > On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote:
> > > This patch only adds new vector from vector extract and init patterns to
> > > the i386 backend, but I had to change many other targets too, because
> > > it needs to have the element mode in the vec_extract/vec_init expander
> > > names.  Seems most of the backends didn't really have a mode attribute
> > > usable for this or had it only in uppercase, while for the names we need
> > > lowercase.  Some backends had a convention on how to name lower case
> > > vs. upper case modes, others didn't have any.  So I'm CCing maintainers
> > > of affected backends to seek advice on what mode attributes they want to
> > > use.
> > 
> > Would it be possible (and useful) to _also_ keep the old names?  Or do
> > you expect all targets will want to support all combinations?
> 
> Richi's preference was to use only a single conversion optab instead of
> old + new when we've discussed it on IRC.  Of course it would be far less

Yep.  To me there's no advantage to having two variants besides avoiding
the mechanical change to existing backends adding the matching
component mode (plus a convenient iterator for that -- or maybe
gen* support for "component of mode").

> work for me to support say:
> OPTAB_CD(vec_extract2_optab, "vec_extract$a$b")
> OPTAB_CD(vec_init2_optab, "vec_init$a$b")
> OPTAB_D (vec_extract_optab, "vec_extract$a")
> OPTAB_D (vec_init_optab, "vec_init$a")
> where the single mode vec_extract/vec_init would be
> extraction/initialization from element mode and the two mode one would be
> used for 2 vector modes.  If there is agreement on that, most of the
> config/*/* changes could go away.
> 
> > > --- gcc/config/rs6000/vector.md.jj	2017-06-08 20:50:49.000000000 +0200
> > > +++ gcc/config/rs6000/vector.md	2017-07-24 17:44:44.699580927 +0200
> > > @@ -74,6 +74,16 @@ (define_mode_attr VEC_base [(V16QI "QI")
> > >  			    (V1TI  "TI")
> > >  			    (TI    "TI")])
> > >  
> > > +;; As above, but in lower case
> > > +(define_mode_attr VEC_base_l [(V16QI "qi")
> > > +			      (V8HI  "hi")
> > > +			      (V4SI  "si")
> > > +			      (V2DI  "di")
> > > +			      (V4SF  "sf")
> > > +			      (V2DF  "df")
> > > +			      (V1TI  "ti")
> > > +			      (TI    "ti")])
> > > +
> > >  ;; Same size integer type for floating point data
> > >  (define_mode_attr VEC_int [(V4SF  "v4si")
> > >  			   (V2DF  "v2di")])
> > 
> > > @@ -520,6 +520,17 @@ (define_mode_attr VEL [(V8QI "QI") (V16Q
> > >  			(SI   "SI") (HI   "HI")
> > >  			(QI   "QI")])
> > >  
> > > +;; Define element mode for each vector mode (lower case).
> > > +(define_mode_attr Vel [(V8QI "qi") (V16QI "qi")
> > > +			(V4HI "hi") (V8HI "hi")
> > > +			(V2SI "si") (V4SI "si")
> > > +			(DI "di")   (V2DI "di")
> > > +			(V4HF "hf") (V8HF "hf")
> > > +			(V2SF "sf") (V4SF "sf")
> > > +			(V2DF "df") (DF "df")
> > > +			(SI   "si") (HI   "hi")
> > > +			(QI   "qi")])
> > 
> > (Inconsistent spacing, please fix).
> 
> It is the same spacing as in VEL right above it, I've tried to follow
> whatever weirdo formatting each backend had.
> 
> > ("vel" instead of "Vel" for this name?)
> 
> That is to follow aarch64 iterator naming convention, where they have
> already preexisting e.g. VDBL for
> ;; Double modes of vector modes.
> and Vdbl for:
> ;; Double modes of vector modes (lower case).
> or similarly VHALF vs Vhalf.
> 
> > How is this different from VEC_base_l?  They can just be merged it seems.
> 
> They can't be merged, each backend has its own iterators and iterator naming
> conventions, we don't really have any gcc/iterators.md that would be used
> for all backends.  VEC_base_l is just rs6000 (and powerpcspe), using
> rs6000 iterator naming conventions, Vel is aarch64 using aarch64 naming
> conventions, V_elem_l is arm using arm iterator naming conventions etc.
> 
> > (And for that matter, VEC_base and VEL as well).  We'll handle that I
> > suppose, don't let it hold up this patch :-)
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)