> +static tree
> +c_check_cilk_loop_incr (location_t loc, tree decl, tree incr)
> +{
> + if (EXPR_HAS_LOCATION (incr))
> + loc = EXPR_LOCATION (incr);
> +
> + if (!incr)
> + {
> + error_at (loc, "missing increment");
> + return error_mark_node;
> + }
Either these tests are swapped, or the second one isn't really needed.
> + switch (TREE_CODE (incr))
> + {
> + case POSTINCREMENT_EXPR:
> + case PREINCREMENT_EXPR:
> + case POSTDECREMENT_EXPR:
> + case PREDECREMENT_EXPR:
> + if (TREE_OPERAND (incr, 0) != decl)
> + break;
> +
> + // Bah... canonicalize into whatever OMP_FOR_INCR needs.
> + if (POINTER_TYPE_P (TREE_TYPE (decl))
> + && TREE_OPERAND (incr, 1))
> + {
> + tree t = fold_convert_loc (loc,
> + sizetype, TREE_OPERAND (incr, 1));
> +
> + if (TREE_CODE (incr) == POSTDECREMENT_EXPR
> + || TREE_CODE (incr) == PREDECREMENT_EXPR)
> + t = fold_build1_loc (loc, NEGATE_EXPR, sizetype, t);
> + t = fold_build_pointer_plus (decl, t);
> + incr = build2 (MODIFY_EXPR, void_type_node, decl, t);
> + }
> + return incr;
Handling pointer types and pointer_plus_expr here (p++) ...
> + case MODIFY_EXPR:
> + {
> + tree rhs;
> +
> + if (TREE_OPERAND (incr, 0) != decl)
> + break;
> +
> + rhs = TREE_OPERAND (incr, 1);
> + if (TREE_CODE (rhs) == PLUS_EXPR
> + && (TREE_OPERAND (rhs, 0) == decl
> + || TREE_OPERAND (rhs, 1) == decl)
> + && INTEGRAL_TYPE_P (TREE_TYPE (rhs)))
> + return incr;
> + else if (TREE_CODE (rhs) == MINUS_EXPR
> + && TREE_OPERAND (rhs, 0) == decl
> + && INTEGRAL_TYPE_P (TREE_TYPE (rhs)))
> + return incr;
> + // Otherwise fail because only PLUS_EXPR and MINUS_EXPR are
> + // allowed.
> + break;
... but not here (p += 1)?
> +c_validate_cilk_plus_loop (tree *tp, int *walk_subtrees, void *data)
> +{
> + if (!tp || !*tp)
> + return NULL_TREE;
> +
> + bool *valid = (bool *) data;
> +
> + switch (TREE_CODE (*tp))
> + {
> + case 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 (EXPR_LOCATION (*tp),
> + "calls to setjmp are not allowed within loops "
> + "annotated with #pragma simd");
> + *valid = false;
> + *walk_subtrees = 0;
> + }
> + }
> + break;
Why bother checking for setjmp? While I agree it makes little sense, there are
plenty of other standard functions which also make no sense to use from within
#pragma simd. What's likely to go wrong with a call to setjmp, as opposed to
getcontext, pthread_create, or even printf?
> + if (DECL_REGISTER (decl))
> + {
> + error_at (loc, "induction variable cannot be declared register");
> + return false;
> + }
Why?
All of the actual gimple changes look good. You could commit those now if you
like to reduce the size of the next patch.
r~
