On 05/21/13 10:46, Iyer, Balaji V wrote:
Thank you for working on this!
static tree
find_invalid_stmts (tree *tp, int *walk_subtrees, void *data)
{
bool *valid = (bool *) data;
location_t loc = EXPR_HAS_LOCATION (*tp) ? EXPR_LOCATION (*tp) :
UNKNOWN_LOCATION;
if (!tp || !*tp)
return NULL_TREE;
else if (TREE_CODE (*tp) == GOTO_EXPR)
{
error_at (loc, "goto statements are not allowed inside "
"loops marked with #pragma simd");
*valid = false;
*walk_subtrees = 0;
}
As I've said in the comment to the analogous C function
(find_invalid_stmts_in_loops), you can't just check for GOTO_EXPRs,
since they can also be generated by switches and other loop constructs.
I will later add some generic code to check this sometime after the
CFG is built. For now, you can remove the check for GOTO_EXPR.
else if (TREE_CODE (*tp) == CALL_EXPR)
{
tree fndecl = CALL_EXPR_FN (*tp);
if (TREE_CODE (fndecl) == ADDR_EXPR)
fndecl = TREE_OPERAND (fndecl, 0);
if (TREE_CODE (fndecl) == FUNCTION_DECL)
{
if (setjmp_call_p (fndecl))
{
error_at (loc, "setjmps are not allowed inside loops marked with"
" #pragma simd");
*valid = false;
*walk_subtrees = 0;
}
}
This is already checked in the C counterpart. Can't you just call
find_invalid_stmts_in_loops() from find_invalid_stmts since it checks
for everything C/C++ related? Ideally your find_invalid_stmts()
function should only check the C++ specific stuff (throw, try, etc) and
call find_invalid_stmts_in_loops.
else if (TREE_CODE (*tp) == THROW_EXPR)
{
error_at (loc, "throw expressions are not allowed inside the loop "
"marked with pragma simd");
s/the loop/loops
else if (TREE_CODE (*tp) == TRY_BLOCK)
{
error_at (loc, "try statements are not allowed inside loop marked with "
s/loop/loops
bool
p_simd_valid_stmts_in_body_p (tree body)
Is there a reason for the p_ prefix? If not, could you remove it?
@@ -29458,6 +29472,7 @@ cp_parser_initial_pragma (cp_token *first_token)
cp_lexer_get_preprocessor_token (NULL, first_token);
}
+
/* Normal parsing of a pragma token. Here we can (and must) use the
regular lexer. */
Whitespace.
@@ -29605,6 +29620,11 @@ cp_parser_pragma (cp_parser *parser, enum
pragma_context context)
"%<#pragma omp sections%> construct");
break;
+ case PRAGMA_CILK_SIMD:
+ if (context == pragma_external)
+ goto bad_stmt;
+ cp_parser_cilk_simd_construct (parser, pragma_tok);
+ return true;
default:
Please add an empty line after the return, since the previous code had a
space there.
Can you put all the cp_parser_cilk_for* functions before
cp_parser_cilk_simd_vectorlength and avoid the prototype for
cp_parser_cilk_for? Just curious, I don't know if there are any
dependencies I'm not seeing.
+/* Main entry-point for parsing Cilk Plus <#pragma simd> for loop. */
s/loop/loops
+ /* #pragma simd is build on top of OpenMP 4.0's OMP_SIMD treees. Thus
+ openmp must be enabled. */
s/build/built
s/treees/trees
s/openmp/OpenMP
+/* Returns the name of the next clause. If the clause is not recognized, then
+ PRAGMA_CILK_CLAUSE_NONE is returned and the next token is not consumed.
+ Otherwise, the appropriate enum. value from the pragma_simd_clause is
+ returned and the token is consumed. */
+
+static pragma_cilk_clause
+cp_parser_cilk_simd_clause_name (cp_parser *parser, tree *name)
Replace "enum. value" with "enum".
Also, you are not documenting the NAME argument.
+ /* Vectorlength clause behaves exactly like Open MP 4.0's safelen clause.
s/Vectorlength/The vectorlength
Regarding the tests, I will merge trunk->cilk-in-gomp so we can reuse
the c-c++-common infrastructure you wrote. There is no sense having
multiple tests for vectorlength, linear, etc.
Aldy
