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)
On Tue, Aug 01, 2017 at 06:21:34PM +0200, Jakub Jelinek wrote: > Apparently I broke power bootstrap with this, because two new spots were > introduced after I wrote the patch and my cross-compiler which didn't have > HAVE_AS_POWER9 defined didn't reveal that. Fixed thusly, committed as > obvious to trunk: Thanks! Segher
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)
On Wed, Jul 26, 2017 at 06:41:23AM -0500, Segher Boessenkool wrote: > > That is to follow aarch64 iterator naming convention, where they have > > Ugh, for some reason I thought this was in rs6000/ as well. I have > fresh coffee now. Sorry. Apparently I broke power bootstrap with this, because two new spots were introduced after I wrote the patch and my cross-compiler which didn't have HAVE_AS_POWER9 defined didn't reveal that. Fixed thusly, committed as obvious to trunk: 2017-08-01 Jakub Jelinek PR target/80846 * config/rs6000/vsx.md (vextract_fp_from_shorth, vextract_fp_from_shortl): Add element mode after mode in gen_vec_init* calls. --- gcc/config/rs6000/vsx.md.jj 2017-07-28 09:10:49.0 +0200 +++ gcc/config/rs6000/vsx.md2017-08-01 18:04:50.0 +0200 @@ -4523,7 +4523,7 @@ (define_expand "vextract_fp_from_shorth" inputs in half words 1,3,5,7 (IBM numbering). Use xxperm to move src half words 0,1,2,3 for the conversion instruction. */ v = gen_rtvec_v (16, rvals); - emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v))); + emit_insn (gen_vec_initv16qiqi (mask, gen_rtx_PARALLEL (V16QImode, v))); emit_insn (gen_altivec_vperm_v8hiv16qi (tmp, operands[1], operands[1], mask)); emit_insn (gen_vsx_xvcvhpsp (operands[0], tmp)); @@ -4552,7 +4552,7 @@ (define_expand "vextract_fp_from_shortl" inputs in half words 1,3,5,7 (IBM numbering). Use xxperm to move src half words 4,5,6,7 for the conversion instruction. */ v = gen_rtvec_v (16, rvals); - emit_insn (gen_vec_initv16qi (mask, gen_rtx_PARALLEL (V16QImode, v))); + emit_insn (gen_vec_initv16qiqi (mask, gen_rtx_PARALLEL (V16QImode, v))); emit_insn (gen_altivec_vperm_v8hiv16qi (tmp, operands[1], operands[1], mask)); emit_insn (gen_vsx_xvcvhpsp (operands[0], tmp)); Jakub
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)
On 25/07/17 10:14, Jakub Jelinek wrote: > Hi! > > The following patch adjusts the vec_init and vec_extract optabs, so that > they don't have in the expander names just the vector mode, but also another > mode, for vec_extract the mode of the result and for vec_init the mode of > the elts of the vector passed as second operand. > > Without this patch, the second mode has been implicit, GET_MODE_INNER of > the vector mode, so one could just extract a single element from a vector > or construct vector from elements. While that is most common, we allow > in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc. > and the vectorizer uses them. By having the second mode in the name > it allows the generic code (vectorizer, expansion) to query whether the > backend supports such vector from vector expansions or inits from vector > elts and use them if available. > > For vec_extract, if we say want to extract high V2SImode from V4SImode > the fallback is try to expand it as DImode extraction from V2DImode. > This works well in many cases, but doesn't really work for very large > vectors, say if we want to extract high V8SImode from V16SImode on x86, > we'd need OImode extraction from V2OImode, which is something the backend > doesn't have any support for. > For vec_init, the fallback is usually to go through memory, which is slow in > many cases. > > 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. > > Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves > e.g. the code generation for slp-43.c and slp-45.c testcases. > make cc1 tested in cross-compilers to the remaining targets. > > Ok for trunk? > > 2017-07-25 Jakub Jelinek > > PR target/80846 > * optabs.def (vec_extract_optab, vec_init_optab): Change from > a direct optab to conversion optab. > * optabs.c (expand_vector_broadcast): Use convert_optab_handler > with GET_MODE_INNER as last argument instead of optab_handler. > * expmed.c (extract_bit_field_1): Likewise. Use vector from > vector extraction if possible and optab is available. > * expr.c (store_constructor): Use convert_optab_handler instead > of optab_handler. Use vector initialization from smaller > vectors if possible and optab is available. > * tree-vect-stmts.c (vectorizable_load): Likewise. > * doc/md.texi (vec_extract, vec_init): Document that the optabs > now have two modes. > * config/i386/i386.c (ix86_expand_vector_init): Handle expansion > of vec_init from half-sized vectors with the same element mode. > * config/i386/sse.md (ssehalfvecmode): Add V4TI case. > (ssehalfvecmodelower, ssescalarmodelower): New mode attributes. > (reduc_plus_scal_v8df, reduc_plus_scal_v4df, reduc_plus_scal_v2df, > reduc_plus_scal_v16sf, reduc_plus_scal_v8sf, reduc_plus_scal_v4sf, > reduc__scal_, reduc_umin_scal_v8hi): Add element mode > after mode in gen_vec_extract* calls. > (vec_extract): Renamed to ... > (vec_extract): ... this. > (vec_extract): New expander. > (rotl3, rotr3, 3, ashrv2di3): Add > element mode after mode in gen_vec_init* calls. > (VEC_INIT_HALF_MODE): New mode iterator. > (vec_init): Renamed to ... > (vec_init): ... this. > (vec_init): New expander. > * config/i386/mmx.md (vec_extractv2sf): Renamed to ... > (vec_extractv2sfsf): ... this. > (vec_initv2sf): Renamed to ... > (vec_initv2sfsf): ... this. > (vec_extractv2si): Renamed to ... > (vec_extractv2sisi): ... this. > (vec_initv2si): Renamed to ... > (vec_initv2sisi): ... this. > (vec_extractv4hi): Renamed to ... > (vec_extractv4hihi): ... this. > (vec_initv4hi): Renamed to ... > (vec_initv4hihi): ... this. > (vec_extractv8qi): Renamed to ... > (vec_extractv8qiqi): ... this. > (vec_initv8qi): Renamed to ... > (vec_initv8qiqi): ... this. > * config/rs6000/vector.md (VEC_base_l): New mode attribute. > (vec_init): Renamed to ... > (vec_init): ... this. > (vec_extract): Renamed to ... > (vec_extract): ... this. > * config/rs6000/paired.md (vec_initv2sf): Renamed to ... > (vec_initv2sfsf): ... this. > * config/rs6000/altivec.md (splitter, altivec_copysign_v4sf3, > vec_unpacku_hi_v16qi, vec_unpacku_hi_v8hi, vec_unpacku_lo_v16qi, > vec_unpack
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)
On 07/25/2017 11:14 AM, Jakub Jelinek wrote: S/390 parts are ok. -Andreas-
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)
On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote: > The following patch adjusts the vec_init and vec_extract optabs, so that > they don't have in the expander names just the vector mode, but also another > mode, for vec_extract the mode of the result and for vec_init the mode of > the elts of the vector passed as second operand. > Ok for trunk? I failed to say this explicitly yet: okay for rs6000. Segher
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)
On Wed, Jul 26, 2017 at 09:09:04AM +0200, 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
> 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.
And less work for backends, old as well as new.
> > > @@ -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
Ugh, for some reason I thought this was in rs6000/ as well. I have
fresh coffee now. Sorry.
Segher
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)
On Tue, Jul 25, 2017 at 11:14 AM, Jakub Jelinek wrote: > Hi! > > The following patch adjusts the vec_init and vec_extract optabs, so that > they don't have in the expander names just the vector mode, but also another > mode, for vec_extract the mode of the result and for vec_init the mode of > the elts of the vector passed as second operand. > > Without this patch, the second mode has been implicit, GET_MODE_INNER of > the vector mode, so one could just extract a single element from a vector > or construct vector from elements. While that is most common, we allow > in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc. > and the vectorizer uses them. By having the second mode in the name > it allows the generic code (vectorizer, expansion) to query whether the > backend supports such vector from vector expansions or inits from vector > elts and use them if available. > > For vec_extract, if we say want to extract high V2SImode from V4SImode > the fallback is try to expand it as DImode extraction from V2DImode. > This works well in many cases, but doesn't really work for very large > vectors, say if we want to extract high V8SImode from V16SImode on x86, > we'd need OImode extraction from V2OImode, which is something the backend > doesn't have any support for. > For vec_init, the fallback is usually to go through memory, which is slow in > many cases. > > 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. > > Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves > e.g. the code generation for slp-43.c and slp-45.c testcases. > make cc1 tested in cross-compilers to the remaining targets. > > Ok for trunk? > > 2017-07-25 Jakub Jelinek > > PR target/80846 > * optabs.def (vec_extract_optab, vec_init_optab): Change from > a direct optab to conversion optab. > * optabs.c (expand_vector_broadcast): Use convert_optab_handler > with GET_MODE_INNER as last argument instead of optab_handler. > * expmed.c (extract_bit_field_1): Likewise. Use vector from > vector extraction if possible and optab is available. > * expr.c (store_constructor): Use convert_optab_handler instead > of optab_handler. Use vector initialization from smaller > vectors if possible and optab is available. > * tree-vect-stmts.c (vectorizable_load): Likewise. > * doc/md.texi (vec_extract, vec_init): Document that the optabs > now have two modes. > * config/i386/i386.c (ix86_expand_vector_init): Handle expansion > of vec_init from half-sized vectors with the same element mode. > * config/i386/sse.md (ssehalfvecmode): Add V4TI case. > (ssehalfvecmodelower, ssescalarmodelower): New mode attributes. > (reduc_plus_scal_v8df, reduc_plus_scal_v4df, reduc_plus_scal_v2df, > reduc_plus_scal_v16sf, reduc_plus_scal_v8sf, reduc_plus_scal_v4sf, > reduc__scal_, reduc_umin_scal_v8hi): Add element mode > after mode in gen_vec_extract* calls. > (vec_extract): Renamed to ... > (vec_extract): ... this. > (vec_extract): New expander. > (rotl3, rotr3, 3, ashrv2di3): Add > element mode after mode in gen_vec_init* calls. > (VEC_INIT_HALF_MODE): New mode iterator. > (vec_init): Renamed to ... > (vec_init): ... this. > (vec_init): New expander. > * config/i386/mmx.md (vec_extractv2sf): Renamed to ... > (vec_extractv2sfsf): ... this. > (vec_initv2sf): Renamed to ... > (vec_initv2sfsf): ... this. > (vec_extractv2si): Renamed to ... > (vec_extractv2sisi): ... this. > (vec_initv2si): Renamed to ... > (vec_initv2sisi): ... this. > (vec_extractv4hi): Renamed to ... > (vec_extractv4hihi): ... this. > (vec_initv4hi): Renamed to ... > (vec_initv4hihi): ... this. > (vec_extractv8qi): Renamed to ... > (vec_extractv8qiqi): ... this. > (vec_initv8qi): Renamed to ... > (vec_initv8qiqi): ... this. > * config/rs6000/vector.md (VEC_base_l): New mode attribute. > (vec_init): Renamed to ... > (vec_init): ... this. > (vec_extract): Renamed to ... > (vec_extract): ... this. > * config/rs6000/paired.md (vec_initv2sf): Renamed to ... > (vec_initv2sfsf): ... this. > * config/rs6000/altivec.md (splitter,
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)
On Tue, 25 Jul 2017, Jakub Jelinek wrote: > Hi! > > The following patch adjusts the vec_init and vec_extract optabs, so that > they don't have in the expander names just the vector mode, but also another > mode, for vec_extract the mode of the result and for vec_init the mode of > the elts of the vector passed as second operand. > > Without this patch, the second mode has been implicit, GET_MODE_INNER of > the vector mode, so one could just extract a single element from a vector > or construct vector from elements. While that is most common, we allow > in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc. > and the vectorizer uses them. By having the second mode in the name > it allows the generic code (vectorizer, expansion) to query whether the > backend supports such vector from vector expansions or inits from vector > elts and use them if available. > > For vec_extract, if we say want to extract high V2SImode from V4SImode > the fallback is try to expand it as DImode extraction from V2DImode. > This works well in many cases, but doesn't really work for very large > vectors, say if we want to extract high V8SImode from V16SImode on x86, > we'd need OImode extraction from V2OImode, which is something the backend > doesn't have any support for. > For vec_init, the fallback is usually to go through memory, which is slow in > many cases. > > 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. > > Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves > e.g. the code generation for slp-43.c and slp-45.c testcases. > make cc1 tested in cross-compilers to the remaining targets. > > Ok for trunk? The non-target specific bits are ok. Thanks, Richard. > 2017-07-25 Jakub Jelinek > > PR target/80846 > * optabs.def (vec_extract_optab, vec_init_optab): Change from > a direct optab to conversion optab. > * optabs.c (expand_vector_broadcast): Use convert_optab_handler > with GET_MODE_INNER as last argument instead of optab_handler. > * expmed.c (extract_bit_field_1): Likewise. Use vector from > vector extraction if possible and optab is available. > * expr.c (store_constructor): Use convert_optab_handler instead > of optab_handler. Use vector initialization from smaller > vectors if possible and optab is available. > * tree-vect-stmts.c (vectorizable_load): Likewise. > * doc/md.texi (vec_extract, vec_init): Document that the optabs > now have two modes. > * config/i386/i386.c (ix86_expand_vector_init): Handle expansion > of vec_init from half-sized vectors with the same element mode. > * config/i386/sse.md (ssehalfvecmode): Add V4TI case. > (ssehalfvecmodelower, ssescalarmodelower): New mode attributes. > (reduc_plus_scal_v8df, reduc_plus_scal_v4df, reduc_plus_scal_v2df, > reduc_plus_scal_v16sf, reduc_plus_scal_v8sf, reduc_plus_scal_v4sf, > reduc__scal_, reduc_umin_scal_v8hi): Add element mode > after mode in gen_vec_extract* calls. > (vec_extract): Renamed to ... > (vec_extract): ... this. > (vec_extract): New expander. > (rotl3, rotr3, 3, ashrv2di3): Add > element mode after mode in gen_vec_init* calls. > (VEC_INIT_HALF_MODE): New mode iterator. > (vec_init): Renamed to ... > (vec_init): ... this. > (vec_init): New expander. > * config/i386/mmx.md (vec_extractv2sf): Renamed to ... > (vec_extractv2sfsf): ... this. > (vec_initv2sf): Renamed to ... > (vec_initv2sfsf): ... this. > (vec_extractv2si): Renamed to ... > (vec_extractv2sisi): ... this. > (vec_initv2si): Renamed to ... > (vec_initv2sisi): ... this. > (vec_extractv4hi): Renamed to ... > (vec_extractv4hihi): ... this. > (vec_initv4hi): Renamed to ... > (vec_initv4hihi): ... this. > (vec_extractv8qi): Renamed to ... > (vec_extractv8qiqi): ... this. > (vec_initv8qi): Renamed to ... > (vec_initv8qiqi): ... this. > * config/rs6000/vector.md (VEC_base_l): New mode attribute. > (vec_init): Renamed to ... > (vec_init): ... this. > (vec_extract): Renamed to ... > (vec_extract): ... this. > * config/rs6000/paired.md (vec_initv2sf): Renamed to ... > (vec_initv2sfsf): ... this. > * config/rs6000/altivec.md (splitter, altivec_copysign_v4sf3, > vec_unpacku_hi_v16qi, vec_
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)
> Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves > e.g. the code generation for slp-43.c and slp-45.c testcases. > make cc1 tested in cross-compilers to the remaining targets. The SPARC bits are OK by me. -- Eric Botcazou
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)
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.jj2017-06-08 20:50:49.0
> > > +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
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
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)
On Tue, 25 Jul 2017, Matthew Fortune wrote: > Jakub Jelinek writes: > > Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves > > e.g. the code generation for slp-43.c and slp-45.c testcases. > > make cc1 tested in cross-compilers to the remaining targets. > > No objections for the MIPS part. I've pointed out this change to Sameera to > see how/if it will affect her autovectorization branch and whether MIPS MSA > should define more forms of vec_init/vec_expand in general. Note the vectorizer will try both variants (punning via integer mode and vector element) so the change is mostly to get around having very large integer modes like OImode of AVX512 halves or even larger ones for ARM SVE. You should only need to have matching component mode vector part inits/extracts (and all vector modes are power-of-two size). Thus for V8SI have V4SI, V2SI and SI components for init/extracts. If those are not available the vectorizer will try to pun V8SI to V2TI and to TImode init/extract or V4DI with DImode, etc. (that's what the bulk conversion of targets besides x86 should provide). Richard.
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)
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
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.0 +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
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)
Jakub Jelinek writes: > Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves > e.g. the code generation for slp-43.c and slp-45.c testcases. > make cc1 tested in cross-compilers to the remaining targets. No objections for the MIPS part. I've pointed out this change to Sameera to see how/if it will affect her autovectorization branch and whether MIPS MSA should define more forms of vec_init/vec_expand in general. Thanks, Matthew
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)
Hi Jakub,
On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote:
> The following patch adjusts the vec_init and vec_extract optabs, so that
> they don't have in the expander names just the vector mode, but also another
> mode, for vec_extract the mode of the result and for vec_init the mode of
> the elts of the vector passed as second operand.
>
> Without this patch, the second mode has been implicit, GET_MODE_INNER of
> the vector mode, so one could just extract a single element from a vector
> or construct vector from elements. While that is most common, we allow
> in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc.
> and the vectorizer uses them. By having the second mode in the name
> it allows the generic code (vectorizer, expansion) to query whether the
> backend supports such vector from vector expansions or inits from vector
> elts and use them if available.
>
> For vec_extract, if we say want to extract high V2SImode from V4SImode
> the fallback is try to expand it as DImode extraction from V2DImode.
> This works well in many cases, but doesn't really work for very large
> vectors, say if we want to extract high V8SImode from V16SImode on x86,
> we'd need OImode extraction from V2OImode, which is something the backend
> doesn't have any support for.
> For vec_init, the fallback is usually to go through memory, which is slow in
> many cases.
>
> 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?
> --- gcc/config/rs6000/vector.md.jj2017-06-08 20:50:49.0 +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).
("vel" instead of "Vel" for this name?)
How is this different from VEC_base_l? They can just be merged it seems.
(And for that matter, VEC_base and VEL as well). We'll handle that I
suppose, don't let it hold up this patch :-)
Segher
