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

Reply via email to