[Jakub, see below]
+ if (!c_parser_elem_fn_vectorlength (parser)) +
{ + c_parser_skip_until_found (parser,
CPP_CLOSE_PAREN, NULL); + /* NO reason to keep
any of these tokens if the + vectorlength is
messed up. */ + vec_free (parser->elem_fn_tokens); +
return;
It may be cleaner to make the caller free the vector.
Well, the caller doesn't know if an error has occurred. I suppose I
could do something like check for seen_error (), but this sounds a
bit more clearer (to me altleast)
Sorry, what I meant to say is that it may be cleaner if
c_parser_elem_fn_vectorlength (or whatever it's called now) frees the
vector upon error before returning.
First of all, it's a really bad idea to scan all the functions
twice. You should have adapted expand_simd_clones() to do the work
for both.
OK. I included this in the first loop itself so we won't have to
scan the functions twice.
But even so, I don't think you need to do this at all. Aren't Cilk
Plus elementals supposed to be tagged as "omp declare simd" as
well? In which case expand_simd_clones() will DTRT. It
should...and then simd_clone_clauses_extract() already has the
smarts to differentiate between the both variants.
Yes, the thing is, there is a big do-while loop in the function and
that needs to be replaced if we have to check for SIMD-enabled
function and #pragma omp declare simd. If we pass it as a type, then
it just needs to check for the type string.
But aren't both OpenMP and Cilk Plus simd clones marked as "omp declare
simd"? In which case you shouldn't have to do anything? Are the Cilk
Plus clones not being marked as "omp declare simd" in the front-ends?
I narrowed this to 5 with the help of Jakub a while back. But now, I
have replaced it with 3, "-O3 -g"," -O3 -g -std=c99"
I would prefer to get rid of -O3, since inlining may do interesting
things to tests, and you'll have to use __attribute__((noinline)) to
test some things.
Jakub, would you be ok with "-O0 -g" and "-O0 -std=c99"? For that
matter, I'd say pass no arguments at all (""), and let the test itself
test something special if required.
For that matter, we should probably get rid of all the variant
testing in the rest of Cilk Plus.
I will send this out as a different patch later for all the others.
Thank you. Do so after Jakub responds as to what he prefers.
Renamed "EF" to "SE" (Simd Enabled function)
If you must, but I would still prefer something more meaningful (i.e.,
not an abbreviation). I know there is precedence with the
array-notation feature, but I dislike that too :). Feel free to ignore
me on this one.
+/* Parses the vector attribute of SIMD enabled functions in Cilk Plus.
<insert space>
+ The VEC_TOKEN is the "vector" token that is replaced with "simd" and
+ pushed into the token list.
<insert space>
+ Syntax:
+ vector
+ vector (<vector attributes>). */
Also, s/The VEC_TOKEN/VEC_TOKEN/
+static void
+c_parser_cilk_simd_fn_expr_list (c_parser *parser, c_token vec_token)
This is a parsing routine for the vector attribute, let's call this
"c_parser_cilk_simd_fn_vector" or "c_parser_cilk_simd_fn_vector_attrs".
The expr_list is confusing.
+ /* NO reason to keep any of these tokens if the
+ vectorlength is messed up. */
Lower case "NO".
+ vec_free (parser->cilk_simd_fn_tokens);
+ // parser->cilk_simd_fn_tokens->release ();
+ return;
What's this commented out code? If unnecessary, remove it.
+ return;
+}
Empty return at end of function. Remove it.
+ /* c_parser_attributes is called in several places, and so if these EOF
s/and so/so/
+ /* Two EOF_token is added as a safety-net since the normal C front-end has
+ two token look-ahead. */
Shouldn't that be, "Two CPP_EOF tokens" ??
+ error ("%<#pragma omp declare simd%> cannot be used in the same "
+ "function marked as a SIMD-enabled function");
Perhaps we should say "...as a Cilk Plus SIMD-enabled...", to make it
absolutely clear that it is OpenMP and Cilk Plus that can't coexist.
/* Cilk Plus:
- vectorlength ( constant-expression ) */
+ This function is shared by SIMD-enabled functions and #pragma simd.
+ If IS_SIMD_FN is true then it is parsing a SIMD-enabled function and
+ CLAUSES is unused.
+ Syntax:
+ vectorlength ( constant-expression ) */
-static tree
-c_parser_cilk_clause_vectorlength (c_parser *parser, tree clauses)
+static bool
+c_parser_cilk_clause_vectorlength (c_parser *parser, tree *clauses,
+ bool is_simd_fn)
Can you document what this function does? Also, document the fact that
when IS_SIMD_FN is true, we are merely caching the tokens, otherwise we
are building the OMP_CLAUSE_SAFELEN.
/* if expr is error_mark_node, then the returning function would have
flagged the error. No need to flag them twice. */
s/if expr/If expr/
Also "flag it twice", in keeping with the tense in the previous sentence.
if (is_simd_fn)
{
c_token new_token;
/* If we are here then it has be a number. */
new_token.type = CPP_NUMBER;
new_token.keyword = RID_MAX;
I thought we agreed integral constants were allowed. Why would we be
expecting only a number? Also, it should have been "it has TO be a number".
Aldy
