Hello, On 07 Oct 11:09, Jeff Law wrote: > On 10/05/2015 07:24 AM, Joseph Myers wrote: > >On Mon, 5 Oct 2015, Kirill Yukhin wrote: > > > >>To enable vectorization of loops w/ calls to math functions it is reasonable > >>to enable parsing of attribute vector for functions unconditionally and > >>change GlibC's header file not to use `omp declare simd', but use > >>__attribute__((vector)) instead. > > > >I assume you mean __vector__, for namespace reasons. Obviously you need > >appropriate GCC version conditionals in the headers to use the attribute > >only when supported. In addition, (a) this attribute doesn't seem to be > >documented in extend.texi, and you'll need to include documentation in > >your GCC patch that makes this a generic extension rather than just part > >of Cilkplus, and (b) you'll need to agree with the x86_64 ABI mailing list > >an extension of the ABI document (as attached to > ><https://sourceware.org/glibc/wiki/libmvec>) to cover this attribute, and > >update the document there. > I'm not sure why this attribute isn't documented, but clearly that should be > fixed. > > From the GCC side, I don't see a compelling reason to keep this attribute > conditional on Cilk+ support. One could very easily want to use the math > library's vector entry points independent of OpenMP or Cilk+. > > I thought the ABI for this stuff was consistent across the implementations > (that was certainly the goal). So aside from an example of how to use the > attribute to get calls into the vector math library, I'm not sure what's > needed. Essentially the attribute is just another way to ensure we exploit > the vector library when possible. > > It also seems to me that showing that example on the libmvec page would be > advisable.
Patch in the bottom introduces new attribute called `simd'. I've decided to introduce a new one since Cilk's `vector' attribute generates only one version of SIMD-enabled function [1] (which one - implementation specific). This new attribute shouldn't be used along w/ Cilk's `vector'. If it is used w/ `pragma omp declare simd' - it is ignored. Bootstrapped. New tests pass. gcc/ * c/c-parser.c (c_parser): Add simd_attr_present flag. (c_parser_declaration_or_fndef): Call c_parser_declaration_or_fndef if simd_attr_present is set. (c_finish_omp_declare_simd): Handle simd_attr_present. * omp-low.c (pass_omp_simd_clone::gate): If target allows - call without additional conditions. gcc/testsuite/ * c-c++-common/attr-simd.c: New test. * c-c++-common/attr-simd-2.c: Ditto. * c-c++-common/attr-simd-3.c: Ditto. Is it ok for trunk? Here is a description for new attribute: simd Enables creation of one or more versions that can process multiple arguments using SIMD instructions from a single invocation from a SIMD loop. It is ultimately an alias to `omp declare simd’ pragma, available w/o additional compiler switches. It is prohibited to use the attribute along with Cilk Plus’s `vector’ attribute. If the attribute is specified and `pragma omp declare simd’ presents on a decl, then the attribute is ignored. [1] - https://www.cilkplus.org/sites/default/files/open_specifications/Intel_Cilk_plus_lang_spec_1.2.htm#elem.attr -- Thanks, K diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 2d24c21..b83c9d8 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -224,6 +224,9 @@ struct GTY(()) c_parser { /* Buffer to hold all the tokens from parsing the vector attribute for the SIMD-enabled functions (formerly known as elemental functions). */ vec <c_token, va_gc> *cilk_simd_fn_tokens; + + /* Designates if "simd" attribute is specified in decl. */ + BOOL_BITFIELD simd_attr_present : 1; }; @@ -1700,7 +1703,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, if (declarator == NULL) { if (omp_declare_simd_clauses.exists () - || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) + || !vec_safe_is_empty (parser->cilk_simd_fn_tokens) + || parser->simd_attr_present) c_finish_omp_declare_simd (parser, NULL_TREE, NULL_TREE, omp_declare_simd_clauses); c_parser_skip_to_end_of_block_or_statement (parser); @@ -1796,7 +1800,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, if (!d) d = error_mark_node; if (omp_declare_simd_clauses.exists () - || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) + || !vec_safe_is_empty (parser->cilk_simd_fn_tokens) + || parser->simd_attr_present) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); } @@ -1809,7 +1814,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, if (!d) d = error_mark_node; if (omp_declare_simd_clauses.exists () - || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) + || !vec_safe_is_empty (parser->cilk_simd_fn_tokens) + || parser->simd_attr_present) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); start_init (d, asm_name, global_bindings_p ()); @@ -1838,7 +1844,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, chainon (postfix_attrs, all_prefix_attrs)); if (omp_declare_simd_clauses.exists () - || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) + || !vec_safe_is_empty (parser->cilk_simd_fn_tokens) + || parser->simd_attr_present) { tree parms = NULL_TREE; if (d && TREE_CODE (d) == FUNCTION_DECL) @@ -1967,7 +1974,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, true, false, NULL, vNULL); store_parm_decls (); if (omp_declare_simd_clauses.exists () - || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) + || !vec_safe_is_empty (parser->cilk_simd_fn_tokens) + || parser->simd_attr_present) c_finish_omp_declare_simd (parser, current_function_decl, NULL_TREE, omp_declare_simd_clauses); DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus @@ -3998,6 +4006,12 @@ c_parser_attributes (c_parser *parser) c_parser_cilk_simd_fn_vector_attrs (parser, *v_token); continue; } + if (is_attribute_p ("simd", attr_name)) + { + parser->simd_attr_present = 1; + c_parser_consume_token (parser); + continue; + } c_parser_consume_token (parser); if (c_parser_next_token_is_not (parser, CPP_OPEN_PAREN)) { @@ -14210,9 +14224,10 @@ c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms, vec<c_token> clauses) { if (flag_cilkplus - && clauses.exists () && !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) + && (clauses.exists () || parser->simd_attr_present) + && !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) { - 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"); vec_free (parser->cilk_simd_fn_tokens); return; @@ -14257,41 +14272,63 @@ c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms, parser->tokens = clauses.address (); parser->tokens_avail = clauses.length (); } - - /* c_parser_omp_declare_simd pushed 2 extra CPP_EOF tokens at the end. */ - while (parser->tokens_avail > 3) + + if (parser->simd_attr_present + && is_cilkplus_cilk_simd_fn) { - c_token *token = c_parser_peek_token (parser); - if (!is_cilkplus_cilk_simd_fn) - gcc_assert (token->type == CPP_NAME - && strcmp (IDENTIFIER_POINTER (token->value), "simd") == 0); - else - gcc_assert (token->type == CPP_NAME - && is_cilkplus_vector_p (token->value)); - c_parser_consume_token (parser); - parser->in_pragma = true; + error ("SIMD-enabled function attributes" + "are allowed when attribute __simd__ is specified"); + clauses[0].type = CPP_EOF; + return; + } + /* Attach `omp declare simd’ attribute if __simd__ is specified AND no OpenMP clauses + present in decl. */ + if (parser->simd_attr_present + && parser->tokens_avail == 0) + { tree c = NULL_TREE; - if (is_cilkplus_cilk_simd_fn) - c = c_parser_omp_all_clauses (parser, CILK_SIMD_FN_CLAUSE_MASK, - "SIMD-enabled functions attribute"); - else - c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK, - "#pragma omp declare simd"); - c = c_omp_declare_simd_clauses_to_numbers (parms, c); - if (c != NULL_TREE) - c = tree_cons (NULL_TREE, c, NULL_TREE); - if (is_cilkplus_cilk_simd_fn) - { - tree k = build_tree_list (get_identifier ("cilk simd function"), - NULL_TREE); - TREE_CHAIN (k) = DECL_ATTRIBUTES (fndecl); - DECL_ATTRIBUTES (fndecl) = k; - } c = build_tree_list (get_identifier ("omp declare simd"), c); TREE_CHAIN (c) = DECL_ATTRIBUTES (fndecl); DECL_ATTRIBUTES (fndecl) = c; } + else + { + /* c_parser_omp_declare_simd pushed 2 extra CPP_EOF tokens at the end. */ + while (parser->tokens_avail > 3) + { + c_token *token = c_parser_peek_token (parser); + if (!is_cilkplus_cilk_simd_fn) + gcc_assert (token->type == CPP_NAME + && strcmp (IDENTIFIER_POINTER (token->value), "simd") == 0); + else + gcc_assert (token->type == CPP_NAME + && is_cilkplus_vector_p (token->value)); + c_parser_consume_token (parser); + parser->in_pragma = true; + + tree c = NULL_TREE; + if (is_cilkplus_cilk_simd_fn) + c = c_parser_omp_all_clauses (parser, CILK_SIMD_FN_CLAUSE_MASK, + "SIMD-enabled functions attribute"); + else + c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK, + "#pragma omp declare simd"); + c = c_omp_declare_simd_clauses_to_numbers (parms, c); + if (c != NULL_TREE) + c = tree_cons (NULL_TREE, c, NULL_TREE); + if (is_cilkplus_cilk_simd_fn) + { + tree k = build_tree_list (get_identifier ("cilk simd function"), + NULL_TREE); + TREE_CHAIN (k) = DECL_ATTRIBUTES (fndecl); + DECL_ATTRIBUTES (fndecl) = k; + } + c = build_tree_list (get_identifier ("omp declare simd"), c); + TREE_CHAIN (c) = DECL_ATTRIBUTES (fndecl); + DECL_ATTRIBUTES (fndecl) = c; + } + } parser->tokens = &parser->tokens_buf[0]; parser->tokens_avail = tokens_avail; diff --git a/gcc/omp-low.c b/gcc/omp-low.c index cdcf9d6..87537cd 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -13919,10 +13919,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..263fa9a --- /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 "simdclone" "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..5dad89c --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-simd.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-fdump-tree-optimized" } */ + +__attribute__((__simd__)) +static int simd_attr (void) +{ + return 0; +} + +/* { dg-final { scan-tree-dump "simdclone" "optimized" } } */