On Mon, Jun 13, 2022 at 09:29:34PM +0800, Chung-Lin Tang wrote:
> > I was hoping you'd drop all this.
> > Seehttps://gcc.gnu.org/r13-1002
> > for implementation (both C and C++ FE) of something very similar,
> > the only difference there is that in the case of linear clause, it is
> > looking for
> > val
> > ref
> > uval
> > step ( whatever )
> > followed by , or )
> > (anod ref and uval not in C FE),
> > while you are looking for
> > memspace ( whatever )
> > traits ( whatever )
> > followed by : or by , (in case of , repeat).
> > But in both cases you can actually use the same parser APIs
> > for raw token pre-parsing to just compute if it is the modifier
> > syntax or not, set bool has_modifiers based on that (when you
> > come over probably valid syntax followed by CPP_COLON).
> 
> The linear clause doesn't have the legacy 'allocator1(t1), allocator2(t2), 
> ...' requirement,
> and c_parser_omp_variable_list doesn't seem to support this pattern.

True, but I don't see why it is relevant.

> Also, the way c_parser_omp_clause_linear is implemented doesn't support the 
> requirement
> you mentioned earlier of allowing the use of "memspace", "traits" as the 
> allocator name when
> it's actually not a modifier.

No, it is exactly the same thing.
As you can see e.g. in the testsuite coverage I've committed in the linear
patch, in the linear clause case after : either one uses a modifier syntax,
or everything after the : is the step expression (assignment-expression in
C/C++).  There is parsing ambiguity and the spec says that it is resolved
to the modifier syntax in that case.
Say if I have:
constexpr int step (int x) { return x; }
and use
linear (a, b, c : step (1))
then it is the modifier syntax (incompatible change from 5.1), while
linear (a, b, c : step (1) + 0)
linear (a, b, c : (step (1)))
linear (a, b, c : 0 + step (1))
etc. have step expressions.  The spec wording is such that it doesn't even
have to be discovered by strictly tentative parsing (like in GCC the C++ and
Fortran FEs do but C FE doesn't), modifier syntax wins if one sees the
modifiers with balanced () after it if it is complex, followed by , or ) as
a terminator of a single modifier.
The first raw token walk in the patch is just a fast "tentative" parsing
check whether it is modifier syntax or not, if it is, then we just parse it
as modifiers, otherwise parse it as expression.

The uses_allocator is similar, although in that case it actually isn't
a parsing ambiguity, just that we need arbitrary number of tokens look-ahead
to decide.  We need to check if the tokens after uses_allocators (
look like one or more complex modifiers (with the 2 modifier names and just
a balanced ()s after them - in the uses_allocators case currently all
supported modifiers are complex), if yes and it is followed by : token,
it is the modifiers syntax, otherwise it is not.
So say:
#include <omp.h>
void foo (void)
{
  omp_alloc_handle_t traits, x;
  const omp_alloctrait_t my_traits[] = { ... };
  #pragma omp target uses_allocators (traits (my_traits) : x)
  ;
  #pragma omp target uses_allocators (traits (my_traits), x (my_traits))
  ;
  #pragma omp target uses_allocators (traits (my_traits), 
omp_high_mem_bw_mem_alloc)
  ;
  #pragma omp target uses_allocators (traits (my_traits))
  ;
}
All the clauses above start with the same tokens, but depending on what
follows we need to either parse it as the modifier syntax (the first
directive) or as the compatibility syntax (the rest).

Which is why I was suggesting to do this quick raw token parsing check
if it is the modifier syntax or not.
If it is, parse modifiers and : and then you know to expect a single
allocator without ()s after it (e.g. you could use
c_parser_omp_variable_list etc. and just verify afterwards the list
has a single entry in it).
If it is not, it might still be old or new syntax, the latter only if
the list contains a single var and not followed by ()s and sure, you need
to write a parsing loop for that.  It isn't the same thing as the modifier
loop though, modifiers start with a keyword, the allocator list with
a identifier for the variable.

For uses_allocators, we can then even simplify when we almost finish
OpenMP 6.0 support, if the old style syntax uses_allocators are gone
by then, we could decide if it is a modifier syntax or not just by
looking at first 2 tokens, whether the first token is allowed modifier
keyword and whether it is followed by (, then we could commit to
modifier parsing right away.  And the loop to do the ()s parsing
can go too...

        Jakub

Reply via email to