Re: [PATCH v3] Extend the simd function attribute
> On Nov 21, 2019, at 4:09 PM, Richard Sandiford
> wrote:
>
> This probably isn't helpful, sorry, but the fact that the second point
> is a (reasonable) area of debate IMO shows the problems with doing
> the first point. If we go for (1) but not (2), everyone will need
> to specify the four initial parameters. But we'd have to allow (2)
> to be optional if we added it later. We might in future also want other
> parameters, e.g. for uniform arguments, by-value vs. by-ref, etc.
>
> Maybe we could have key-value pairs?
>
> __attribute__((__simd__("simdlen", 2, "simdabi", "n")))
>
> This would also allow linear parameters to be specified like this:
>
> __attribute__((__simd__("simdlen", 2, "linear", 2, "linear", 3, ...)))
>
> The current attribute syntax doesn't allow { 1, 2 } as an attribute
> parameter, and adding it now wouldn't be backward compatible (since
> older toolchains should be free to ignore this).
>
If we cannot do { 1, 2 }, I’d rather have the following syntax
__attribute__((__simd__(“name”, [not]inbranch, simdlen(2), simdabi(“n”),
linear(ref(1)[:step], uval(2)[:step],…), linear_pos(ref(1)[:pos],
uval(2)[:pos],…), uniform(4,5…), align(8,9[:val]
With no_linear, no_linear_pos, no_uniform and no_align to be specified when not
used, and step/pos/val with the same default values as in the OpenMP Standard
when not specified.
With this, all that we need to implement (for now) for sincos is (just use
no_linear for sin/exp/log...)
__attribute__((__simd__(“name”, [not]inbranch, simdlen(2), simdabi(“n”),
linear(1,2), no_linear_pos, no_uniform, no_align
(Btw, happy for all those values to be strings if we cannot use identifiers)
__attribute__((__simd__(“name”, "[not]inbranch", "simdlen(2)", "simdabi(n)",
"linear(1,2)", “no_linear_pos", “no_uniform", “no_align"
> I realise the key-value approach makes things more complicated though.
> I guess we're in danger of reinventing omp declare variant that way. :-)
>
Yep :)
How much this is gonna be useful beyond “flat vectorization” and sincos
vectorization is debatable (users can use OpenMP for this...), but at least we
would have some ground for expanding it if / when needed, with backward
compatibility in mind.
> I'm in two minds about whether we should expose the simdabi as the
> mangling character. On the one hand it's simple, on the other it seems
> a bit too low-level for something like this. I think we'd need to check
> whether the parameter is valid anyway, so maybe we should have some more
> user-friendly mnemonics?
>
No preference here, if not for the fact that the mangling character is already
encoded in the specs, so there is no space for interpretation: all we need to
do is to refer to the Vector Function ABIs.
Thanks!
Francesco
Re: [PATCH v3] Extend the simd function attribute
Szabolcs Nagy writes:
> On 14/11/2019 20:23, Szabolcs Nagy wrote:
>> Sorry v2 had a bug.
>>
>> v2: added documentation and tests.
>> v3: fixed expand_simd_clones so different isa variants are actually
>> generated.
>>
>> GCC currently supports two ways to declare the availability of vector
>> variants of a scalar function:
>>
>> #pragma omp declare simd
>> void f (void);
>>
>> and
>>
>> __attribute__ ((simd))
>> void f (void);
>>
>> However these declare a set of symbols that are different simd variants
>> of f, so a library either provides definitions for all those symbols or
>> it cannot use these declarations. (The set of declared symbols can be
>> narrowed down with additional omp clauses, but not enough to allow
>> declaring a single symbol. OpenMP 5 has a declare variant feature that
>> allows declaring more specific simd variants, but it is complicated and
>> still requires gcc or vendor extension for unambiguous declarations.)
>>
>> This patch extends the gcc specific simd attribute such that it can
>> specify a single vector variant of simple scalar functions (functions
>> that only take and return scalar integer or floating type values):
>>
>> __attribute__ ((simd (mask, simdlen, simdabi, name
>
> ping.
>
> so the comments so far
>
> - make all attribute arguments mandatory (e.g don't allow
> simd(mask, simdlen)), this is fine with me if others agree.
>
> - support the linear clause for pointer arguments (sincos).
> this requires listing arguments to which linear applies,
> i would only want to do that if there is a hope that it
> will ever be useful (currently gcc won't vectorize calls
> with pointer arguments, but maybe it should?). i don't know
> of a precedent for "list of integers" used in the attribute
> syntax, so i wonder what's the right way to do it.
This probably isn't helpful, sorry, but the fact that the second point
is a (reasonable) area of debate IMO shows the problems with doing
the first point. If we go for (1) but not (2), everyone will need
to specify the four initial parameters. But we'd have to allow (2)
to be optional if we added it later. We might in future also want other
parameters, e.g. for uniform arguments, by-value vs. by-ref, etc.
Maybe we could have key-value pairs?
__attribute__((__simd__("simdlen", 2, "simdabi", "n")))
This would also allow linear parameters to be specified like this:
__attribute__((__simd__("simdlen", 2, "linear", 2, "linear", 3, ...)))
The current attribute syntax doesn't allow { 1, 2 } as an attribute
parameter, and adding it now wouldn't be backward compatible (since
older toolchains should be free to ignore this).
I realise the key-value approach makes things more complicated though.
I guess we're in danger of reinventing omp declare variant that way. :-)
I'm in two minds about whether we should expose the simdabi as the
mangling character. On the one hand it's simple, on the other it seems
a bit too low-level for something like this. I think we'd need to check
whether the parameter is valid anyway, so maybe we should have some more
user-friendly mnemonics?
Thanks,
Richard
Re: [PATCH v3] Extend the simd function attribute
On 11/20/19 7:54 AM, Szabolcs Nagy wrote:
> On 14/11/2019 20:23, Szabolcs Nagy wrote:
>> Sorry v2 had a bug.
>>
>> v2: added documentation and tests.
>> v3: fixed expand_simd_clones so different isa variants are actually
>> generated.
>>
>> GCC currently supports two ways to declare the availability of vector
>> variants of a scalar function:
>>
>>#pragma omp declare simd
>>void f (void);
>>
>> and
>>
>>__attribute__ ((simd))
>>void f (void);
>>
>> However these declare a set of symbols that are different simd variants
>> of f, so a library either provides definitions for all those symbols or
>> it cannot use these declarations. (The set of declared symbols can be
>> narrowed down with additional omp clauses, but not enough to allow
>> declaring a single symbol. OpenMP 5 has a declare variant feature that
>> allows declaring more specific simd variants, but it is complicated and
>> still requires gcc or vendor extension for unambiguous declarations.)
>>
>> This patch extends the gcc specific simd attribute such that it can
>> specify a single vector variant of simple scalar functions (functions
>> that only take and return scalar integer or floating type values):
>>
>>__attribute__ ((simd (mask, simdlen, simdabi, name
> ping.
>
> so the comments so far
>
> - make all attribute arguments mandatory (e.g don't allow
>simd(mask, simdlen)), this is fine with me if others agree.
works for me :)
> - support the linear clause for pointer arguments (sincos).
>this requires listing arguments to which linear applies,
>i would only want to do that if there is a hope that it
>will ever be useful (currently gcc won't vectorize calls
>with pointer arguments, but maybe it should?).
If the C attribute inherit the properties of `declare simd` (or the
`variant` equivalent), it means that the function can be invoked
concurrently. This to me is enough to say that the following loop is
vectorizable (provided that the presence of vector sincos is the only
condition that prevents the loop from vectorizing)
void sincos(double , double *, double *) __attribute__((simd(noinbranch,
2, {1,2} /*linear*/, "n", "_ZGVnN2vl8l8_sincos"));
...
for (int i=...)
sincos(in[i], [i], [i]);
So overall, yes, I think a compiler should vectorize this example.
Please let me know if I am missing anything.
Side question: what would be the behavior of the attribute when attached
to a function definition? Are you expecting the compiler to
auto-vectorize the function? Given that the attribute is needed only for
interfacing libraries, I wouldn't recommend to use to auto-vectorize
functions. I am asking because I think you mentioned that the attribute
mimics the OpenMP clause...
> i don't know
>of a precedent for "list of integers" used in the attribute
>syntax, so i wonder what's the right way to do it.
>
> - plain simd should have fixed abi for a given architecture:
>aarch64 can of course do this, but if we include sve, then
>libmvec with plain simd attr won't be testable with gcc-10
>since gcc-10 does not support simd attr for sve, so we still
>need the attribute extension to do work on libmvec.
>
> any more comments on supporting linear clause in simd attr?
> or if the posted patch is reasonable as is?
Hum, I can't find the patch update in your last message...
>> where mask is "inbranch" or "notinbranch" like now, simdlen is an int
>> with the same meaning as in omp declare simd and simdabi is a string
>> specifying the call ABI (which the intel vector ABI calls ISA). The
>> name is optional and allows a library to use a different symbol name
>> than what the vector ABI specifies.
>>
>> The simd attribute currently can be used for both declarations and
>> definitions, in the latter case the simd varaints of the function are
>> generated, which should work with the extended simd attribute too.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>>
>> gcc/ChangeLog:
>>
>> 2019-11-14 Szabolcs Nagy
>>
>> * cgraph.h (struct cgraph_simd_clone): Add simdname field.
>> * doc/extend.texi: Update the simd attribute documentation.
>> * tree.h (OMP_CLAUSE__SIMDABI__EXPR): Define.
>> (OMP_CLAUSE__SIMDNAME__EXPR): Define.
>> * tree.c (walk_tree_1): Handle new omp clauses.
>> * tree-core.h (enum omp_clause_code): Likewise.
>> * tree-nested.c (convert_nonlocal_omp_clauses): Likewise.
>> * tree-pretty-print.c (dump_omp_clause): Likewise.
>> * omp-low.c (scan_sharing_clauses): Likewise.
>> * omp-simd-clone.c (simd_clone_clauses_extract): Likewise.
>> (simd_clone_mangle): Handle simdname.
>> (expand_simd_clones): Reset vecsize_mangle when generating clones.
>> * config/aarch64/aarch64.c
>> (aarch64_simd_clone_compute_vecsize_and_simdlen): Warn about
>> unsupported SIMD ABI.
>> * config/i386/i386.c
>> (ix86_simd_clone_compute_vecsize_and_simdlen): Likewise.
>>
>>
Re: [PATCH v3] Extend the simd function attribute
On 14/11/2019 20:23, Szabolcs Nagy wrote: > Sorry v2 had a bug. > > v2: added documentation and tests. > v3: fixed expand_simd_clones so different isa variants are actually > generated. > > GCC currently supports two ways to declare the availability of vector > variants of a scalar function: > > #pragma omp declare simd > void f (void); > > and > > __attribute__ ((simd)) > void f (void); > > However these declare a set of symbols that are different simd variants > of f, so a library either provides definitions for all those symbols or > it cannot use these declarations. (The set of declared symbols can be > narrowed down with additional omp clauses, but not enough to allow > declaring a single symbol. OpenMP 5 has a declare variant feature that > allows declaring more specific simd variants, but it is complicated and > still requires gcc or vendor extension for unambiguous declarations.) > > This patch extends the gcc specific simd attribute such that it can > specify a single vector variant of simple scalar functions (functions > that only take and return scalar integer or floating type values): > > __attribute__ ((simd (mask, simdlen, simdabi, name ping. so the comments so far - make all attribute arguments mandatory (e.g don't allow simd(mask, simdlen)), this is fine with me if others agree. - support the linear clause for pointer arguments (sincos). this requires listing arguments to which linear applies, i would only want to do that if there is a hope that it will ever be useful (currently gcc won't vectorize calls with pointer arguments, but maybe it should?). i don't know of a precedent for "list of integers" used in the attribute syntax, so i wonder what's the right way to do it. - plain simd should have fixed abi for a given architecture: aarch64 can of course do this, but if we include sve, then libmvec with plain simd attr won't be testable with gcc-10 since gcc-10 does not support simd attr for sve, so we still need the attribute extension to do work on libmvec. any more comments on supporting linear clause in simd attr? or if the posted patch is reasonable as is? > > where mask is "inbranch" or "notinbranch" like now, simdlen is an int > with the same meaning as in omp declare simd and simdabi is a string > specifying the call ABI (which the intel vector ABI calls ISA). The > name is optional and allows a library to use a different symbol name > than what the vector ABI specifies. > > The simd attribute currently can be used for both declarations and > definitions, in the latter case the simd varaints of the function are > generated, which should work with the extended simd attribute too. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. > > gcc/ChangeLog: > > 2019-11-14 Szabolcs Nagy > > * cgraph.h (struct cgraph_simd_clone): Add simdname field. > * doc/extend.texi: Update the simd attribute documentation. > * tree.h (OMP_CLAUSE__SIMDABI__EXPR): Define. > (OMP_CLAUSE__SIMDNAME__EXPR): Define. > * tree.c (walk_tree_1): Handle new omp clauses. > * tree-core.h (enum omp_clause_code): Likewise. > * tree-nested.c (convert_nonlocal_omp_clauses): Likewise. > * tree-pretty-print.c (dump_omp_clause): Likewise. > * omp-low.c (scan_sharing_clauses): Likewise. > * omp-simd-clone.c (simd_clone_clauses_extract): Likewise. > (simd_clone_mangle): Handle simdname. > (expand_simd_clones): Reset vecsize_mangle when generating clones. > * config/aarch64/aarch64.c > (aarch64_simd_clone_compute_vecsize_and_simdlen): Warn about > unsupported SIMD ABI. > * config/i386/i386.c > (ix86_simd_clone_compute_vecsize_and_simdlen): Likewise. > > gcc/c-family/ChangeLog: > > 2019-11-14 Szabolcs Nagy > > * c-attribs.c (handle_simd_attribute): Handle 4 arguments. > > gcc/testsuite/ChangeLog: > > 2019-11-14 Szabolcs Nagy > > * c-c++-common/attr-simd-5.c: Update. > * c-c++-common/attr-simd-6.c: New test. > * c-c++-common/attr-simd-7.c: New test. > * c-c++-common/attr-simd-8.c: New test. >
Re: [PATCH v3] Extend the simd function attribute
> On Nov 15, 2019, at 10:01 AM, Szabolcs Nagy wrote:
>
> On 15/11/2019 15:05, Francesco Petrogalli wrote:
>> Thank you Szabolcs for working on this.
>>
>>> On Nov 14, 2019, at 2:23 PM, Szabolcs Nagy wrote:
>>>
>>> Sorry v2 had a bug.
>>>
>>> v2: added documentation and tests.
>>> v3: fixed expand_simd_clones so different isa variants are actually
>>> generated.
>>>
>>> GCC currently supports two ways to declare the availability of vector
>>> variants of a scalar function:
>>>
>>> #pragma omp declare simd
>>> void f (void);
>>>
>>> and
>>>
>>> __attribute__ ((simd))
>>> void f (void);
>>>
>>> However these declare a set of symbols that are different simd variants
>>> of f, so a library either provides definitions for all those symbols or
>>> it cannot use these declarations. (The set of declared symbols can be
>>> narrowed down with additional omp clauses, but not enough to allow
>>> declaring a single symbol. OpenMP 5 has a declare variant feature that
>>> allows declaring more specific simd variants, but it is complicated and
>>> still requires gcc or vendor extension for unambiguous declarations.)
>>>
>>
>> It is not just that it is complicated, it is also a good idea to make math
>> function vectorization orthogonal to OpenMP.
>> This is needed din clang too, thank you for shaping a solution. It would be
>> good if we could come up with a common solution!
>>
>>> This patch extends the gcc specific simd attribute such that it can
>>> specify a single vector variant of simple scalar functions (functions
>>> that only take and return scalar integer or floating type values):
>>>
>>>
>>>
>>> where mask is "inbranch" or "notinbranch" like now, simdlen is an int
>>> with the same meaning as in omp declare simd and simdabi is a string
>>> specifying the call ABI (which the intel vector ABI calls ISA). The
>>> name is optional and allows a library to use a different symbol name
>>> than what the vector ABI specifies.
>>>
>>
>> Can we have also handling also the simplest case for the use of linear (just
>> with step = OpenMP default = 1)? It will be useful for `sincos`.
>>
>> OpenMP `linear` uses parameters name, in the attribute, which applies to
>> declaration with unnamed parameters, we could use positions.
>>
>> Also, can we avoid making the attribute a varargs attribute, but requires
>> all arguments to be present? We could use dummy values when the descriptor
>> is not needed (e.g. `no_linear`).
>>
>> I would also require the name to be specified, with the additional
>> requirement to make the declaration of name visible in the same compilation
>> unit.
>>
>> For example, on Arm, mapping `sincos` and `exp` to unmasked vector versions
>> with 2 lanes would be:
>>
>> ```
>> void _ZGVnN2vl8l8_sincos(float64x2_t, double *, double *);
>>
>> void sincos(double, double*, double *)
>> __attribute__(simd(notinbranch,2,”simd”,_ZGVnN2vl8l8_sincos, linear(2,3)));
>>
>> void _ZGVnN2v_exp(float64x2_t);
>>
>> void exp(double) __attribute__(simd(notinbranch,2,”simd”,_ZGVnN2v_exp,
>> no_linear));
>> ```
>
>
> note that exp returns double.
>
Facepalm myself. Sorry.
The examples still holds though, replacing `void` with `double` for `exp`.
> the simdabi is "n" instead of "simd", it is the
> 'ISA character' from the vector abi, to make it clear
> how it is tied to the vector abi name mangling.
>
Good choice, I like it more.
> pre-declaration of the simd symbol is not required,
> since the current simd attr (and omp pragma) works
> that way (e.g. it means simd types need not be visible
> where it is used, which may be tricky if they need
> target specific header inclusion).
>
Fair enough.
> notinbranch is a string in quotes, to avoid macro
> name collision in standard headers.
>
Fine by me.
> the same issue applies to linear, so if we add such
> argument it must not use bare 'linear' identifier, e.g.
> it could be an int list like
>
> simd("notinbranch", 2, "n", {2,3}, "vsincos")
>
I like it. Can it be an empty `{}` when no linear parameters are available?
> or we could just specify the vector abi name and
> reverse engineer the requirements from that:
>
> simd("", "_ZGVnN2vl8l8_sincos", "vsincos")
>
> (first argument is just to disambiguate from the mask form)
>
> i considered this name demangling more difficult to
> implement and error prone,
The demangling is not that difficult to implement:
https://reviews.llvm.org/D66024
But I agree that relying on it at user level is more error prone. The users of
the compiler should stay away from the ABI as much as possible.
> but it unambiguously
> specifies a single vector abi symbol for all future
> vector abi extensions.
>
Future extensions should be covered by a new ISA token, so I think we are good
with your solution.
> but there is a bigger problem with linear: currently
> the vectorizer only considers vector clones if they
> are 'const' or the vectorization of a loop was
> explicitly requested with omp
Re: [PATCH v3] Extend the simd function attribute
On 15/11/2019 15:05, Francesco Petrogalli wrote:
> Thank you Szabolcs for working on this.
>
>> On Nov 14, 2019, at 2:23 PM, Szabolcs Nagy wrote:
>>
>> Sorry v2 had a bug.
>>
>> v2: added documentation and tests.
>> v3: fixed expand_simd_clones so different isa variants are actually
>>generated.
>>
>> GCC currently supports two ways to declare the availability of vector
>> variants of a scalar function:
>>
>> #pragma omp declare simd
>> void f (void);
>>
>> and
>>
>> __attribute__ ((simd))
>> void f (void);
>>
>> However these declare a set of symbols that are different simd variants
>> of f, so a library either provides definitions for all those symbols or
>> it cannot use these declarations. (The set of declared symbols can be
>> narrowed down with additional omp clauses, but not enough to allow
>> declaring a single symbol. OpenMP 5 has a declare variant feature that
>> allows declaring more specific simd variants, but it is complicated and
>> still requires gcc or vendor extension for unambiguous declarations.)
>>
>
> It is not just that it is complicated, it is also a good idea to make math
> function vectorization orthogonal to OpenMP.
> This is needed din clang too, thank you for shaping a solution. It would be
> good if we could come up with a common solution!
>
>> This patch extends the gcc specific simd attribute such that it can
>> specify a single vector variant of simple scalar functions (functions
>> that only take and return scalar integer or floating type values):
>>
>>
>>
>> where mask is "inbranch" or "notinbranch" like now, simdlen is an int
>> with the same meaning as in omp declare simd and simdabi is a string
>> specifying the call ABI (which the intel vector ABI calls ISA). The
>> name is optional and allows a library to use a different symbol name
>> than what the vector ABI specifies.
>>
>
> Can we have also handling also the simplest case for the use of linear (just
> with step = OpenMP default = 1)? It will be useful for `sincos`.
>
> OpenMP `linear` uses parameters name, in the attribute, which applies to
> declaration with unnamed parameters, we could use positions.
>
> Also, can we avoid making the attribute a varargs attribute, but requires all
> arguments to be present? We could use dummy values when the descriptor is not
> needed (e.g. `no_linear`).
>
> I would also require the name to be specified, with the additional
> requirement to make the declaration of name visible in the same compilation
> unit.
>
> For example, on Arm, mapping `sincos` and `exp` to unmasked vector versions
> with 2 lanes would be:
>
> ```
> void _ZGVnN2vl8l8_sincos(float64x2_t, double *, double *);
>
> void sincos(double, double*, double *)
> __attribute__(simd(notinbranch,2,”simd”,_ZGVnN2vl8l8_sincos, linear(2,3)));
>
> void _ZGVnN2v_exp(float64x2_t);
>
> void exp(double) __attribute__(simd(notinbranch,2,”simd”,_ZGVnN2v_exp,
> no_linear));
> ```
note that exp returns double.
the simdabi is "n" instead of "simd", it is the
'ISA character' from the vector abi, to make it clear
how it is tied to the vector abi name mangling.
pre-declaration of the simd symbol is not required,
since the current simd attr (and omp pragma) works
that way (e.g. it means simd types need not be visible
where it is used, which may be tricky if they need
target specific header inclusion).
notinbranch is a string in quotes, to avoid macro
name collision in standard headers.
the same issue applies to linear, so if we add such
argument it must not use bare 'linear' identifier, e.g.
it could be an int list like
simd("notinbranch", 2, "n", {2,3}, "vsincos")
or we could just specify the vector abi name and
reverse engineer the requirements from that:
simd("", "_ZGVnN2vl8l8_sincos", "vsincos")
(first argument is just to disambiguate from the mask form)
i considered this name demangling more difficult to
implement and error prone, but it unambiguously
specifies a single vector abi symbol for all future
vector abi extensions.
but there is a bigger problem with linear: currently
the vectorizer only considers vector clones if they
are 'const' or the vectorization of a loop was
explicitly requested with omp simd pragma. so functions
with pointer arguments won't be used for vectorization.
is that different in clang? or expected to change somehow?
Re: [PATCH v3] Extend the simd function attribute
On 11/15/19 4:05 PM, Francesco Petrogalli wrote: Thank you Szabolcs for working on this. OpenMP 5 has a declare variant feature that allows declaring more specific simd variants, but it is complicated and still requires gcc or vendor extension for unambiguous declarations.) It is not just that it is complicated, it is also a good idea to make math function vectorization orthogonal to OpenMP. Definitely agree. I always found it a strained relationship, and only supported it being done that way because it worked. -- Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/ Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news
Re: [PATCH v3] Extend the simd function attribute
Thank you Szabolcs for working on this. > On Nov 14, 2019, at 2:23 PM, Szabolcs Nagy wrote: > > Sorry v2 had a bug. > > v2: added documentation and tests. > v3: fixed expand_simd_clones so different isa variants are actually >generated. > > GCC currently supports two ways to declare the availability of vector > variants of a scalar function: > > #pragma omp declare simd > void f (void); > > and > > __attribute__ ((simd)) > void f (void); > > However these declare a set of symbols that are different simd variants > of f, so a library either provides definitions for all those symbols or > it cannot use these declarations. (The set of declared symbols can be > narrowed down with additional omp clauses, but not enough to allow > declaring a single symbol. OpenMP 5 has a declare variant feature that > allows declaring more specific simd variants, but it is complicated and > still requires gcc or vendor extension for unambiguous declarations.) > It is not just that it is complicated, it is also a good idea to make math function vectorization orthogonal to OpenMP. This is needed din clang too, thank you for shaping a solution. It would be good if we could come up with a common solution! > This patch extends the gcc specific simd attribute such that it can > specify a single vector variant of simple scalar functions (functions > that only take and return scalar integer or floating type values): > > > > where mask is "inbranch" or "notinbranch" like now, simdlen is an int > with the same meaning as in omp declare simd and simdabi is a string > specifying the call ABI (which the intel vector ABI calls ISA). The > name is optional and allows a library to use a different symbol name > than what the vector ABI specifies. > Can we have also handling also the simplest case for the use of linear (just with step = OpenMP default = 1)? It will be useful for `sincos`. OpenMP `linear` uses parameters name, in the attribute, which applies to declaration with unnamed parameters, we could use positions. Also, can we avoid making the attribute a varargs attribute, but requires all arguments to be present? We could use dummy values when the descriptor is not needed (e.g. `no_linear`). I would also require the name to be specified, with the additional requirement to make the declaration of name visible in the same compilation unit. For example, on Arm, mapping `sincos` and `exp` to unmasked vector versions with 2 lanes would be: ``` void _ZGVnN2vl8l8_sincos(float64x2_t, double *, double *); void sincos(double, double*, double *) __attribute__(simd(notinbranch,2,”simd”,_ZGVnN2vl8l8_sincos, linear(2,3))); void _ZGVnN2v_exp(float64x2_t); void exp(double) __attribute__(simd(notinbranch,2,”simd”,_ZGVnN2v_exp, no_linear)); ``` For scalable vectorization on SVE, this could be rendered as (note that SVE requires mask parameter to be present independently of the [not]inbranch clause, see [1]): ``` void _ZGVsMxvl8l8_sincos(svfloat64_t, double *, double *, svbool_t); void sincos(double, double*, double *) __attribute__(simd(inbranch,scalable,”sve”,_ZGVsMxvl8l8_sincos, linear(2,3))); void _ZGVsNxv_exp(svfloat64_t, svbool_t); void exp(double) __attribute__(simd(inbranch,scalable,”sve”,_ZGVsMxv_exp, no_linear)); ``` SVE specific note: the “scalable” token could be replaced with 0. My preference would be for using scalable as a keywords, because 0 is not allowed by OpenMP (and might also be misleading). Kind regards, Francesco [1] https://github.com/ARM-software/software-standards/blob/master/abi/vfabia64/vfabia64.rst#masking > The simd attribute currently can be used for both declarations and > definitions, in the latter case the simd varaints of the function are > generated, which should work with the extended simd attribute too. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. > > gcc/ChangeLog: > > 2019-11-14 Szabolcs Nagy > > * cgraph.h (struct cgraph_simd_clone): Add simdname field. > * doc/extend.texi: Update the simd attribute documentation. > * tree.h (OMP_CLAUSE__SIMDABI__EXPR): Define. > (OMP_CLAUSE__SIMDNAME__EXPR): Define. > * tree.c (walk_tree_1): Handle new omp clauses. > * tree-core.h (enum omp_clause_code): Likewise. > * tree-nested.c (convert_nonlocal_omp_clauses): Likewise. > * tree-pretty-print.c (dump_omp_clause): Likewise. > * omp-low.c (scan_sharing_clauses): Likewise. > * omp-simd-clone.c (simd_clone_clauses_extract): Likewise. > (simd_clone_mangle): Handle simdname. > (expand_simd_clones): Reset vecsize_mangle when generating clones. > * config/aarch64/aarch64.c > (aarch64_simd_clone_compute_vecsize_and_simdlen): Warn about > unsupported SIMD ABI. > * config/i386/i386.c > (ix86_simd_clone_compute_vecsize_and_simdlen): Likewise. > > gcc/c-family/ChangeLog: > > 2019-11-14 Szabolcs Nagy > > * c-attribs.c (handle_simd_attribute): Handle
[PATCH v3] Extend the simd function attribute
Sorry v2 had a bug.
v2: added documentation and tests.
v3: fixed expand_simd_clones so different isa variants are actually
generated.
GCC currently supports two ways to declare the availability of vector
variants of a scalar function:
#pragma omp declare simd
void f (void);
and
__attribute__ ((simd))
void f (void);
However these declare a set of symbols that are different simd variants
of f, so a library either provides definitions for all those symbols or
it cannot use these declarations. (The set of declared symbols can be
narrowed down with additional omp clauses, but not enough to allow
declaring a single symbol. OpenMP 5 has a declare variant feature that
allows declaring more specific simd variants, but it is complicated and
still requires gcc or vendor extension for unambiguous declarations.)
This patch extends the gcc specific simd attribute such that it can
specify a single vector variant of simple scalar functions (functions
that only take and return scalar integer or floating type values):
__attribute__ ((simd (mask, simdlen, simdabi, name
where mask is "inbranch" or "notinbranch" like now, simdlen is an int
with the same meaning as in omp declare simd and simdabi is a string
specifying the call ABI (which the intel vector ABI calls ISA). The
name is optional and allows a library to use a different symbol name
than what the vector ABI specifies.
The simd attribute currently can be used for both declarations and
definitions, in the latter case the simd varaints of the function are
generated, which should work with the extended simd attribute too.
Tested on aarch64-linux-gnu and x86_64-linux-gnu.
gcc/ChangeLog:
2019-11-14 Szabolcs Nagy
* cgraph.h (struct cgraph_simd_clone): Add simdname field.
* doc/extend.texi: Update the simd attribute documentation.
* tree.h (OMP_CLAUSE__SIMDABI__EXPR): Define.
(OMP_CLAUSE__SIMDNAME__EXPR): Define.
* tree.c (walk_tree_1): Handle new omp clauses.
* tree-core.h (enum omp_clause_code): Likewise.
* tree-nested.c (convert_nonlocal_omp_clauses): Likewise.
* tree-pretty-print.c (dump_omp_clause): Likewise.
* omp-low.c (scan_sharing_clauses): Likewise.
* omp-simd-clone.c (simd_clone_clauses_extract): Likewise.
(simd_clone_mangle): Handle simdname.
(expand_simd_clones): Reset vecsize_mangle when generating clones.
* config/aarch64/aarch64.c
(aarch64_simd_clone_compute_vecsize_and_simdlen): Warn about
unsupported SIMD ABI.
* config/i386/i386.c
(ix86_simd_clone_compute_vecsize_and_simdlen): Likewise.
gcc/c-family/ChangeLog:
2019-11-14 Szabolcs Nagy
* c-attribs.c (handle_simd_attribute): Handle 4 arguments.
gcc/testsuite/ChangeLog:
2019-11-14 Szabolcs Nagy
* c-c++-common/attr-simd-5.c: Update.
* c-c++-common/attr-simd-6.c: New test.
* c-c++-common/attr-simd-7.c: New test.
* c-c++-common/attr-simd-8.c: New test.
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index c62cebf7bfd..bf2301eb790 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -448,7 +448,7 @@ const struct attribute_spec c_common_attribute_table[] =
handle_omp_declare_variant_attribute, NULL },
{ "omp declare variant variant", 0, -1, true, false, false, false,
handle_omp_declare_variant_attribute, NULL },
- { "simd", 0, 1, true, false, false, false,
+ { "simd", 0, 4, true, false, false, false,
handle_simd_attribute, NULL },
{ "omp declare target", 0, -1, true, false, false, false,
handle_omp_declare_target_attribute, NULL },
@@ -3094,13 +3094,22 @@ handle_simd_attribute (tree *node, tree name, tree args, int, bool *no_add_attrs
{
tree t = get_identifier ("omp declare simd");
tree attr = NULL_TREE;
+
+ /* Allow
+ simd
+ simd (mask)
+ simd (mask, simdlen)
+ simd (mask, simdlen, simdabi)
+ simd (mask, simdlen, simdabi, name)
+ forms. */
+
if (args)
{
tree id = TREE_VALUE (args);
if (TREE_CODE (id) != STRING_CST)
{
- error ("attribute %qE argument not a string", name);
+ error ("attribute %qE first argument not a string", name);
*no_add_attrs = true;
return NULL_TREE;
}
@@ -3113,13 +3122,75 @@ handle_simd_attribute (tree *node, tree name, tree args, int, bool *no_add_attrs
OMP_CLAUSE_INBRANCH);
else
{
- error ("only % and % flags are "
- "allowed for %<__simd__%> attribute");
+ error ("%qE attribute first argument must be % or "
+ "%", name);
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ args = TREE_CHAIN (args);
+ }
+
+ if (args)
+ {
+ tree arg = TREE_VALUE (args);
+
+ arg = c_fully_fold (arg, false, NULL);
+ if (TREE_CODE (arg) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (arg))
+ || tree_int_cst_sgn
