On 09/09/13 07:54, Aldy Hernandez wrote:
PING^3
Hi guys!
PING for both C and C++.
Thanks.
-------- Original Message --------
Subject: Re: PING: Fwd: Re: [patch] implement Cilk Plus simd loops on trunk
Date: Tue, 27 Aug 2013 15:03:26 -0500
From: Aldy Hernandez <al...@redhat.com>
To: Richard Henderson <r...@redhat.com>
CC: jason merrill <ja...@redhat.com>, gcc-patches
<gcc-patches@gcc.gnu.org>
On 08/26/13 12:22, Richard Henderson wrote:
+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.
Swapped. Fixed.
+ 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)?
I should make the code more obvious. What I'm trying to do is generate
what the gimplifier for OMP_FOR is expecting. OMP rewrites pointer
increment/decrement expressions into a corresponding MODIFY_EXPR.
I have abstracted the OMP code and shared it between both type checks,
and I have also added an assert in the gimplifier just in case some
future front-end extension generates OMP_FOR_INCR in a wonky way.
+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?
Sigh...the standard specifically disallows setjmp.
+ if (DECL_REGISTER (decl))
+ {
+ error_at (loc, "induction variable cannot be declared register");
+ return false;
+ }
Why?
The standard :(.
All of the actual gimple changes look good. You could commit those
now if you
like to reduce the size of the next patch.
Ughh...got lazy on this round. How about I commit the gimple changes
for the next round?
How does this look?
