This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, VECTOR ABI] Add __attribute__((__simd__)) to GCC.
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Kirill Yukhin <kirill dot yukhin at gmail dot com>
- Cc: Joseph Myers <joseph at codesourcery dot com>, Jeff Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 10 Nov 2015 09:58:04 +0100
- Subject: Re: [PATCH, VECTOR ABI] Add __attribute__((__simd__)) to GCC.
- Authentication-results: sourceware.org; auth=none
- References: <20151015144739 dot GB2191 at msticlxl57 dot ims dot intel dot com> <20151022115151 dot GA58371 at msticlxl57 dot ims dot intel dot com> <alpine dot DEB dot 2 dot 10 dot 1510221246280 dot 32382 at digraph dot polyomino dot org dot uk> <20151023141102 dot GA63293 at msticlxl57 dot ims dot intel dot com> <alpine dot DEB dot 2 dot 10 dot 1510231414210 dot 11051 at digraph dot polyomino dot org dot uk> <20151027140655 dot GC46998 at msticlxl57 dot ims dot intel dot com> <20151027141505 dot GG478 at tucnak dot redhat dot com> <20151028091601 dot GA53647 at msticlxl57 dot ims dot intel dot com> <20151029085447 dot GP478 at tucnak dot redhat dot com> <20151110084414 dot GA62112 at msticlxl57 dot ims dot intel dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, Nov 10, 2015 at 11:44:18AM +0300, Kirill Yukhin wrote:
> gcc/
> * omp-low.c (pass_omp_simd_clone::gate): If target allows - call
> without additional conditions.
Please make sure CHangeLog entries are tab indented. I would just use
a comma instead of " -" above.
> * c-c++-common/attr-simd.c: New test.
> * c-c++-common/attr-simd-2.c: Ditto.
> * c-c++-common/attr-simd-3.c: Ditto.
New test is IMHO short enough that it is better to spell it
in each case.
> bool *);
> +static tree handle_simd_attribute (tree *, tree, tree, int, bool *);
> static tree handle_omp_declare_target_attribute (tree *, tree, tree, int,
> bool *);
> static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
> @@ -818,6 +819,8 @@ const struct attribute_spec c_common_attribute_table[] =
> handle_omp_declare_simd_attribute, false },
> { "cilk simd function", 0, -1, true, false, false,
> handle_omp_declare_simd_attribute, false },
> + { "simd", 0, -1, true, false, false,
> + handle_simd_attribute, false },
Why the -1 in there? I thought the simd attribute has exactly zero
arguments, so 0, 0.
> +static tree
> +handle_simd_attribute (tree *node, tree name, tree ARG_UNUSED (args),
> + int ARG_UNUSED (flags), bool *no_add_attrs)
As per recent discussion, please leave ARG_UNUSED (args) etc. out, just
use unnamed arguments.
> +{
> + if (TREE_CODE (*node) == FUNCTION_DECL)
> + {
> + if (lookup_attribute ("cilk simd function", DECL_ATTRIBUTES (*node)) != NULL)
Too long line.
> + {
> + error_at (DECL_SOURCE_LOCATION (*node),
> + "%<__simd__%> attribute cannot be "
> + "used in the same function marked as a Cilk Plus SIMD-enabled function");
Too long line. You should just move "use in the same " one line above.
> + *no_add_attrs = true;
> + }
> + else
> + {
> + DECL_ATTRIBUTES (*node)
> + = tree_cons (get_identifier ("omp declare simd"),
> + NULL_TREE, DECL_ATTRIBUTES (*node));
> + }
Please avoid {}s around single statement in the body.
> {
> - error ("%<#pragma omp declare simd%> cannot be used in the same "
> + error ("%<#pragma omp declare simd%> or %<simd%> attribute cannot be used in the same "
> "function marked as a Cilk Plus SIMD-enabled function");
Too long line.
> + if (lookup_attribute ("simd", DECL_ATTRIBUTES (fndecl)) != NULL)
> + {
> + error_at (DECL_SOURCE_LOCATION (fndecl),
> + "%<__simd__%> attribute cannot be "
> + "used in the same function marked as a Cilk Plus SIMD-enabled function");
Too long line, see above.
> c = build_tree_list (get_identifier ("omp declare simd"), c);
> TREE_CHAIN (c) = DECL_ATTRIBUTES (fndecl);
> DECL_ATTRIBUTES (fndecl) = c;
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 7555bf3..f3831b9 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -34534,10 +34534,11 @@ cp_parser_late_parsing_cilk_simd_fn_info (cp_parser *parser, tree attrs)
> cp_omp_declare_simd_data *info = parser->cilk_simd_fn_info;
> int ii = 0;
>
> - if (parser->omp_declare_simd != NULL)
> + if (parser->omp_declare_simd != NULL
> + || lookup_attribute ("simd", attrs))
> {
> - error ("%<#pragma omp declare simd%> cannot be used in the same function"
> - " marked as a Cilk Plus SIMD-enabled function");
> + error ("%<#pragma omp declare simd%> of %<simd%> attribute cannot be used "
> + "in the same function marked as a Cilk Plus SIMD-enabled function");
Too long lines.
> XDELETE (parser->cilk_simd_fn_info);
> parser->cilk_simd_fn_info = NULL;
> return attrs;
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index fdb1547..32994a2 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -3066,6 +3066,21 @@ This function attribute make a stack protection of the function if
> flags @option{fstack-protector} or @option{fstack-protector-strong}
> or @option{fstack-protector-explicit} are set.
>
> +@item simd
> +@cindex @code{simd} function attribute.
> +This attribute enables creation of one or more function versions that
> +can process multiple arguments using SIMD instructions from a
> +single invocation. Specifying this attribute allows compiler to
> +assume that such a versions are available at link time (provided
> +in the same or another translation unit). Generated versions are
> +target dependent and described in corresponding Vector ABI document. For
> +x86_64 target this document can be found
> +@w{@uref{https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt,here}}.
Are you going to update VectorABI.txt based on the latest changes in
the Intel ABI document? I mean especially using L, R or U for linear
%val(), %ref() or %uval() (if references), not using s for linear with
uniform parameter stride, but instead using ls, Ls, Rs or Us for those?
> +It is prohibited to use the attribute along with Cilk Plus's @code{vector}
> +attribute. If the attribute is specified and @code{#pragma omp declare simd}
> +presented on a declaration and @code{-fopenmp} or @code{-fopenmp-simd}
> +switch is specified, then the attribute is ignored.
> +
> @item target (@var{options})
> @cindex @code{target} function attribute
> Multiple target back ends implement the @code{target} attribute
> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
> index ad7c017..232dc5c 100644
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -17412,10 +17412,7 @@ public:
> bool
> pass_omp_simd_clone::gate (function *)
> {
> - return ((flag_openmp || flag_openmp_simd
> - || flag_cilkplus
> - || (in_lto_p && !flag_wpa))
> - && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
> + return targetm.simd_clone.compute_vecsize_and_simdlen != NULL;
> }
>
> } // anon namespace
> diff --git a/gcc/testsuite/c-c++-common/attr-simd-2.c b/gcc/testsuite/c-c++-common/attr-simd-2.c
> new file mode 100644
> index 0000000..e9afc11
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-simd-2.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-optimized -fopenmp-simd" } */
> +
> +#pragma omp declare simd
> +__attribute__((__simd__))
> +static int simd_attr (void)
> +{
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump "omp declare simd" "optimized" } } */
> diff --git a/gcc/testsuite/c-c++-common/attr-simd-3.c b/gcc/testsuite/c-c++-common/attr-simd-3.c
> new file mode 100644
> index 0000000..2bbdf04
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-simd-3.c
> @@ -0,0 +1,5 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fcilkplus" } */
> +/* { dg-prune-output "undeclared here \\(not in a function\\)|\[^\n\r\]* was not declared in this scope" } */
> +
> +void f () __attribute__((__simd__, __vector__)); /* { dg-error "in the same function marked as a Cilk Plus" } */
> diff --git a/gcc/testsuite/c-c++-common/attr-simd.c b/gcc/testsuite/c-c++-common/attr-simd.c
> new file mode 100644
> index 0000000..6fd757a
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-simd.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-optimized" } */
> +
> +__attribute__((__simd__))
> +static int simd_attr (void)
> +{
> + return 0;
> +}
> +
> +__attribute__((simd))
> +static int simd_attr2 (void)
> +{
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized" } } */
> +/* { dg-final { scan-tree-dump "simd_attr2\[ \\t\]simdclone|vector" "optimized" } } */
You should add a few scan-assembler lines for x86_64/i?86 (see some
declare-simd*.{c,C} tests) to test mangling. Of course not when the
function is static...
Otherwise LGTM.
Jakub