Hi Jakub, Did you get a chance to look at this patch? Is it Ok to install?
Thanks, Balaji V. Iyer. > -----Original Message----- > From: Iyer, Balaji V > Sent: Tuesday, November 19, 2013 5:09 PM > To: Jakub Jelinek > Cc: Joseph S. Myers; gcc-patches@gcc.gnu.org; Aldy Hernandez > (al...@redhat.com); Jeff Law > Subject: RE: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental > functions) for C > > > > > -----Original Message----- > > From: Jakub Jelinek [mailto:ja...@redhat.com] > > Sent: Tuesday, November 19, 2013 12:00 PM > > To: Iyer, Balaji V > > Cc: Joseph S. Myers; gcc-patches@gcc.gnu.org; Aldy Hernandez > > (al...@redhat.com); Jeff Law > > Subject: Re: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly > > Elemental > > functions) for C > > > > On Tue, Nov 19, 2013 at 04:54:29PM +0000, Iyer, Balaji V wrote: > > > I just need a clarification, so I am sorry if I am making you repeat: > > > > > > Right now, the array of tokens (vec<c_token> elem_fn_tokens) is > > > passed in as a parameter and then passed back to the > > > c_parser_declaration_or_fndef function. > > > > > > Instead of that, you would like this information to be stored in > > > parser->specs (using the c_parser_declspecs function) and then have > > > parser->my own > > > > No, somewhere in *parser, it can be a new field with vector of the > > vec<c_token, va_gc> vectors or similar and store something that can be > > used to find that out into vector attribute argument. Then at the > > spot where the #pragma omp declare simd tokens are parsed right now > > (when arguments are finally available) you could look up the vector > > attribute and parse the corresponding tokens. > > Hi Jakub, > Thanks for all your help and advice on this. I have made the changes as > you > suggested. I added a new field to parser to hold the tokens for elemental > functions. Also removed the call for c_parser_attributes that I added in > c_parser_declaration_or_fndef and then let c_parser_declspec handle the > token collection. Also, I added a check for usage of #pragma omp declare > simd and __attribute__((vector...)) and flag them as error, also added a test > to catch this. > > Also, fixed the spelling mistake in a comment and the mis-comment I made > about OpenMP requiring two CPP_EOF. Is this Patch OK to install? > > Here is the Fixed ChangeLog entry: > gcc/c/ChangeLog > 2013-11-19 Balaji V. Iyer <balaji.v.i...@intel.com> > > * c-parser.c (struct c_parser::elem_fn_tokens): Added new field. > (c_parser_declaration_or_fndef): Added a check if elem_fn_tokens > field in parser is not empty. If not-empty, call the function > c_parser_finish_omp_declare_simd. > (c_parser_elem_fn_vectorlength): New function. > (c_parser_elem_fn_expr_list): Likewise. > (c_finish_elem_fn_tokens): Likewise. > (c_parser_attributes): Added a elem_fn_tokens parameter. Added a > check for vector attribute and if so call c_parser_elem_fn_expr_list. > Also, called c_finish_elem_fn_tokens when Cilk Plus is enabled. > (c_finish_omp_declare_simd): Added a check if elem_fn_tokens in > parser field is non-empty. If so, parse them as you would parse > the omp declare simd pragma. > > gcc/testsuite/ChangeLog > 2013-11-19 Balaji V. Iyer <balaji.v.i...@intel.com> > > * c-c++-common/cilk-plus/EF/ef_test.c: New test. > * c-c++-common/cilk-plus/EF/ef_test2.c: Likewise. > * c-c++-common/cilk-plus/EF/vlength_errors.c: Likewise. > * c-c++-common/cilk-plus/EF/ef_error.c: Likewise. > * gcc.dg/cilk-plus/cilk-plus.exp: Added calls for the above tests. > > Thanks, > > Balaji V. Iyer. > > > > > Jakub
Index: gcc/c/c-parser.c =================================================================== --- gcc/c/c-parser.c (revision 205055) +++ gcc/c/c-parser.c (working copy) @@ -203,6 +203,12 @@ /* True if we are in a context where the Objective-C "Property attribute" keywords are valid. */ BOOL_BITFIELD objc_property_attr_context : 1; + + /* Cilk Plus specific parser/lexer information. */ + + /* 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> *elem_fn_tokens; } c_parser; @@ -1642,7 +1648,8 @@ C_DTR_NORMAL, &dummy); if (declarator == NULL) { - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses.exists () + || !vec_safe_is_empty (parser->elem_fn_tokens)) c_finish_omp_declare_simd (parser, NULL_TREE, NULL_TREE, omp_declare_simd_clauses); c_parser_skip_to_end_of_block_or_statement (parser); @@ -1729,7 +1736,8 @@ chainon (postfix_attrs, all_prefix_attrs)); if (!d) d = error_mark_node; - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses.exists () + || !vec_safe_is_empty (parser->elem_fn_tokens)) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); } @@ -1741,7 +1749,8 @@ chainon (postfix_attrs, all_prefix_attrs)); if (!d) d = error_mark_node; - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses.exists () + || !vec_safe_is_empty (parser->elem_fn_tokens)) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); start_init (d, asm_name, global_bindings_p ()); @@ -1769,7 +1778,8 @@ tree d = start_decl (declarator, specs, false, chainon (postfix_attrs, all_prefix_attrs)); - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses.exists () + || !vec_safe_is_empty (parser->elem_fn_tokens)) { tree parms = NULL_TREE; if (d && TREE_CODE (d) == FUNCTION_DECL) @@ -1897,7 +1907,8 @@ c_parser_declaration_or_fndef (parser, false, false, false, true, false, NULL, vNULL); store_parm_decls (); - if (omp_declare_simd_clauses.exists ()) + if (omp_declare_simd_clauses.exists () + || !vec_safe_is_empty (parser->elem_fn_tokens)) c_finish_omp_declare_simd (parser, current_function_decl, NULL_TREE, omp_declare_simd_clauses); DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus @@ -3760,6 +3771,147 @@ return attr_name; } +/* Parses the vectorlength vector attribute for the SIMD Enabled functions + in Cilk Plus. + Syntax: + vectorlength (<integer constant expression>) */ + +static bool +c_parser_elem_fn_vectorlength (c_parser *parser) +{ + c_token *token = c_parser_peek_token (parser); + gcc_assert (simple_cst_equal (token->value, + get_identifier ("vectorlength")) == 1); + token->value = get_identifier ("simdlen"); + vec_safe_push (parser->elem_fn_tokens, *token); + + c_parser_consume_token (parser); + + token = c_parser_peek_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return false; + + vec_safe_push (parser->elem_fn_tokens, *token); + token = c_parser_peek_token (parser); + tree value = token->value; + if (!value) + { + error_at (token->location, "expected vectorlength value"); + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL); + return false; + } + c_parser_consume_token (parser); + if (TREE_CODE (value) != INTEGER_CST) + { + error_at (token->location, "vectorlength must be a constant integer"); + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL); + return false; + } + if (!integer_pow2p (value)) + { + error_at (token->location, "vectorlength must be a power of 2"); + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL); + return false; + } + vec_safe_push (parser->elem_fn_tokens, *token); + + token = c_parser_peek_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + vec_safe_push (parser->elem_fn_tokens, *token); + return true; +} + +/* Parses the vector attribute of SIMD enabled functions in Cilk Plus. + Syntax: + vector + vector (<vector attributes>). */ + +static void +c_parser_elem_fn_expr_list (c_parser *parser, c_token vec_token) +{ + gcc_assert (simple_cst_equal (vec_token.value, + get_identifier ("vector")) == 1); + int paren_scope = 0; + /* Replace the vector keyword with SIMD. */ + vec_token.value = get_identifier ("simd"); + vec_safe_push (parser->elem_fn_tokens, vec_token); + /* Consume the "vector" token. */ + c_parser_consume_token (parser); + + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)) + { + c_parser_consume_token (parser); + paren_scope++; + } + while (paren_scope > 0) + { + c_token *token = c_parser_peek_token (parser); + if (token->type == CPP_OPEN_PAREN) + paren_scope++; + else if (token->type == CPP_CLOSE_PAREN) + paren_scope--; + if (token->type == CPP_NAME + && TREE_CODE (token->value) == IDENTIFIER_NODE) + { + tree value = token->value; + if (simple_cst_equal (value, get_identifier ("mask")) == 1) + token->value = get_identifier ("inbranch"); + else if (simple_cst_equal (value, get_identifier ("nomask")) == 1) + token->value = get_identifier ("notinbranch"); + else if (simple_cst_equal (value, + get_identifier ("vectorlength")) == 1) + { + if (!c_parser_elem_fn_vectorlength (parser)) + { + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL); + /* NO reason to keep any of these tokens if the + vectorlength is messed up. */ + vec_free (parser->elem_fn_tokens); + // parser->elem_fn_tokens->release (); + return; + } + else + continue; + } + /* linear and uniform are the same between SIMD + enabled functions and #pragma omp declare simd. */ + } + /* Do not push the last ')' */ + if (!(token->type == CPP_CLOSE_PAREN && paren_scope == 0)) + vec_safe_push (parser->elem_fn_tokens, *token); + c_parser_consume_token (parser); + } + + /* Since we are converting an attribute to a pragma, we need to end the + attribute with PRAGMA_EOL. */ + c_token eol_token; + memset (&eol_token, 0, sizeof (eol_token)); + eol_token.type = CPP_PRAGMA_EOL; + vec_safe_push (parser->elem_fn_tokens, eol_token); + return; +} + +/* Add 2 CPP_EOF at the end of PARSER->ELEM_FN_TOKENS vector. */ + +static void +c_finish_elem_fn_tokens (c_parser *parser) +{ + c_token last_token = parser->elem_fn_tokens->last (); + + /* c_parser_attributes is called in several places, and so if these EOF + tokens are already inserted, then don't do them again. */ + if (last_token.type == CPP_EOF) + return; + + /* 2 EOF_token is added as a safety-net since the normal C front-end has + two token look-ahead. */ + c_token eof_token; + eof_token.type = CPP_EOF; + vec_safe_push (parser->elem_fn_tokens, eof_token); + vec_safe_push (parser->elem_fn_tokens, eof_token); +} + /* Parse (possibly empty) attributes. This is a GNU extension. attributes: @@ -3824,6 +3976,13 @@ attr_name = c_parser_attribute_any_word (parser); if (attr_name == NULL) break; + if (flag_enable_cilkplus + && simple_cst_equal (attr_name, get_identifier ("vector")) == 1) + { + c_token *v_token = c_parser_peek_token (parser); + c_parser_elem_fn_expr_list (parser, *v_token); + continue; + } c_parser_consume_token (parser); if (c_parser_next_token_is_not (parser, CPP_OPEN_PAREN)) { @@ -3904,6 +4063,9 @@ } parser->lex_untranslated_string = false; } + + if (flag_enable_cilkplus && !vec_safe_is_empty (parser->elem_fn_tokens)) + c_finish_elem_fn_tokens (parser); return attrs; } @@ -12749,10 +12911,20 @@ c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms, vec<c_token> clauses) { + + if (flag_enable_cilkplus + && clauses.exists () && !vec_safe_is_empty (parser->elem_fn_tokens)) + { + error ("%<#pragma omp declare simd%> cannot be used in the same" + "function marked as a SIMD-enabled function"); + vec_free (parser->elem_fn_tokens); + return; + } + /* Normally first token is CPP_NAME "simd". CPP_EOF there indicates error has been reported and CPP_PRAGMA that c_finish_omp_declare_simd has already processed the tokens. */ - if (clauses[0].type == CPP_EOF) + if (clauses.exists () && clauses[0].type == CPP_EOF) return; if (fndecl == NULL_TREE || TREE_CODE (fndecl) != FUNCTION_DECL) { @@ -12761,7 +12933,7 @@ clauses[0].type = CPP_EOF; return; } - if (clauses[0].type != CPP_NAME) + if (clauses.exists () && clauses[0].type != CPP_NAME) { error_at (DECL_SOURCE_LOCATION (fndecl), "%<#pragma omp declare simd%> not immediately followed by " @@ -12775,9 +12947,18 @@ unsigned int tokens_avail = parser->tokens_avail; gcc_assert (parser->tokens == &parser->tokens_buf[0]); - parser->tokens = clauses.address (); - parser->tokens_avail = clauses.length (); + if (flag_enable_cilkplus && !vec_safe_is_empty (parser->elem_fn_tokens)) + { + parser->tokens = parser->elem_fn_tokens->address (); + parser->tokens_avail = vec_safe_length (parser->elem_fn_tokens); + } + else + { + 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) { @@ -12799,7 +12980,11 @@ parser->tokens = &parser->tokens_buf[0]; parser->tokens_avail = tokens_avail; - clauses[0].type = CPP_PRAGMA; + if (clauses.exists ()) + clauses[0].type = CPP_PRAGMA; + + if (!vec_safe_is_empty (parser->elem_fn_tokens)) + vec_free (parser->elem_fn_tokens); } Index: gcc/testsuite/gcc.dg/cilk-plus/cilk-plus.exp =================================================================== --- gcc/testsuite/gcc.dg/cilk-plus/cilk-plus.exp (revision 205055) +++ gcc/testsuite/gcc.dg/cilk-plus/cilk-plus.exp (working copy) @@ -59,6 +59,12 @@ dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -flto -g -fcilkplus" " " } +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/EF/*.c]] " -g" " " +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/EF/*.c]] " -O1" " " +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/EF/*.c]] " -O2 -std=c99" " " +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/EF/*.c]] " -O2 -ftree-vectorize" " " +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/EF/*.c]] " -O3 -g" " " + dg-finish unset TEST_EXTRA_LIBS Index: gcc/testsuite/c-c++-common/cilk-plus/EF/vlength_errors.c =================================================================== --- gcc/testsuite/c-c++-common/cilk-plus/EF/vlength_errors.c (revision 0) +++ gcc/testsuite/c-c++-common/cilk-plus/EF/vlength_errors.c (revision 0) @@ -0,0 +1,49 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus -Wunknown-pragmas" } */ + +#define Q 4 + +int z = Q; + +__attribute__ ((vector (uniform(x), vectorlength (), linear (y:1) ))) /* { dg-error "expected vectorlength value" } */ +int func2 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (4.5) ))) /* { dg-error "vectorlength must be a constant integer" } */ +int func3 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (z) ))) /* { dg-error "vectorlength must be a constant integer" } */ +int func4 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (Q) ))) /* This is OK! */ +int func5 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), vectorlength (z), linear (y:1)))) /* { dg-error "vectorlength must be a constant integer" } */ +int func6 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +int main (void) +{ + int ii = 0, q = 5; + for (ii = 0; ii < 10; ii++) + q += func2 (z, ii); + return q; +} Index: gcc/testsuite/c-c++-common/cilk-plus/EF/ef_error.c =================================================================== --- gcc/testsuite/c-c++-common/cilk-plus/EF/ef_error.c (revision 0) +++ gcc/testsuite/c-c++-common/cilk-plus/EF/ef_error.c (revision 0) @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus -fopenmp" } */ + +#pragma omp declare simd linear(y:1) simdlen(4) +__attribute__((vector (linear (y:1), vectorlength(4)))) +int func (int x, int y) +{ /* { dg-error "cannot be used in the same function marked as a SIMD-enabled" } */ + return (x+y); +} + +int main (void) +{ + return (func (5,6)); +} Index: gcc/testsuite/c-c++-common/cilk-plus/EF/ef_test.c =================================================================== --- gcc/testsuite/c-c++-common/cilk-plus/EF/ef_test.c (revision 0) +++ gcc/testsuite/c-c++-common/cilk-plus/EF/ef_test.c (revision 0) @@ -0,0 +1,78 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus -Wunknown-pragmas" } */ + +/* Tests the clauses in several combinations put in different locations. */ +/* This is mostly a parser test. */ +#define Q 4 + +int z = Q; + + __attribute__ ((vector (uniform(x), linear (y:1), vectorlength (4) ))) +int func (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + __attribute__ ((vector (uniform(x), vectorlength (2), linear (y:1) ))) +int func2 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(y), linear (x), vectorlength (4) ))) +int func3 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), linear (y:1), mask))) +int func4 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), linear (y:1), nomask))) +int func5 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform(x), mask, linear (y:1)))) +int func6 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform (x), mask, linear (y:1)), vector)) +int func7 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector (uniform (x), mask, linear (y:1)), vector (uniform (y), mask))) +int func8 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +__attribute__ ((vector, vector (uniform (y), mask))) +int func9 (int x, int y) +{ + int zq = 5; + return x + (y*zq); +} + +int main (int argc, char *argv[]) +{ + int ii = 0, q = 5; + for (ii = 0; ii < 10; ii++) + q += func (argc, ii); + return q; +} Index: gcc/testsuite/c-c++-common/cilk-plus/EF/ef_test2.c =================================================================== --- gcc/testsuite/c-c++-common/cilk-plus/EF/ef_test2.c (revision 0) +++ gcc/testsuite/c-c++-common/cilk-plus/EF/ef_test2.c (revision 0) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-fcilkplus" } */ +void func (int x, int y) __attribute__((vector(linear(x:1), uniform (y)), + vector)); + +int q; +int main (void) +{ + int ii = 0; + q = 5; + for (ii = 0; ii < 100; ii++) + func (ii, q); + + return 0; +} +