* [OpenACC] num_gangs, num_workers and vector_length in c++ @ 2015-10-30 0:03 Cesar Philippidis 2015-10-30 13:41 ` Jakub Jelinek 0 siblings, 1 reply; 10+ messages in thread From: Cesar Philippidis @ 2015-10-30 0:03 UTC (permalink / raw) To: gcc-patches, Nathan Sidwell, Jason Merrill, Jakub Jelinek [-- Attachment #1: Type: text/plain, Size: 932 bytes --] I noticed that num_gangs, num_workers and vector_length are parsed in somewhat insistent ways in the c++ FE. Both vector_length and num_gangs bail out whenever as soon as they detect errors, whereas num_workers does not. Besides for that, they are also checking for integral expressions as the arguments are scanned instead of deferring that check to finish_omp_clauses. That check will cause ICEs when template arguments are used when we add support for template arguments later on. Rather than fix each function individually, I've consolidated them into a single cp_parser_oacc_positive_int_clause function. While this function could be extended to support openmp clauses which accept an integer expression argument, like num_threads, I've decided to leave those as-is since there are no known problems with those functions at this moment. It this OK for trunk? I've regression tested and bootstrapped on x86_64-linux. Cesar [-- Attachment #2: cxx-size.diff --] [-- Type: text/x-patch, Size: 6599 bytes --] 2015-10-29 Cesar Philippidis <cesar@codesourcery.com> gcc/cp/ * parser.c (cp_parser_oacc_positive_int_clause): New function. (cp_parser_oacc_clause_vector_length): Delete. (cp_parser_omp_clause_num_gangs): Delete. (cp_parser_omp_clause_num_workers): Delete. (cp_parser_oacc_all_clauses): Use cp_parser_oacc_positive_int_clause to handle OMP_CLAUSE_{NUM_GANGS,NUM_WORKERS,VECTOR_LENGTH}. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index c8f8b3d..b1172e7 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -29603,6 +29603,39 @@ cp_parser_oacc_simple_clause (cp_parser * /* parser */, return c; } + /* OpenACC: + num_gangs ( expression ) + num_workers ( expression ) + vector_length ( expression ) */ + +static tree +cp_parser_oacc_positive_int_clause (cp_parser *parser, omp_clause_code code, + const char *str, tree list) +{ + location_t loc = cp_lexer_peek_token (parser->lexer)->location; + + if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) + return list; + + tree t = cp_parser_assignment_expression (parser, NULL, false, false); + + if (t == error_mark_node + || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) + { + cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); + return list; + } + + check_no_duplicate_clause (list, code, str, loc); + + tree c = build_omp_clause (loc, code); + OMP_CLAUSE_OPERAND (c, 0) = t; + OMP_CLAUSE_CHAIN (c) = list; + return c; +} + /* OpenACC: gang [( gang-arg-list )] @@ -29726,45 +29759,6 @@ cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind, return list; } -/* OpenACC: - vector_length ( expression ) */ - -static tree -cp_parser_oacc_clause_vector_length (cp_parser *parser, tree list) -{ - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - bool error = false; - - if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) - return list; - - t = cp_parser_condition (parser); - if (t == error_mark_node || !INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - error = true; - } - - if (error || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - { - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - return list; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length", - location); - - c = build_omp_clause (location, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; -} - /* OpenACC 2.0 Parse wait clause or directive parameters. */ @@ -30143,42 +30137,6 @@ cp_parser_omp_clause_nowait (cp_parser * /*parser*/, return c; } -/* OpenACC: - num_gangs ( expression ) */ - -static tree -cp_parser_omp_clause_num_gangs (cp_parser *parser, tree list) -{ - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - - if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) - return list; - - t = cp_parser_condition (parser); - - if (t == error_mark_node - || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - return list; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs", location); - - c = build_omp_clause (location, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; -} - /* OpenMP 2.5: num_threads ( expression ) */ @@ -30387,43 +30345,6 @@ cp_parser_omp_clause_defaultmap (cp_parser *parser, tree list, return list; } -/* OpenACC: - num_workers ( expression ) */ - -static tree -cp_parser_omp_clause_num_workers (cp_parser *parser, tree list) -{ - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - - if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) - return list; - - t = cp_parser_condition (parser); - - if (t == error_mark_node - || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - return list; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_gangs", - location); - - c = build_omp_clause (location, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; -} - /* OpenMP 2.5: ordered @@ -31435,6 +31356,7 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, { location_t here; pragma_omp_clause c_kind; + omp_clause_code code; const char *c_name; tree prev = clauses; @@ -31501,12 +31423,16 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, c_name = "if"; break; case PRAGMA_OACC_CLAUSE_NUM_GANGS: - clauses = cp_parser_omp_clause_num_gangs (parser, clauses); + code = OMP_CLAUSE_NUM_GANGS; c_name = "num_gangs"; + clauses = cp_parser_oacc_positive_int_clause (parser, code, + c_name, clauses); break; case PRAGMA_OACC_CLAUSE_NUM_WORKERS: - clauses = cp_parser_omp_clause_num_workers (parser, clauses); c_name = "num_workers"; + code = OMP_CLAUSE_NUM_WORKERS; + clauses = cp_parser_oacc_positive_int_clause (parser, code, + c_name, clauses); break; case PRAGMA_OACC_CLAUSE_PRESENT: clauses = cp_parser_oacc_data_clause (parser, c_kind, clauses); @@ -31547,8 +31473,10 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: - clauses = cp_parser_oacc_clause_vector_length (parser, clauses); c_name = "vector_length"; + code = OMP_CLAUSE_VECTOR_LENGTH; + clauses = cp_parser_oacc_positive_int_clause (parser, code, + c_name, clauses); break; case PRAGMA_OACC_CLAUSE_WAIT: clauses = cp_parser_oacc_clause_wait (parser, clauses); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [OpenACC] num_gangs, num_workers and vector_length in c++ 2015-10-30 0:03 [OpenACC] num_gangs, num_workers and vector_length in c++ Cesar Philippidis @ 2015-10-30 13:41 ` Jakub Jelinek 2015-10-30 14:43 ` Cesar Philippidis 0 siblings, 1 reply; 10+ messages in thread From: Jakub Jelinek @ 2015-10-30 13:41 UTC (permalink / raw) To: Cesar Philippidis, Jason Merrill; +Cc: gcc-patches, Nathan Sidwell On Thu, Oct 29, 2015 at 04:02:11PM -0700, Cesar Philippidis wrote: > I noticed that num_gangs, num_workers and vector_length are parsed in > somewhat insistent ways in the c++ FE. Both vector_length and num_gangs > bail out whenever as soon as they detect errors, whereas num_workers > does not. Besides for that, they are also checking for integral > expressions as the arguments are scanned instead of deferring that check > to finish_omp_clauses. That check will cause ICEs when template > arguments are used when we add support for template arguments later on. > > Rather than fix each function individually, I've consolidated them into > a single cp_parser_oacc_positive_int_clause function. While this > function could be extended to support openmp clauses which accept an > integer expression argument, like num_threads, I've decided to leave > those as-is since there are no known problems with those functions at > this moment. First question is what int-expr in OpenACC actually stands for (but I'll have to raise similar question for OpenMP too). Previously you were using cp_parser_condition, which is clearly undesirable in this case, it allows e.g. num_gangs (int a = 5) but the question is if num_gangs (5, 6) is valid and stands for (5, 6) expression, then it should use cp_parser_expression, or if you want to error on it, then you should use cp_parser_assignment_expression. From quick skimming of the (now removed) C/C++ Grammar Appendix in OpenMP, I believe all the places where expression or scalar-expression is used in the grammar are meant to be cp_parser_expression cases (except expression-list used in UDRs which stands for normal C++ expression-list non-terminal), so clearly I need to fix up omp_clause_{if,final} to call cp_parser_expression instead of cp_parser_condition, and the various OpenMP clauses that use cp_parser_assignment_expression to instead use cp_parser_expression. Say schedule(static, 3, 6) should be valid IMHO. But, in OpenMP expression or scalar-expression in the grammar is never followed by , or optional , while in OpenACC grammar clearly is (e.g. for the gang clause). If OpenACC wants something different, clearly you can't share the parsing routines between say num_tasks and num_workers. Another thing is what Jason as C++ maintainer wants, it is nice to get rid of some code redundancies, on the other side the fact that there is one function per non-terminal in the grammar is also quite nice property. I know I've violated this a few times too. Next question is, why do you call it cp_parser_oacc_positive_int_clause when the parsing function actually doesn't verify neither the positive nor the int properties (and it should not), so perhaps it should just reflect in the name that it is a clause with assignment? expression. Or, see the previous paragraph, have a helper that does that and then have a separate function for each clause kind that calls those with the right arguments. Jakub ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [OpenACC] num_gangs, num_workers and vector_length in c++ 2015-10-30 13:41 ` Jakub Jelinek @ 2015-10-30 14:43 ` Cesar Philippidis 2015-10-30 17:07 ` Jakub Jelinek 0 siblings, 1 reply; 10+ messages in thread From: Cesar Philippidis @ 2015-10-30 14:43 UTC (permalink / raw) To: Jakub Jelinek, Jason Merrill; +Cc: gcc-patches, Nathan Sidwell On 10/30/2015 06:37 AM, Jakub Jelinek wrote: > On Thu, Oct 29, 2015 at 04:02:11PM -0700, Cesar Philippidis wrote: >> I noticed that num_gangs, num_workers and vector_length are parsed in >> somewhat insistent ways in the c++ FE. Both vector_length and num_gangs >> bail out whenever as soon as they detect errors, whereas num_workers >> does not. Besides for that, they are also checking for integral >> expressions as the arguments are scanned instead of deferring that check >> to finish_omp_clauses. That check will cause ICEs when template >> arguments are used when we add support for template arguments later on. >> >> Rather than fix each function individually, I've consolidated them into >> a single cp_parser_oacc_positive_int_clause function. While this >> function could be extended to support openmp clauses which accept an >> integer expression argument, like num_threads, I've decided to leave >> those as-is since there are no known problems with those functions at >> this moment. > > First question is what int-expr in OpenACC actually stands for (but I'll > have to raise similar question for OpenMP too). > > Previously you were using cp_parser_condition, which is clearly undesirable > in this case, it allows e.g. > num_gangs (int a = 5) > but the question is if > num_gangs (5, 6) > is valid and stands for (5, 6) expression, then it should use > cp_parser_expression, or if you want to error on it, then you should use > cp_parser_assignment_expression. The openacc spec doesn't actually define int-expr, but we take to me mean a single integral value. In general, the openacc spec uses the term list to describe comma separated expressions. So we've been assuming that expr cannot contain commas. Besides, for num_gangs, num_workers and vector_length it doesn't make sense to accept more than one value. A construct can accept one than one of those clauses, but they'd have to be associated with a different device_type. > From quick skimming of the (now removed) C/C++ Grammar Appendix in OpenMP, > I believe all the places where expression or scalar-expression is used > in the grammar are meant to be cp_parser_expression cases (except > expression-list used in UDRs which stands for normal C++ expression-list > non-terminal), so clearly I need to fix up omp_clause_{if,final} to call > cp_parser_expression instead of cp_parser_condition, and the various > OpenMP clauses that use cp_parser_assignment_expression to instead use > cp_parser_expression. Say schedule(static, 3, 6) should be valid IMHO. > But, in OpenMP expression or scalar-expression in the grammar is never > followed by , or optional , while in OpenACC grammar clearly is (e.g. for > the gang clause). > If OpenACC wants something different, clearly you can't share the parsing > routines between say num_tasks and num_workers. So num_threads, num_tasks, grainsize, priority, hint, num_teams, thread_limit should all accept comma-separated lists? > Another thing is what Jason as C++ maintainer wants, it is nice to get rid > of some code redundancies, on the other side the fact that there is one > function per non-terminal in the grammar is also quite nice property. > I know I've violated this a few times too. > > Next question is, why do you call it cp_parser_oacc_positive_int_clause > when the parsing function actually doesn't verify neither the positive nor > the int properties (and it should not), so perhaps it should just reflect > in the name that it is a clause with assignment? expression. > Or, see the previous paragraph, have a helper that does that and then > have a separate function for each clause kind that calls those with the > right arguments. That name had some legacy from the c FE in gomp-4_0-branch which the function was inherited from. On one hand, it doesn't make sense to allow negative integer values for those clauses, but at the same time, those values aren't checked during scanning. Maybe it should be renamed cp_parser_oacc_single_int_clause instead? If you like, I could make a more general cp_parser_omp_generic_expression that has a scan_list argument so that it can be used for both general expressions and assignment-expressions. That way it can be used for both omp and oacc clauses of the form 'foo ( expression )'. What's your preference? Thanks, Cesar ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [OpenACC] num_gangs, num_workers and vector_length in c++ 2015-10-30 14:43 ` Cesar Philippidis @ 2015-10-30 17:07 ` Jakub Jelinek 2015-10-30 21:28 ` Cesar Philippidis 0 siblings, 1 reply; 10+ messages in thread From: Jakub Jelinek @ 2015-10-30 17:07 UTC (permalink / raw) To: Cesar Philippidis; +Cc: Jason Merrill, gcc-patches, Nathan Sidwell On Fri, Oct 30, 2015 at 07:42:39AM -0700, Cesar Philippidis wrote: > The openacc spec doesn't actually define int-expr, but we take to me > mean a single integral value. In general, the openacc spec uses the term > list to describe comma separated expressions. So we've been assuming So does OpenMP use *-list, except one place in the grammar where expression-list is used, but that is to describe function call arguments and the C++ expression-list non-terminal is essentially {assignment-expression}-list. > that expr cannot contain commas. Besides, for num_gangs, num_workers and > vector_length it doesn't make sense to accept more than one value. A Well, the comma expression are not a way to supply two values, but to evaluate some side-effects before evaluating the expression you want. > > From quick skimming of the (now removed) C/C++ Grammar Appendix in OpenMP, > > I believe all the places where expression or scalar-expression is used > > in the grammar are meant to be cp_parser_expression cases (except > > expression-list used in UDRs which stands for normal C++ expression-list > > non-terminal), so clearly I need to fix up omp_clause_{if,final} to call > > cp_parser_expression instead of cp_parser_condition, and the various > > OpenMP clauses that use cp_parser_assignment_expression to instead use > > cp_parser_expression. Say schedule(static, 3, 6) should be valid IMHO. > > But, in OpenMP expression or scalar-expression in the grammar is never > > followed by , or optional , while in OpenACC grammar clearly is (e.g. for > > the gang clause). > > If OpenACC wants something different, clearly you can't share the parsing > > routines between say num_tasks and num_workers. > > So num_threads, num_tasks, grainsize, priority, hint, num_teams, > thread_limit should all accept comma-separated lists? They accept expression, which is e.g. for C++: expression: assignment-expression expression , assignment-expression > > > Another thing is what Jason as C++ maintainer wants, it is nice to get rid > > of some code redundancies, on the other side the fact that there is one > > function per non-terminal in the grammar is also quite nice property. > > I know I've violated this a few times too. > That name had some legacy from the c FE in gomp-4_0-branch which the > function was inherited from. On one hand, it doesn't make sense to allow > negative integer values for those clauses, but at the same time, those > values aren't checked during scanning. Maybe it should be renamed > cp_parser_oacc_single_int_clause instead? That is better. > If you like, I could make a more general > cp_parser_omp_generic_expression that has a scan_list argument so that > it can be used for both general expressions and assignment-expressions. > That way it can be used for both omp and oacc clauses of the form 'foo ( > expression )'. No, that will only confuse readers of the parser. After all, the code to parse an expression argument of a clause is not that large. So, either cp_parser_oacc_single_int_clause or just keeping the old separate parsing functions, just remove the cruft from those (testing the type, using cp_parser_condition instead of cp_parser_assignment_expression) is ok with me. Please ping Jason on what he prefers from those two. Jakub ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [OpenACC] num_gangs, num_workers and vector_length in c++ 2015-10-30 17:07 ` Jakub Jelinek @ 2015-10-30 21:28 ` Cesar Philippidis 2015-11-04 18:09 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Cesar Philippidis @ 2015-10-30 21:28 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 1841 bytes --] On 10/30/2015 10:05 AM, Jakub Jelinek wrote: > On Fri, Oct 30, 2015 at 07:42:39AM -0700, Cesar Philippidis wrote: >>> Another thing is what Jason as C++ maintainer wants, it is nice to get rid >>> of some code redundancies, on the other side the fact that there is one >>> function per non-terminal in the grammar is also quite nice property. >>> I know I've violated this a few times too. > >> That name had some legacy from the c FE in gomp-4_0-branch which the >> function was inherited from. On one hand, it doesn't make sense to allow >> negative integer values for those clauses, but at the same time, those >> values aren't checked during scanning. Maybe it should be renamed >> cp_parser_oacc_single_int_clause instead? > > That is better. > >> If you like, I could make a more general >> cp_parser_omp_generic_expression that has a scan_list argument so that >> it can be used for both general expressions and assignment-expressions. >> That way it can be used for both omp and oacc clauses of the form 'foo ( >> expression )'. > > No, that will only confuse readers of the parser. After all, the code to > parse an expression argument of a clause is not that large. > So, either cp_parser_oacc_single_int_clause or just keeping the old separate > parsing functions, just remove the cruft from those (testing the type, > using cp_parser_condition instead of cp_parser_assignment_expression) is ok > with me. Please ping Jason on what he prefers from those two. Jason, what's your preference here? Should I create a single function to parser num_gangs, num_workers and vector_length since they all accept the same type of argument or should I just correct the existing functions as I did in the attached patch? Either one would be specific to openacc. This patch has been bootstrapped and regression tested on trunk. Cesar [-- Attachment #2: cxx-ng-nw-vl.diff --] [-- Type: text/x-patch, Size: 4971 bytes --] 2015-10-30 Cesar Philippidis <cesar@codesourcery.com> gcc/cp/ * parser.c (cp_parser_oacc_clause_vector_length): Parse the clause argument as an assignment expression. Bail out early on error. (cp_parser_omp_clause_num_gangs): Likewise. (cp_parser_omp_clause_num_workers): Likewise. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index c8f8b3d..a0d3f3b 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -29732,37 +29732,29 @@ cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind, static tree cp_parser_oacc_clause_vector_length (cp_parser *parser, tree list) { - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - bool error = false; + location_t loc = cp_lexer_peek_token (parser->lexer)->location; if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; - t = cp_parser_condition (parser); - if (t == error_mark_node || !INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - error = true; - } + tree t = cp_parser_assignment_expression (parser, NULL, false, false); - if (error || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) + if (t == error_mark_node + || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) { cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); + /*or_comma=*/false, + /*consume_paren=*/true); return list; } check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length", - location); + loc); - c = build_omp_clause (location, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; + tree c = build_omp_clause (loc, OMP_CLAUSE_VECTOR_LENGTH); + OMP_CLAUSE_OPERAND (c, 0) = t; OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; + return c; } /* OpenACC 2.0 @@ -30149,34 +30141,28 @@ cp_parser_omp_clause_nowait (cp_parser * /*parser*/, static tree cp_parser_omp_clause_num_gangs (cp_parser *parser, tree list) { - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; + location_t loc = cp_lexer_peek_token (parser->lexer)->location; if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; - t = cp_parser_condition (parser); + tree t = cp_parser_assignment_expression (parser, NULL, false, false); if (t == error_mark_node || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - error_at (location, "expected positive integer expression"); + cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); return list; } - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs", location); + check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs", loc); - c = build_omp_clause (location, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; + tree c = build_omp_clause (loc, OMP_CLAUSE_NUM_GANGS); + OMP_CLAUSE_OPERAND (c, 0) = t; OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; + return c; } /* OpenMP 2.5: @@ -30393,35 +30379,28 @@ cp_parser_omp_clause_defaultmap (cp_parser *parser, tree list, static tree cp_parser_omp_clause_num_workers (cp_parser *parser, tree list) { - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; + location_t loc = cp_lexer_peek_token (parser->lexer)->location; if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) return list; - t = cp_parser_condition (parser); + tree t = cp_parser_assignment_expression (parser, NULL, false, false); if (t == error_mark_node || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - error_at (location, "expected positive integer expression"); + cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); return list; } - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_gangs", - location); + check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_workers", loc); - c = build_omp_clause (location, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; + tree c = build_omp_clause (loc, OMP_CLAUSE_NUM_WORKERS); + OMP_CLAUSE_OPERAND (c, 0) = t; OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; + return c; } /* OpenMP 2.5: ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [OpenACC] num_gangs, num_workers and vector_length in c++ 2015-10-30 21:28 ` Cesar Philippidis @ 2015-11-04 18:09 ` Jason Merrill 2015-11-04 22:10 ` Cesar Philippidis 0 siblings, 1 reply; 10+ messages in thread From: Jason Merrill @ 2015-11-04 18:09 UTC (permalink / raw) To: Cesar Philippidis, Jakub Jelinek; +Cc: gcc-patches, Nathan Sidwell On 10/30/2015 05:21 PM, Cesar Philippidis wrote: > On 10/30/2015 10:05 AM, Jakub Jelinek wrote: >> On Fri, Oct 30, 2015 at 07:42:39AM -0700, Cesar Philippidis wrote: > >>>> Another thing is what Jason as C++ maintainer wants, it is nice to get rid >>>> of some code redundancies, on the other side the fact that there is one >>>> function per non-terminal in the grammar is also quite nice property. >>>> I know I've violated this a few times too. >> >>> That name had some legacy from the c FE in gomp-4_0-branch which the >>> function was inherited from. On one hand, it doesn't make sense to allow >>> negative integer values for those clauses, but at the same time, those >>> values aren't checked during scanning. Maybe it should be renamed >>> cp_parser_oacc_single_int_clause instead? >> >> That is better. >> >>> If you like, I could make a more general >>> cp_parser_omp_generic_expression that has a scan_list argument so that >>> it can be used for both general expressions and assignment-expressions. >>> That way it can be used for both omp and oacc clauses of the form 'foo ( >>> expression )'. >> >> No, that will only confuse readers of the parser. After all, the code to >> parse an expression argument of a clause is not that large. >> So, either cp_parser_oacc_single_int_clause or just keeping the old separate >> parsing functions, just remove the cruft from those (testing the type, >> using cp_parser_condition instead of cp_parser_assignment_expression) is ok >> with me. Please ping Jason on what he prefers from those two. > > Jason, what's your preference here? Should I create a single function to > parser num_gangs, num_workers and vector_length since they all accept > the same type of argument or should I just correct the existing > functions as I did in the attached patch? Either one would be specific > to openacc. A single function is better, to avoid unnecessary code duplication. Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [OpenACC] num_gangs, num_workers and vector_length in c++ 2015-11-04 18:09 ` Jason Merrill @ 2015-11-04 22:10 ` Cesar Philippidis 2017-05-09 21:27 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause (was: [OpenACC] num_gangs, num_workers and vector_length in c++) Thomas Schwinge 0 siblings, 1 reply; 10+ messages in thread From: Cesar Philippidis @ 2015-11-04 22:10 UTC (permalink / raw) To: Jason Merrill, Jakub Jelinek; +Cc: gcc-patches, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 167 bytes --] On 11/04/2015 10:09 AM, Jason Merrill wrote: > A single function is better, to avoid unnecessary code duplication. Thanks. I've applied this patch to trunk. Cesar [-- Attachment #2: cxx-single-int-clause.diff --] [-- Type: text/x-patch, Size: 6577 bytes --] 2015-11-04 Cesar Philippidis <cesar@codesourcery.com> gcc/cp/ * (cp_parser_oacc_single_int_clause): New function. (cp_parser_oacc_clause_vector_length): Delete. (cp_parser_omp_clause_num_gangs): Delete. (cp_parser_omp_clause_num_workers): Delete. (cp_parser_oacc_all_clauses): Use cp_parser_oacc_single_int_clause for num_gangs, num_workers and vector_length. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 12452e6..4f6cd2d 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -29590,6 +29590,39 @@ cp_parser_oacc_simple_clause (cp_parser * /* parser */, return c; } + /* OpenACC: + num_gangs ( expression ) + num_workers ( expression ) + vector_length ( expression ) */ + +static tree +cp_parser_oacc_single_int_clause (cp_parser *parser, omp_clause_code code, + const char *str, tree list) +{ + location_t loc = cp_lexer_peek_token (parser->lexer)->location; + + if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) + return list; + + tree t = cp_parser_assignment_expression (parser, NULL, false, false); + + if (t == error_mark_node + || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) + { + cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); + return list; + } + + check_no_duplicate_clause (list, code, str, loc); + + tree c = build_omp_clause (loc, code); + OMP_CLAUSE_OPERAND (c, 0) = t; + OMP_CLAUSE_CHAIN (c) = list; + return c; +} + /* OpenACC: gang [( gang-arg-list )] @@ -29713,45 +29746,6 @@ cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind, return list; } -/* OpenACC: - vector_length ( expression ) */ - -static tree -cp_parser_oacc_clause_vector_length (cp_parser *parser, tree list) -{ - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - bool error = false; - - if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) - return list; - - t = cp_parser_condition (parser); - if (t == error_mark_node || !INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - error = true; - } - - if (error || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - { - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - return list; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length", - location); - - c = build_omp_clause (location, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; -} - /* OpenACC 2.0 Parse wait clause or directive parameters. */ @@ -30130,42 +30124,6 @@ cp_parser_omp_clause_nowait (cp_parser * /*parser*/, return c; } -/* OpenACC: - num_gangs ( expression ) */ - -static tree -cp_parser_omp_clause_num_gangs (cp_parser *parser, tree list) -{ - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - - if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) - return list; - - t = cp_parser_condition (parser); - - if (t == error_mark_node - || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - return list; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs", location); - - c = build_omp_clause (location, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; -} - /* OpenMP 2.5: num_threads ( expression ) */ @@ -30374,43 +30332,6 @@ cp_parser_omp_clause_defaultmap (cp_parser *parser, tree list, return list; } -/* OpenACC: - num_workers ( expression ) */ - -static tree -cp_parser_omp_clause_num_workers (cp_parser *parser, tree list) -{ - tree t, c; - location_t location = cp_lexer_peek_token (parser->lexer)->location; - - if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN)) - return list; - - t = cp_parser_condition (parser); - - if (t == error_mark_node - || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) - cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true, - /*or_comma=*/false, - /*consume_paren=*/true); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - error_at (location, "expected positive integer expression"); - return list; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_gangs", - location); - - c = build_omp_clause (location, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - - return list; -} - /* OpenMP 2.5: ordered @@ -31422,6 +31343,7 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, { location_t here; pragma_omp_clause c_kind; + omp_clause_code code; const char *c_name; tree prev = clauses; @@ -31488,12 +31410,16 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, c_name = "if"; break; case PRAGMA_OACC_CLAUSE_NUM_GANGS: - clauses = cp_parser_omp_clause_num_gangs (parser, clauses); + code = OMP_CLAUSE_NUM_GANGS; c_name = "num_gangs"; + clauses = cp_parser_oacc_single_int_clause (parser, code, c_name, + clauses); break; case PRAGMA_OACC_CLAUSE_NUM_WORKERS: - clauses = cp_parser_omp_clause_num_workers (parser, clauses); c_name = "num_workers"; + code = OMP_CLAUSE_NUM_WORKERS; + clauses = cp_parser_oacc_single_int_clause (parser, code, c_name, + clauses); break; case PRAGMA_OACC_CLAUSE_PRESENT: clauses = cp_parser_oacc_data_clause (parser, c_kind, clauses); @@ -31534,8 +31460,10 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: - clauses = cp_parser_oacc_clause_vector_length (parser, clauses); c_name = "vector_length"; + code = OMP_CLAUSE_VECTOR_LENGTH; + clauses = cp_parser_oacc_single_int_clause (parser, code, c_name, + clauses); break; case PRAGMA_OACC_CLAUSE_WAIT: clauses = cp_parser_oacc_clause_wait (parser, clauses); ^ permalink raw reply [flat|nested] 10+ messages in thread
* OpenACC C front end maintenance: c_parser_oacc_single_int_clause (was: [OpenACC] num_gangs, num_workers and vector_length in c++) 2015-11-04 22:10 ` Cesar Philippidis @ 2017-05-09 21:27 ` Thomas Schwinge 2017-05-10 16:36 ` Jakub Jelinek 0 siblings, 1 reply; 10+ messages in thread From: Thomas Schwinge @ 2017-05-09 21:27 UTC (permalink / raw) To: gcc-patches, Jakub Jelinek Hi! On Wed, 4 Nov 2015 14:10:29 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote: > > [...] > > Thanks. I've applied this patch to trunk. > gcc/cp/ > * (cp_parser_oacc_single_int_clause): New function. > (cp_parser_oacc_clause_vector_length): Delete. > (cp_parser_omp_clause_num_gangs): Delete. > (cp_parser_omp_clause_num_workers): Delete. > (cp_parser_oacc_all_clauses): Use cp_parser_oacc_single_int_clause > for num_gangs, num_workers and vector_length. Here is a similar patch for the C front end, and it also adds test cases. OK for trunk? During testing I also noticed some "strangeness" (also regarding some similar OpenMP clauses), which I'll file a PR for, later on. commit 70baf3fc0c544fe63b9d3b3bebcca88daf7ce554 Author: Thomas Schwinge <thomas@codesourcery.com> Date: Fri May 5 16:38:52 2017 +0200 OpenACC C front end maintenance: c_parser_oacc_single_int_clause gcc/c/ * c-parser.c (c_parser_omp_clause_num_gangs) (c_parser_omp_clause_num_workers) (c_parser_omp_clause_vector_length): Merge functions into... (c_parser_oacc_single_int_clause): ... this new function. Adjust all users. gcc/testsuite/ * c-c++-common/goacc/parallel-dims-1.c: New file. * c-c++-common/goacc/parallel-dims-2.c: Likewise. --- gcc/c/c-parser.c | 176 ++++++--------------- gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c | 8 + gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c | 134 ++++++++++++++++ 3 files changed, 191 insertions(+), 127 deletions(-) diff --git gcc/c/c-parser.c gcc/c/c-parser.c index 9398652..90d2d17 100644 --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -11339,51 +11339,6 @@ c_parser_omp_clause_nowait (c_parser *parser ATTRIBUTE_UNUSED, tree list) return c; } -/* OpenACC: - num_gangs ( expression ) */ - -static tree -c_parser_omp_clause_num_gangs (c_parser *parser, tree list) -{ - location_t num_gangs_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) - { - location_t expr_loc = c_parser_peek_token (parser)->location; - c_expr expr = c_parser_expression (parser); - expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); - tree c, t = expr.value; - t = c_fully_fold (t, false, NULL); - - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<num_gangs%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs"); - - c = build_omp_clause (num_gangs_loc, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - } - - return list; -} - /* OpenMP 2.5: num_threads ( expression ) */ @@ -11671,48 +11626,54 @@ c_parser_omp_clause_is_device_ptr (c_parser *parser, tree list) } /* OpenACC: - num_workers ( expression ) */ + num_gangs ( expression ) + num_workers ( expression ) + vector_length ( expression ) */ static tree -c_parser_omp_clause_num_workers (c_parser *parser, tree list) +c_parser_oacc_single_int_clause (c_parser *parser, omp_clause_code code, + tree list) { - location_t num_workers_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + location_t loc = c_parser_peek_token (parser)->location; + + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return list; + + location_t expr_loc = c_parser_peek_token (parser)->location; + c_expr expr = c_parser_expression (parser); + expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); + tree c, t = expr.value; + t = c_fully_fold (t, false, NULL); + + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); + + if (t == error_mark_node) + return list; + else if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - location_t expr_loc = c_parser_peek_token (parser)->location; - c_expr expr = c_parser_expression (parser); - expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); - tree c, t = expr.value; - t = c_fully_fold (t, false, NULL); + error_at (expr_loc, "%qs expression must be integral", + omp_clause_code_name[code]); + return list; + } - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, + /* Attempt to statically determine when the number isn't positive. */ + c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<num_workers%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_workers"); - - c = build_omp_clause (num_workers_loc, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; + protected_set_expr_location (c, expr_loc); + if (c == boolean_true_node) + { + warning_at (expr_loc, 0, + "%qs value must be positive", + omp_clause_code_name[code]); + t = integer_one_node; } - return list; + check_no_duplicate_clause (list, code, omp_clause_code_name[code]); + + c = build_omp_clause (loc, code); + OMP_CLAUSE_OPERAND (c, 0) = t; + OMP_CLAUSE_CHAIN (c) = list; + return c; } /* OpenACC: @@ -12354,51 +12315,6 @@ c_parser_omp_clause_untied (c_parser *parser ATTRIBUTE_UNUSED, tree list) return c; } -/* OpenACC: - vector_length ( expression ) */ - -static tree -c_parser_omp_clause_vector_length (c_parser *parser, tree list) -{ - location_t vector_length_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) - { - location_t expr_loc = c_parser_peek_token (parser)->location; - c_expr expr = c_parser_expression (parser); - expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); - tree c, t = expr.value; - t = c_fully_fold (t, false, NULL); - - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<vector_length%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length"); - - c = build_omp_clause (vector_length_loc, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - } - - return list; -} - /* OpenMP 4.0: inbranch notinbranch */ @@ -13283,11 +13199,15 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name = "link"; break; case PRAGMA_OACC_CLAUSE_NUM_GANGS: - clauses = c_parser_omp_clause_num_gangs (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_NUM_GANGS, + clauses); c_name = "num_gangs"; break; case PRAGMA_OACC_CLAUSE_NUM_WORKERS: - clauses = c_parser_omp_clause_num_workers (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_NUM_WORKERS, + clauses); c_name = "num_workers"; break; case PRAGMA_OACC_CLAUSE_PRESENT: @@ -13341,7 +13261,9 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: - clauses = c_parser_omp_clause_vector_length (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_VECTOR_LENGTH, + clauses); c_name = "vector_length"; break; case PRAGMA_OACC_CLAUSE_WAIT: diff --git gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c new file mode 100644 index 0000000..a85d3d3 --- /dev/null +++ gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c @@ -0,0 +1,8 @@ +/* Valid use of OpenACC parallelism dimensions clauses: num_gangs, num_workers, + vector_length. */ + +void f(int i) +{ +#pragma acc parallel num_gangs(i) num_workers(i) vector_length(i) + ; +} diff --git gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c new file mode 100644 index 0000000..30a3d17 --- /dev/null +++ gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c @@ -0,0 +1,134 @@ +/* Invalid use of OpenACC parallelism dimensions clauses: num_gangs, + num_workers, vector_length. */ + +void acc_kernels(int i) +{ +#pragma acc kernels num_gangs(i) /* { dg-error "'num_gangs' is not valid for '#pragma acc kernels'" } */ + ; +#pragma acc kernels num_workers(i) /* { dg-error "'num_workers' is not valid for '#pragma acc kernels'" } */ + ; +#pragma acc kernels vector_length(i) /* { dg-error "'vector_length' is not valid for '#pragma acc kernels'" } */ + ; +} + +void acc_parallel(int i, float f) +{ +#pragma acc parallel num_gangs /* { dg-error "expected '\\(' before end of line" } */ + ; +#pragma acc parallel num_workers /* { dg-error "expected '\\(' before end of line" } */ + ; +#pragma acc parallel vector_length /* { dg-error "expected '\\(' before end of line" } */ + ; + +#pragma acc parallel num_gangs( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; +#pragma acc parallel num_workers( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; +#pragma acc parallel vector_length( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; + +#pragma acc parallel num_gangs() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; +#pragma acc parallel num_workers() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; +#pragma acc parallel vector_length() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; + +#pragma acc parallel num_gangs(1 /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel num_workers(1 /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel vector_length(1 /* { dg-error "expected '\\)' before end of line" } */ + ; + +#pragma acc parallel num_gangs(i /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel num_workers(i /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel vector_length(i /* { dg-error "expected '\\)' before end of line" } */ + ; + +#pragma acc parallel num_gangs(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel num_workers(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel vector_length(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; + +#pragma acc parallel num_gangs(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel num_workers(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel vector_length(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; + +#pragma acc parallel num_gangs(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; +#pragma acc parallel num_workers(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; +#pragma acc parallel vector_length(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; + +#pragma acc parallel num_gangs(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; +#pragma acc parallel num_workers(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; +#pragma acc parallel vector_length(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; + +#pragma acc parallel num_gangs(num_gangs) /* { dg-error "'num_gangs' (un|was not )declared" } */ + ; +#pragma acc parallel num_workers(num_workers) /* { dg-error "'num_workers' (un|was not )declared" } */ + ; +#pragma acc parallel vector_length(vector_length) /* { dg-error "'vector_length' (un|was not )declared" } */ + ; + +#pragma acc parallel num_gangs(f) /* { dg-error "'num_gangs' expression must be integral" } */ + ; +#pragma acc parallel num_workers(f) /* { dg-error "'num_workers' expression must be integral" } */ + ; +#pragma acc parallel vector_length(f) /* { dg-error "'vector_length' expression must be integral" } */ + ; + +#pragma acc parallel num_gangs((float) 1) /* { dg-error "'num_gangs' expression must be integral" } */ + ; +#pragma acc parallel num_workers((float) 1) /* { dg-error "'num_workers' expression must be integral" } */ + ; +#pragma acc parallel vector_length((float) 1) /* { dg-error "'vector_length' expression must be integral" } */ + ; + +#pragma acc parallel num_gangs(0) /* { dg-warning "'num_gangs' value must be positive" } */ + ; +#pragma acc parallel num_workers(0) /* { dg-warning "'num_workers' value must be positive" } */ + ; +#pragma acc parallel vector_length(0) /* { dg-warning "'vector_length' value must be positive" } */ + ; + +#pragma acc parallel num_gangs((int) -1.2) /* { dg-warning "'num_gangs' value must be positive" } */ + ; +#pragma acc parallel num_workers((int) -1.2) /* { dg-warning "'num_workers' value must be positive" } */ + ; +#pragma acc parallel vector_length((int) -1.2) /* { dg-warning "'vector_length' value must be positive" } */ + ; + +#pragma acc parallel \ + num_gangs(1) /* { dg-error "too many 'num_gangs' clauses" "" { target c } } */ \ + num_workers(1) /* { dg-error "too many 'num_workers' clauses" "" { target c } } */ \ + vector_length(1) /* { dg-error "too many 'vector_length' clauses" "" { target c } } */ \ + num_workers(1) /* { dg-error "too many 'num_workers' clauses" "" { target c++ } } */ \ + vector_length(1) /* { dg-error "too many 'vector_length' clauses" "" { target c++ } } */ \ + num_gangs(1) /* { dg-error "too many 'num_gangs' clauses" "" { target c++ } } */ + ; + +#pragma acc parallel \ + num_gangs(-1) /* { dg-warning "'num_gangs' value must be positive" } */ \ + num_workers() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ \ + vector_length(abc) /* { dg-error "'abc' (un|was not )declared" } */ \ + num_workers(0.5) /* { dg-error "'num_workers' expression must be integral" } */ \ + vector_length(&acc_parallel) /* { dg-error "'vector_length' expression must be integral" } */ \ + num_gangs( /* { dg-error "expected (primary-|)expression before end of line" "TODO" { xfail c } } */ + ; +} Grüße Thomas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: OpenACC C front end maintenance: c_parser_oacc_single_int_clause (was: [OpenACC] num_gangs, num_workers and vector_length in c++) 2017-05-09 21:27 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause (was: [OpenACC] num_gangs, num_workers and vector_length in c++) Thomas Schwinge @ 2017-05-10 16:36 ` Jakub Jelinek 2017-05-12 9:38 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause Thomas Schwinge 0 siblings, 1 reply; 10+ messages in thread From: Jakub Jelinek @ 2017-05-10 16:36 UTC (permalink / raw) To: Thomas Schwinge; +Cc: gcc-patches On Tue, May 09, 2017 at 11:27:14PM +0200, Thomas Schwinge wrote: > Hi! > > On Wed, 4 Nov 2015 14:10:29 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote: > > > [...] > > > > Thanks. I've applied this patch to trunk. > > > gcc/cp/ > > * (cp_parser_oacc_single_int_clause): New function. > > (cp_parser_oacc_clause_vector_length): Delete. > > (cp_parser_omp_clause_num_gangs): Delete. > > (cp_parser_omp_clause_num_workers): Delete. > > (cp_parser_oacc_all_clauses): Use cp_parser_oacc_single_int_clause > > for num_gangs, num_workers and vector_length. > > Here is a similar patch for the C front end, and it also adds test cases. > OK for trunk? During testing I also noticed some "strangeness" (also > regarding some similar OpenMP clauses), which I'll file a PR for, later > on. > > commit 70baf3fc0c544fe63b9d3b3bebcca88daf7ce554 > Author: Thomas Schwinge <thomas@codesourcery.com> > Date: Fri May 5 16:38:52 2017 +0200 > > OpenACC C front end maintenance: c_parser_oacc_single_int_clause > > gcc/c/ > * c-parser.c (c_parser_omp_clause_num_gangs) > (c_parser_omp_clause_num_workers) > (c_parser_omp_clause_vector_length): Merge functions into... > (c_parser_oacc_single_int_clause): ... this new function. Adjust > all users. > gcc/testsuite/ > * c-c++-common/goacc/parallel-dims-1.c: New file. > * c-c++-common/goacc/parallel-dims-2.c: Likewise. Ok. Jakub ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: OpenACC C front end maintenance: c_parser_oacc_single_int_clause 2017-05-10 16:36 ` Jakub Jelinek @ 2017-05-12 9:38 ` Thomas Schwinge 0 siblings, 0 replies; 10+ messages in thread From: Thomas Schwinge @ 2017-05-12 9:38 UTC (permalink / raw) To: Jakub Jelinek, gcc-patches Hi! On Wed, 10 May 2017 18:32:28 +0200, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, May 09, 2017 at 11:27:14PM +0200, Thomas Schwinge wrote: > > OpenACC C front end maintenance: c_parser_oacc_single_int_clause > Ok. Thanks. Committed to trunk in r247960: commit 641fc3aef896abe9687038abfb5fe186c4f350c8 Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Fri May 12 09:33:18 2017 +0000 OpenACC C front end maintenance: c_parser_oacc_single_int_clause gcc/c/ * c-parser.c (c_parser_omp_clause_num_gangs) (c_parser_omp_clause_num_workers) (c_parser_omp_clause_vector_length): Merge functions into... (c_parser_oacc_single_int_clause): ... this new function. Adjust all users. gcc/testsuite/ * c-c++-common/goacc/parallel-dims-1.c: New file. * c-c++-common/goacc/parallel-dims-2.c: Likewise. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@247960 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/c/ChangeLog | 8 + gcc/c/c-parser.c | 176 ++++++--------------- gcc/testsuite/ChangeLog | 3 + gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c | 8 + gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c | 134 ++++++++++++++++ 5 files changed, 202 insertions(+), 127 deletions(-) diff --git gcc/c/ChangeLog gcc/c/ChangeLog index 497c9b9..b12f41d 100644 --- gcc/c/ChangeLog +++ gcc/c/ChangeLog @@ -1,3 +1,11 @@ +2017-05-12 Thomas Schwinge <thomas@codesourcery.com> + + * c-parser.c (c_parser_omp_clause_num_gangs) + (c_parser_omp_clause_num_workers) + (c_parser_omp_clause_vector_length): Merge functions into... + (c_parser_oacc_single_int_clause): ... this new function. Adjust + all users. + 2017-05-11 Nathan Sidwell <nathan@acm.org> * gimple-parser.c: Don't #include tree-dump.h. diff --git gcc/c/c-parser.c gcc/c/c-parser.c index 9398652..90d2d17 100644 --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -11339,51 +11339,6 @@ c_parser_omp_clause_nowait (c_parser *parser ATTRIBUTE_UNUSED, tree list) return c; } -/* OpenACC: - num_gangs ( expression ) */ - -static tree -c_parser_omp_clause_num_gangs (c_parser *parser, tree list) -{ - location_t num_gangs_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) - { - location_t expr_loc = c_parser_peek_token (parser)->location; - c_expr expr = c_parser_expression (parser); - expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); - tree c, t = expr.value; - t = c_fully_fold (t, false, NULL); - - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<num_gangs%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs"); - - c = build_omp_clause (num_gangs_loc, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - } - - return list; -} - /* OpenMP 2.5: num_threads ( expression ) */ @@ -11671,48 +11626,54 @@ c_parser_omp_clause_is_device_ptr (c_parser *parser, tree list) } /* OpenACC: - num_workers ( expression ) */ + num_gangs ( expression ) + num_workers ( expression ) + vector_length ( expression ) */ static tree -c_parser_omp_clause_num_workers (c_parser *parser, tree list) +c_parser_oacc_single_int_clause (c_parser *parser, omp_clause_code code, + tree list) { - location_t num_workers_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + location_t loc = c_parser_peek_token (parser)->location; + + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return list; + + location_t expr_loc = c_parser_peek_token (parser)->location; + c_expr expr = c_parser_expression (parser); + expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); + tree c, t = expr.value; + t = c_fully_fold (t, false, NULL); + + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); + + if (t == error_mark_node) + return list; + else if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - location_t expr_loc = c_parser_peek_token (parser)->location; - c_expr expr = c_parser_expression (parser); - expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); - tree c, t = expr.value; - t = c_fully_fold (t, false, NULL); + error_at (expr_loc, "%qs expression must be integral", + omp_clause_code_name[code]); + return list; + } - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, + /* Attempt to statically determine when the number isn't positive. */ + c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<num_workers%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_workers"); - - c = build_omp_clause (num_workers_loc, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; + protected_set_expr_location (c, expr_loc); + if (c == boolean_true_node) + { + warning_at (expr_loc, 0, + "%qs value must be positive", + omp_clause_code_name[code]); + t = integer_one_node; } - return list; + check_no_duplicate_clause (list, code, omp_clause_code_name[code]); + + c = build_omp_clause (loc, code); + OMP_CLAUSE_OPERAND (c, 0) = t; + OMP_CLAUSE_CHAIN (c) = list; + return c; } /* OpenACC: @@ -12354,51 +12315,6 @@ c_parser_omp_clause_untied (c_parser *parser ATTRIBUTE_UNUSED, tree list) return c; } -/* OpenACC: - vector_length ( expression ) */ - -static tree -c_parser_omp_clause_vector_length (c_parser *parser, tree list) -{ - location_t vector_length_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) - { - location_t expr_loc = c_parser_peek_token (parser)->location; - c_expr expr = c_parser_expression (parser); - expr = convert_lvalue_to_rvalue (expr_loc, expr, false, true); - tree c, t = expr.value; - t = c_fully_fold (t, false, NULL); - - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<vector_length%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length"); - - c = build_omp_clause (vector_length_loc, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - } - - return list; -} - /* OpenMP 4.0: inbranch notinbranch */ @@ -13283,11 +13199,15 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name = "link"; break; case PRAGMA_OACC_CLAUSE_NUM_GANGS: - clauses = c_parser_omp_clause_num_gangs (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_NUM_GANGS, + clauses); c_name = "num_gangs"; break; case PRAGMA_OACC_CLAUSE_NUM_WORKERS: - clauses = c_parser_omp_clause_num_workers (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_NUM_WORKERS, + clauses); c_name = "num_workers"; break; case PRAGMA_OACC_CLAUSE_PRESENT: @@ -13341,7 +13261,9 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: - clauses = c_parser_omp_clause_vector_length (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_VECTOR_LENGTH, + clauses); c_name = "vector_length"; break; case PRAGMA_OACC_CLAUSE_WAIT: diff --git gcc/testsuite/ChangeLog gcc/testsuite/ChangeLog index e1e2641..13241be 100644 --- gcc/testsuite/ChangeLog +++ gcc/testsuite/ChangeLog @@ -1,5 +1,8 @@ 2017-05-12 Thomas Schwinge <thomas@codesourcery.com> + * c-c++-common/goacc/parallel-dims-1.c: New file. + * c-c++-common/goacc/parallel-dims-2.c: Likewise. + * c-c++-common/goacc/classify-kernels-unparallelized.c: Adjust. * c-c++-common/goacc/classify-kernels.c: Likewise. * c-c++-common/goacc/kernels-counter-vars-function-scope.c: diff --git gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c new file mode 100644 index 0000000..a85d3d3 --- /dev/null +++ gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c @@ -0,0 +1,8 @@ +/* Valid use of OpenACC parallelism dimensions clauses: num_gangs, num_workers, + vector_length. */ + +void f(int i) +{ +#pragma acc parallel num_gangs(i) num_workers(i) vector_length(i) + ; +} diff --git gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c new file mode 100644 index 0000000..30a3d17 --- /dev/null +++ gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c @@ -0,0 +1,134 @@ +/* Invalid use of OpenACC parallelism dimensions clauses: num_gangs, + num_workers, vector_length. */ + +void acc_kernels(int i) +{ +#pragma acc kernels num_gangs(i) /* { dg-error "'num_gangs' is not valid for '#pragma acc kernels'" } */ + ; +#pragma acc kernels num_workers(i) /* { dg-error "'num_workers' is not valid for '#pragma acc kernels'" } */ + ; +#pragma acc kernels vector_length(i) /* { dg-error "'vector_length' is not valid for '#pragma acc kernels'" } */ + ; +} + +void acc_parallel(int i, float f) +{ +#pragma acc parallel num_gangs /* { dg-error "expected '\\(' before end of line" } */ + ; +#pragma acc parallel num_workers /* { dg-error "expected '\\(' before end of line" } */ + ; +#pragma acc parallel vector_length /* { dg-error "expected '\\(' before end of line" } */ + ; + +#pragma acc parallel num_gangs( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; +#pragma acc parallel num_workers( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; +#pragma acc parallel vector_length( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; + +#pragma acc parallel num_gangs() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; +#pragma acc parallel num_workers() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; +#pragma acc parallel vector_length() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; + +#pragma acc parallel num_gangs(1 /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel num_workers(1 /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel vector_length(1 /* { dg-error "expected '\\)' before end of line" } */ + ; + +#pragma acc parallel num_gangs(i /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel num_workers(i /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel vector_length(i /* { dg-error "expected '\\)' before end of line" } */ + ; + +#pragma acc parallel num_gangs(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel num_workers(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel vector_length(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; + +#pragma acc parallel num_gangs(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel num_workers(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel vector_length(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; + +#pragma acc parallel num_gangs(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; +#pragma acc parallel num_workers(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; +#pragma acc parallel vector_length(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; + +#pragma acc parallel num_gangs(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; +#pragma acc parallel num_workers(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; +#pragma acc parallel vector_length(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; + +#pragma acc parallel num_gangs(num_gangs) /* { dg-error "'num_gangs' (un|was not )declared" } */ + ; +#pragma acc parallel num_workers(num_workers) /* { dg-error "'num_workers' (un|was not )declared" } */ + ; +#pragma acc parallel vector_length(vector_length) /* { dg-error "'vector_length' (un|was not )declared" } */ + ; + +#pragma acc parallel num_gangs(f) /* { dg-error "'num_gangs' expression must be integral" } */ + ; +#pragma acc parallel num_workers(f) /* { dg-error "'num_workers' expression must be integral" } */ + ; +#pragma acc parallel vector_length(f) /* { dg-error "'vector_length' expression must be integral" } */ + ; + +#pragma acc parallel num_gangs((float) 1) /* { dg-error "'num_gangs' expression must be integral" } */ + ; +#pragma acc parallel num_workers((float) 1) /* { dg-error "'num_workers' expression must be integral" } */ + ; +#pragma acc parallel vector_length((float) 1) /* { dg-error "'vector_length' expression must be integral" } */ + ; + +#pragma acc parallel num_gangs(0) /* { dg-warning "'num_gangs' value must be positive" } */ + ; +#pragma acc parallel num_workers(0) /* { dg-warning "'num_workers' value must be positive" } */ + ; +#pragma acc parallel vector_length(0) /* { dg-warning "'vector_length' value must be positive" } */ + ; + +#pragma acc parallel num_gangs((int) -1.2) /* { dg-warning "'num_gangs' value must be positive" } */ + ; +#pragma acc parallel num_workers((int) -1.2) /* { dg-warning "'num_workers' value must be positive" } */ + ; +#pragma acc parallel vector_length((int) -1.2) /* { dg-warning "'vector_length' value must be positive" } */ + ; + +#pragma acc parallel \ + num_gangs(1) /* { dg-error "too many 'num_gangs' clauses" "" { target c } } */ \ + num_workers(1) /* { dg-error "too many 'num_workers' clauses" "" { target c } } */ \ + vector_length(1) /* { dg-error "too many 'vector_length' clauses" "" { target c } } */ \ + num_workers(1) /* { dg-error "too many 'num_workers' clauses" "" { target c++ } } */ \ + vector_length(1) /* { dg-error "too many 'vector_length' clauses" "" { target c++ } } */ \ + num_gangs(1) /* { dg-error "too many 'num_gangs' clauses" "" { target c++ } } */ + ; + +#pragma acc parallel \ + num_gangs(-1) /* { dg-warning "'num_gangs' value must be positive" } */ \ + num_workers() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ \ + vector_length(abc) /* { dg-error "'abc' (un|was not )declared" } */ \ + num_workers(0.5) /* { dg-error "'num_workers' expression must be integral" } */ \ + vector_length(&acc_parallel) /* { dg-error "'vector_length' expression must be integral" } */ \ + num_gangs( /* { dg-error "expected (primary-|)expression before end of line" "TODO" { xfail c } } */ + ; +} Committed to gomp-4_0-branch in r247961: commit 1c87e0644c7e9f84f62c4b1a4f5bca35fba80b36 Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Fri May 12 09:34:37 2017 +0000 OpenACC C front end maintenance: c_parser_oacc_single_int_clause gcc/c/ * c-parser.c (c_parser_omp_clause_num_gangs) (c_parser_omp_clause_num_workers) (c_parser_omp_clause_vector_length): Merge functions into... (c_parser_oacc_single_int_clause): ... this new function. Adjust all users. gcc/testsuite/ * c-c++-common/goacc/parallel-dims-1.c: New file. * c-c++-common/goacc/parallel-dims-2.c: Likewise. trunk r247960 git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@247961 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/c/ChangeLog.gomp | 8 + gcc/c/c-parser.c | 172 ++++++--------------- gcc/testsuite/ChangeLog.gomp | 3 + gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c | 9 ++ gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c | 134 ++++++++++++++++ 5 files changed, 202 insertions(+), 124 deletions(-) diff --git gcc/c/ChangeLog.gomp gcc/c/ChangeLog.gomp index c3b6798..3efcc8b 100644 --- gcc/c/ChangeLog.gomp +++ gcc/c/ChangeLog.gomp @@ -1,3 +1,11 @@ +2017-05-12 Thomas Schwinge <thomas@codesourcery.com> + + * c-parser.c (c_parser_omp_clause_num_gangs) + (c_parser_omp_clause_num_workers) + (c_parser_omp_clause_vector_length): Merge functions into... + (c_parser_oacc_single_int_clause): ... this new function. Adjust + all users. + 2017-05-04 Cesar Philippidis <cesar@codesourcery.com> * c-parser.c (c_parser_omp_clause_name): Add support for if_present. diff --git gcc/c/c-parser.c gcc/c/c-parser.c index 957007e..ef61c5f 100644 --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -11217,50 +11217,6 @@ c_parser_omp_clause_nowait (c_parser *parser ATTRIBUTE_UNUSED, tree list) return c; } -/* OpenACC: - num_gangs ( expression ) */ - -static tree -c_parser_omp_clause_num_gangs (c_parser *parser, tree list) -{ - location_t num_gangs_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) - { - location_t expr_loc = c_parser_peek_token (parser)->location; - tree c, t = c_parser_expression (parser).value; - mark_exp_read (t); - t = c_fully_fold (t, false, NULL); - - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<num_gangs%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_GANGS, "num_gangs"); - - c = build_omp_clause (num_gangs_loc, OMP_CLAUSE_NUM_GANGS); - OMP_CLAUSE_NUM_GANGS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - } - - return list; -} - /* OpenMP 2.5: num_threads ( expression ) */ @@ -11542,47 +11498,53 @@ c_parser_omp_clause_is_device_ptr (c_parser *parser, tree list) } /* OpenACC: - num_workers ( expression ) */ + num_gangs ( expression ) + num_workers ( expression ) + vector_length ( expression ) */ static tree -c_parser_omp_clause_num_workers (c_parser *parser, tree list) +c_parser_oacc_single_int_clause (c_parser *parser, omp_clause_code code, + tree list) { - location_t num_workers_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + location_t loc = c_parser_peek_token (parser)->location; + + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return list; + + location_t expr_loc = c_parser_peek_token (parser)->location; + tree c, t = c_parser_expression (parser).value; + mark_exp_read (t); + t = c_fully_fold (t, false, NULL); + + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); + + if (t == error_mark_node) + return list; + else if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) { - location_t expr_loc = c_parser_peek_token (parser)->location; - tree c, t = c_parser_expression (parser).value; - mark_exp_read (t); - t = c_fully_fold (t, false, NULL); + error_at (expr_loc, "%qs expression must be integral", + omp_clause_code_name[code]); + return list; + } - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, + /* Attempt to statically determine when the number isn't positive. */ + c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<num_workers%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_NUM_WORKERS, "num_workers"); - - c = build_omp_clause (num_workers_loc, OMP_CLAUSE_NUM_WORKERS); - OMP_CLAUSE_NUM_WORKERS_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; + protected_set_expr_location (c, expr_loc); + if (c == boolean_true_node) + { + warning_at (expr_loc, 0, + "%qs value must be positive", + omp_clause_code_name[code]); + t = integer_one_node; } - return list; + check_no_duplicate_clause (list, code, omp_clause_code_name[code]); + + c = build_omp_clause (loc, code); + OMP_CLAUSE_OPERAND (c, 0) = t; + OMP_CLAUSE_CHAIN (c) = list; + return c; } /* OpenACC: @@ -12342,50 +12304,6 @@ c_parser_omp_clause_untied (c_parser *parser ATTRIBUTE_UNUSED, tree list) return c; } -/* OpenACC: - vector_length ( expression ) */ - -static tree -c_parser_omp_clause_vector_length (c_parser *parser, tree list) -{ - location_t vector_length_loc = c_parser_peek_token (parser)->location; - if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) - { - location_t expr_loc = c_parser_peek_token (parser)->location; - tree c, t = c_parser_expression (parser).value; - mark_exp_read (t); - t = c_fully_fold (t, false, NULL); - - c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>"); - - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))) - { - c_parser_error (parser, "expected integer expression"); - return list; - } - - /* Attempt to statically determine when the number isn't positive. */ - c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, - build_int_cst (TREE_TYPE (t), 0)); - protected_set_expr_location (c, expr_loc); - if (c == boolean_true_node) - { - warning_at (expr_loc, 0, - "%<vector_length%> value must be positive"); - t = integer_one_node; - } - - check_no_duplicate_clause (list, OMP_CLAUSE_VECTOR_LENGTH, "vector_length"); - - c = build_omp_clause (vector_length_loc, OMP_CLAUSE_VECTOR_LENGTH); - OMP_CLAUSE_VECTOR_LENGTH_EXPR (c) = t; - OMP_CLAUSE_CHAIN (c) = list; - list = c; - } - - return list; -} - /* OpenMP 4.0: inbranch notinbranch */ @@ -13292,11 +13210,15 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name = "nohost"; break; case PRAGMA_OACC_CLAUSE_NUM_GANGS: - clauses = c_parser_omp_clause_num_gangs (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_NUM_GANGS, + clauses); c_name = "num_gangs"; break; case PRAGMA_OACC_CLAUSE_NUM_WORKERS: - clauses = c_parser_omp_clause_num_workers (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_NUM_WORKERS, + clauses); c_name = "num_workers"; break; case PRAGMA_OACC_CLAUSE_PRESENT: @@ -13330,7 +13252,9 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask, c_name, clauses); break; case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH: - clauses = c_parser_omp_clause_vector_length (parser, clauses); + clauses = c_parser_oacc_single_int_clause (parser, + OMP_CLAUSE_VECTOR_LENGTH, + clauses); c_name = "vector_length"; break; case PRAGMA_OACC_CLAUSE_WAIT: diff --git gcc/testsuite/ChangeLog.gomp gcc/testsuite/ChangeLog.gomp index dadff1a..c24820d 100644 --- gcc/testsuite/ChangeLog.gomp +++ gcc/testsuite/ChangeLog.gomp @@ -1,5 +1,8 @@ 2017-05-12 Thomas Schwinge <thomas@codesourcery.com> + * c-c++-common/goacc/parallel-dims-1.c: New file. + * c-c++-common/goacc/parallel-dims-2.c: Likewise. + * c-c++-common/goacc/classify-kernels-unparallelized.c: Adjust. * c-c++-common/goacc/classify-kernels.c: Likewise. * c-c++-common/goacc/kernels-acc-loop-reduction.c: Likewise. diff --git gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c new file mode 100644 index 0000000..9e4cfaa --- /dev/null +++ gcc/testsuite/c-c++-common/goacc/parallel-dims-1.c @@ -0,0 +1,9 @@ +/* Valid use of OpenACC parallelism dimensions clauses: num_gangs, num_workers, + vector_length. */ + +void f(int i) +{ +#pragma acc parallel /* { dg-bogus "region is (gang|worker|vector) partitioned" "" { xfail *-*-* } } */ \ + num_gangs(i) num_workers(i) vector_length(i) + ; +} diff --git gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c new file mode 100644 index 0000000..30a3d17 --- /dev/null +++ gcc/testsuite/c-c++-common/goacc/parallel-dims-2.c @@ -0,0 +1,134 @@ +/* Invalid use of OpenACC parallelism dimensions clauses: num_gangs, + num_workers, vector_length. */ + +void acc_kernels(int i) +{ +#pragma acc kernels num_gangs(i) /* { dg-error "'num_gangs' is not valid for '#pragma acc kernels'" } */ + ; +#pragma acc kernels num_workers(i) /* { dg-error "'num_workers' is not valid for '#pragma acc kernels'" } */ + ; +#pragma acc kernels vector_length(i) /* { dg-error "'vector_length' is not valid for '#pragma acc kernels'" } */ + ; +} + +void acc_parallel(int i, float f) +{ +#pragma acc parallel num_gangs /* { dg-error "expected '\\(' before end of line" } */ + ; +#pragma acc parallel num_workers /* { dg-error "expected '\\(' before end of line" } */ + ; +#pragma acc parallel vector_length /* { dg-error "expected '\\(' before end of line" } */ + ; + +#pragma acc parallel num_gangs( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; +#pragma acc parallel num_workers( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; +#pragma acc parallel vector_length( /* { dg-error "expected (primary-|)expression before end of line" } */ + ; + +#pragma acc parallel num_gangs() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; +#pragma acc parallel num_workers() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; +#pragma acc parallel vector_length() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ + ; + +#pragma acc parallel num_gangs(1 /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel num_workers(1 /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel vector_length(1 /* { dg-error "expected '\\)' before end of line" } */ + ; + +#pragma acc parallel num_gangs(i /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel num_workers(i /* { dg-error "expected '\\)' before end of line" } */ + ; +#pragma acc parallel vector_length(i /* { dg-error "expected '\\)' before end of line" } */ + ; + +#pragma acc parallel num_gangs(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel num_workers(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel vector_length(1 i /* { dg-error "expected '\\)' before 'i'" } */ + ; + +#pragma acc parallel num_gangs(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel num_workers(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; +#pragma acc parallel vector_length(1 i) /* { dg-error "expected '\\)' before 'i'" } */ + ; + +#pragma acc parallel num_gangs(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; +#pragma acc parallel num_workers(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; +#pragma acc parallel vector_length(1, i /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + /* { dg-bogus "expected '\\)' before end of line" "TODO" { xfail c } .-1 } */ + ; + +#pragma acc parallel num_gangs(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; +#pragma acc parallel num_workers(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; +#pragma acc parallel vector_length(1, i) /* { dg-error "expected '\\)' before ',' token" "TODO" { xfail c } } */ + ; + +#pragma acc parallel num_gangs(num_gangs) /* { dg-error "'num_gangs' (un|was not )declared" } */ + ; +#pragma acc parallel num_workers(num_workers) /* { dg-error "'num_workers' (un|was not )declared" } */ + ; +#pragma acc parallel vector_length(vector_length) /* { dg-error "'vector_length' (un|was not )declared" } */ + ; + +#pragma acc parallel num_gangs(f) /* { dg-error "'num_gangs' expression must be integral" } */ + ; +#pragma acc parallel num_workers(f) /* { dg-error "'num_workers' expression must be integral" } */ + ; +#pragma acc parallel vector_length(f) /* { dg-error "'vector_length' expression must be integral" } */ + ; + +#pragma acc parallel num_gangs((float) 1) /* { dg-error "'num_gangs' expression must be integral" } */ + ; +#pragma acc parallel num_workers((float) 1) /* { dg-error "'num_workers' expression must be integral" } */ + ; +#pragma acc parallel vector_length((float) 1) /* { dg-error "'vector_length' expression must be integral" } */ + ; + +#pragma acc parallel num_gangs(0) /* { dg-warning "'num_gangs' value must be positive" } */ + ; +#pragma acc parallel num_workers(0) /* { dg-warning "'num_workers' value must be positive" } */ + ; +#pragma acc parallel vector_length(0) /* { dg-warning "'vector_length' value must be positive" } */ + ; + +#pragma acc parallel num_gangs((int) -1.2) /* { dg-warning "'num_gangs' value must be positive" } */ + ; +#pragma acc parallel num_workers((int) -1.2) /* { dg-warning "'num_workers' value must be positive" } */ + ; +#pragma acc parallel vector_length((int) -1.2) /* { dg-warning "'vector_length' value must be positive" } */ + ; + +#pragma acc parallel \ + num_gangs(1) /* { dg-error "too many 'num_gangs' clauses" "" { target c } } */ \ + num_workers(1) /* { dg-error "too many 'num_workers' clauses" "" { target c } } */ \ + vector_length(1) /* { dg-error "too many 'vector_length' clauses" "" { target c } } */ \ + num_workers(1) /* { dg-error "too many 'num_workers' clauses" "" { target c++ } } */ \ + vector_length(1) /* { dg-error "too many 'vector_length' clauses" "" { target c++ } } */ \ + num_gangs(1) /* { dg-error "too many 'num_gangs' clauses" "" { target c++ } } */ + ; + +#pragma acc parallel \ + num_gangs(-1) /* { dg-warning "'num_gangs' value must be positive" } */ \ + num_workers() /* { dg-error "expected (primary-|)expression before '\\)' token" } */ \ + vector_length(abc) /* { dg-error "'abc' (un|was not )declared" } */ \ + num_workers(0.5) /* { dg-error "'num_workers' expression must be integral" } */ \ + vector_length(&acc_parallel) /* { dg-error "'vector_length' expression must be integral" } */ \ + num_gangs( /* { dg-error "expected (primary-|)expression before end of line" "TODO" { xfail c } } */ + ; +} Grüße Thomas ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-05-12 9:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-30 0:03 [OpenACC] num_gangs, num_workers and vector_length in c++ Cesar Philippidis 2015-10-30 13:41 ` Jakub Jelinek 2015-10-30 14:43 ` Cesar Philippidis 2015-10-30 17:07 ` Jakub Jelinek 2015-10-30 21:28 ` Cesar Philippidis 2015-11-04 18:09 ` Jason Merrill 2015-11-04 22:10 ` Cesar Philippidis 2017-05-09 21:27 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause (was: [OpenACC] num_gangs, num_workers and vector_length in c++) Thomas Schwinge 2017-05-10 16:36 ` Jakub Jelinek 2017-05-12 9:38 ` OpenACC C front end maintenance: c_parser_oacc_single_int_clause Thomas Schwinge
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).