On Mon, Dec 16, 2013 at 09:41:43PM +0000, Iyer, Balaji V wrote:
> --- gcc/c/c-parser.c (revision 205759)
> +++ gcc/c/c-parser.c (working copy)
> @@ -208,6 +208,12 @@
> /* True if we are in a context where the Objective-C "Property attribute"
> keywords are valid. */
> BOOL_BITFIELD objc_property_attr_context : 1;
> +
> + /* Cilk Plus specific parser/lexer information. */
> +
> + /* Buffer to hold all the tokens from parsing the vector attribute for the
> + SIMD-enabled functions (formerly known as elemental functions). */
> + vec <c_token, va_gc> *cilk_simd_fn_tokens;
> } c_parser;
Joseph, is this ok for you?
> +/* Returns true of NAME is an IDENTIFIER_NODE with identiifer "vector,"
> + "__vector" or "__vector__." */
> +
> +static bool
static inline bool
> +is_cilkplus_vector_p (tree name)
> +{
> + if (flag_enable_cilkplus
> + && (simple_cst_equal (name, get_identifier ("vector")) == 1
> + || simple_cst_equal (name, get_identifier ("__vector")) == 1
> + || simple_cst_equal (name, get_identifier ("__vector__")) == 1))
> + return true;
> + return false;
> +}
Why not just
return flag_enable_cilkplus && is_attribute_p ("vector", name);
?
> +#define CILK_SIMD_FN_CLAUSE_MASK \
> + ( (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SIMDLEN) \
> + | (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_LINEAR) \
> + | (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_UNIFORM) \
> + | (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_INBRANCH) \
> + | (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NOTINBRANCH))
I thought you'd instead add there PRAGMA_CILK_CLAUSE_VECTORLENGTH,
PRAGMA_CILK_CLAUSE_MASK and PRAGMA_CILK_CLAUSE_NOMASK (or similar).
> + if (token->type == CPP_NAME
> + && TREE_CODE (token->value) == IDENTIFIER_NODE)
> + if (simple_cst_equal (token->value,
> + get_identifier ("vectorlength")) == 1)
Why the simple_cst_equal + get_identifier? I mean,
strcmp on IDENTIFIER_POINTER should be cheaper, and done elsewhere
in c-parser.c.
> + {
> + if (!c_parser_cilk_clause_vectorlength (parser, NULL, true))
Why do you parse it here? Just parse it when parsing other clauses,
and only during parsing of vectorlength clause create OMP_CLAUSE_SAFELEN
out of it (after all the verifications).
> + if (is_cilk_simd_fn && TREE_CODE (step) == PARM_DECL)
> + {
> + error_at (clause_loc, "using parameters for %<linear%> step is "
> + "not supported in this release");
> + step = integer_one_node;
That would be sorry, not error_at.
> here = c_parser_peek_token (parser)->location;
> - c_kind = c_parser_omp_clause_name (parser);
> +
> + if (mask == CILK_SIMD_FN_CLAUSE_MASK)
Ugh, no.
> + c_kind = c_parser_cilk_simd_fn_clause_name (parser);
> + else
> + c_kind = c_parser_omp_clause_name (parser);
Always parse it the same, just use PRAGMA_CILK_CLAUSE_* for
the Cilk+ specific clauses.
>
> switch (c_kind)
> {
> @@ -10933,7 +11092,8 @@
> c_name = "aligned";
> break;
> case PRAGMA_OMP_CLAUSE_LINEAR:
> - clauses = c_parser_omp_clause_linear (parser, clauses);
> + clauses = c_parser_omp_clause_linear
> + (parser, clauses, mask == CILK_SIMD_FN_CLAUSE_MASK);
Again, this is too ugly. Perhaps check if
(mask & PRAGMA_CILK_CLAUSE_VECTORLENGTH) != 0 or similar.
Jakub
