On Thu, Dec 19, 2013 at 06:12:29PM +0000, Iyer, Balaji V wrote:
> 2013-12-19 Balaji V. Iyer <[email protected]>
>
> * parser.c (cp_parser_direct_declarator): When Cilk Plus is enabled
> see if there is an attribute after function decl. If so, then
> parse them now.
> (cp_parser_late_return_type_opt): Handle parsing of Cilk Plus SIMD
> enabled function late parsing.
> (cp_parser_gnu_attribute_list): Parse all the tokens for the vector
> attribute for a SIMD-enabled function.
> (cp_parser_omp_all_clauses): Skip parsing to the end of pragma when
> the function is used by SIMD-enabled function (indicated by NULL
> pragma token). Added 3 new clauses: PRAGMA_CILK_CLAUSE_MASK,
> PRAGMA_CILK_CLAUSE_NOMASK and PRAGMA_CILK_CLAUSE_VECTORLENGTH
> (cp_parser_cilk_simd_vectorlength): Modified this function to handle
> vectorlength clause in SIMD-enabled function and #pragma SIMD's
> vectorlength clause. Added a new bool parameter to differentiate
> between the two.
> (cp_parser_cilk_simd_fn_vector_attrs): New function.
> (is_cilkplus_vector_p): Likewise.
> (cp_parser_late_parsing_elem_fn_info): Likewise.
> (cp_parser_omp_clause_name): Added a check for "mask," "nomask"
The comma should have been after " .
> + /* In here, we handle cases where attribute is used after
> + the function declaration. For example:
> + void func (int x) __attribute__((vector(..))); */
> + if (flag_enable_cilkplus
> + && cp_next_tokens_can_be_attribute_p (parser))
As you are just calling cp_parser_gnu_attributes_opt here and not
..._std_..., I'd say the above should be
cp_next_tokens_can_be_gnu_attribute_p rather than
cp_next_tokens_can_be_attribute_p. I think [[...]] attributes at this
position are ignored, so no need to handle them, not sure about
whether we allow e.g. combination of GNU and std attributes or vice versa.
> + {
> + cp_parser_parse_tentatively (parser);
> + tree attr = cp_parser_gnu_attributes_opt (parser);
> + if (cp_lexer_next_token_is_not (parser->lexer,
> + CPP_SEMICOLON)
> + && cp_lexer_next_token_is_not (parser->lexer,
> + CPP_OPEN_BRACE))
> + cp_parser_abort_tentative_parse (parser);
> + else if (!cp_parser_parse_definitely (parser))
> + ;
> + else
> + attrs = chainon (attr, attrs);
> + }
> late_return = (cp_parser_late_return_type_opt
> (parser, declarator,
> memfn ? cv_quals : -1));
> @@ -17842,6 +17868,10 @@ cp_parser_late_return_type_opt (cp_parser* parser,
> cp_declarator *declarator,
> type = cp_parser_trailing_type_id (parser);
> }
>
> + if (cilk_simd_fn_vector_p)
> + declarator->std_attributes
> + = cp_parser_late_parsing_cilk_simd_fn_info (parser,
> + declarator->std_attributes);
Please make sure declarator is aligned below parser.
> + token->type = CPP_PRAGMA_EOL;
> + parser->lexer->next_token = token;
> + cp_lexer_consume_token (parser->lexer);
> +
> + struct cp_token_cache *cp =
> + cp_token_cache_new (v_token, cp_lexer_peek_token (parser->lexer));
The = should already go on the next line.
> +/* Handles the delayed parsing of the Cilk Plus SIMD-enabled function.
> + This function is modelled similar to the late parsing of omp declare
> + simd. */
> +
> +static tree
> +cp_parser_late_parsing_cilk_simd_fn_info (cp_parser *parser, tree attrs)
> +{
> + struct cp_token_cache *ce;
> + cp_omp_declare_simd_data *info = parser->cilk_simd_fn_info;
> + int ii = 0;
> +
> + if (parser->omp_declare_simd != NULL)
> + {
> + error ("%<#pragma omp declare simd%> cannot be used in the same
> function"
> + " marked as a Cilk Plus SIMD-enabled function");
> + parser->cilk_simd_fn_info = NULL;
This will leak parser->cilk_simd_fn_info memory. Please XDELETE it first.
> + return attrs;
> + }
> + if (!info->error_seen && info->fndecl_seen)
> + {
> + error ("vector attribute not immediately followed by a single function"
> + " declaration or definition");
> + info->error_seen = true;
> + }
> + if (info->error_seen)
> + return attrs;
> +
> + /* Vector attributes are converted to #pragma omp declare simd values and
> + so we need them enabled. */
> + flag_openmp = 1;
The C FE doesn't do this. I thought all the omp-low.c spots are now guarded
by flag_openmp || flag_enable_cilkplus etc. conditions.
> + c = build_tree_list (get_identifier ("cilk simd function"), cl);
Please use NULL_TREE instead of cl here and please fix up the C FE as well,
the "cilk simd function" attribute should be just an attribute without
value, serving only as a boolean, if present, decl with
"omp declare simd" attribute is Cilk+, otherwise OpenMP.
> + TREE_CHAIN (c) = attrs;
> + if (processing_template_decl)
> + ATTR_IS_DEPENDENT (c) = 1;
But, as a boolean attribute it doesn't and shouldn't be ATTR_IS_DEPENDENT,
there is nothing to adjust in it.
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -8602,9 +8602,12 @@ apply_late_template_attributes (tree *decl_p, tree
> attributes, int attr_flags,
> {
> *p = TREE_CHAIN (t);
> TREE_CHAIN (t) = NULL_TREE;
> - if (flag_openmp
> - && is_attribute_p ("omp declare simd",
> - get_attribute_name (t))
> + if (((flag_openmp
> + && is_attribute_p ("omp declare simd",
> + get_attribute_name (t)))
> + || (flag_enable_cilkplus
> + && is_attribute_p ("cilk simd function",
> + get_attribute_name (t))))
> && TREE_VALUE (t))
> {
> tree clauses = TREE_VALUE (TREE_VALUE (t));
And this change is unnecessary after the above suggested change.
Jakub
