[PATCH v3] Extend the simd function attribute

[Francesco Petrogalli]

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], &sin[i], &cos[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  <szabolcs.nagy@arm.com>
>>
>> 	* 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  <szabolcs.nagy@arm.com>
>>
>> 	* c-attribs.c (handle_simd_attribute): Handle 4 arguments.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-11-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>> 	* 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.
>>