[PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C

[Jakub Jelinek]

On Fri, Dec 13, 2013 at 06:37:22PM +0000, Iyer, Balaji V wrote:
> @@ -3765,6 +3777,93 @@
>    return attr_name;
>  }
>  
> +/* Parses the vector attribute of SIMD enabled functions in Cilk Plus.
> +   VEC_TOKEN is the "vector" token that is replaced with "simd" and
> +   pushed into the token list. 
> +   Syntax:
> +   vector
> +   vector (<vector attributes>).  */
> +
> +static void
> +c_parser_cilk_simd_fn_vector_attrs (c_parser *parser, c_token vec_token)
> +{
> +  gcc_assert (simple_cst_equal (vec_token.value,
> +                                get_identifier ("vector")) == 1);
> +  int paren_scope = 0;
> +  /* Replace the vector keyword with SIMD.  */
> +  vec_token.value = get_identifier ("simd");
> +  vec_safe_push (parser->cilk_simd_fn_tokens, vec_token);

So, why don't you just push the "vector" token as is?

> +          if (simple_cst_equal (value, get_identifier ("mask")) == 1)
> +            token->value = get_identifier ("inbranch");
> +          else if (simple_cst_equal (value, get_identifier ("nomask")) == 1)
> +            token->value = get_identifier ("notinbranch");
> +          else if (simple_cst_equal (value,
> +                                     get_identifier ("vectorlength")) == 1)
> +            {
> +              if (!c_parser_cilk_clause_vectorlength (parser, NULL, true))
> +                {
> +                  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
> +                  return;
> +                }
> +              else
> +                continue;
> +            }

This I don't like at all, tweaking tokens is just too ugly.
I'd prefer if you just keep the tokens as is, and simply:

>    while (parser->tokens_avail > 3)
>      {
> @@ -12792,11 +12922,22 @@

Allow "vector" (and I think you need to also allow __vector and __vector__,
don't you?) here too, for flag_enable_cilkplus.

BTW, can you as a followup rename flag_enable_cilkplus to flag_cilkplus?


>        c_parser_consume_token (parser);
>        parser->in_pragma = true;
>  
> -      tree c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK,
> -					 "#pragma omp declare simd");
> +      tree c = NULL_TREE;
> +      if (is_cilkplus_cilk_simd_fn) 
> +	c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK,
> +				      "SIMD-enabled functions attribute");

You'd use different mask here that would not include the clauses Cilk+
doesn't have (inbranch/notinbranch/safelen) and include the one Cilk+ has,
and make sure you handle parsing those.  And just parsing of say mask clause
tokens would create OMP_CLAUSE_INBRANCH clause.

BTW, I don't see how you handle the Cilk+ specific enhancement to linear
clause, that you can refer to some parameter instead.  The question is how
would c_parser_omp_clause_linear know whether it is now parsing OpenMP
clauses or Cilk+ vector attribute tokens.  Plus there are no testcases for
that (though, it will likely need some further omp-low.c and vectorizer
changes), but at least Aldy has reserved a bit and created macro for it
on the clause.

	Jakub