On Mon, Nov 18, 2013 at 10:35:50PM +0000, Iyer, Balaji V wrote:
> Attached, please find a patch that will implement SIMD enabled functions
> for C targeting the gomp-4_0-branch. Here are the ChangeLog entries. Is
> this OK to install?
Have you tested say:
int func9 (int x, int y) __attribute__ ((vector (uniform (y), mask)));
? I mean, mostly you pass NULL to c_parser_attributes as the new argument,
but then dereference it unconditionally.
You should error out if a function has
#pragma omp declare simd
and
vector attribute, defining what it means doesn't make sense.
Also, I'm not sure I like doing the transformation from Cilk+ to OpenMP
syntax through rewriting tokens, rather than at the parsing level.
After all, the Cilk+ syntax is quite different, even when the patch pretends
it is the same, consider e.g. the linear clause, which in Cilk+ allows
to refer to an argument, in OpenMP doesn't.
Also, I wonder if you couldn't save the tokens wrapped into some tree
temporarily into the attribute, rather than having to adjust
c_parser_attribute callers. Joseph, what do you prefer here?
> @@ -1502,9 +1502,17 @@
> c_parser_peek_token (parser)->value = error_mark_node;
> fndef_ok = !nested;
> }
> + /* In Cilk Plus SIMD-enabled functions (formerly known as Elemental
> + Functions), attributes are used right above the functoin declaration or
> + the function itself. */
Spelling.
> + tree attrs = NULL_TREE;
> + if (flag_enable_cilkplus
> + && c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
> + attrs = c_parser_attributes (parser, &omp_declare_simd_clauses);
This looks wrong, parsing of the attributes is supposed to be done
in c_parser_declspecs and can be intermixed with various other tokens
(e.g. static, extern, etc.).
> c_parser_declspecs (parser, specs, true, true, start_attr_ok,
> true, true, cla_nonabstract_decl);
> + specs->attrs = chainon (attrs, specs->attrs);
> if (parser->error)
> {
> c_parser_skip_to_end_of_block_or_statement (parser);
> + /* Since we are converting an attribute to a pragma, we need to end the
> + attribute with PRAGMA_EOL. OpenMP guys would like to have 2 CPP_EOF
> + at the end, and so we insert that also. */
Comment wrongly indented. It isn't about me who likes to have there
2 CPP_EOF, but just a safety net, because normally C FE has two tokens
look-ahead.
Jakub
