This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [OpenACC] num_gangs, num_workers and vector_length in c++


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
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:

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]