Hi!
On Wed, Nov 04, 2015 at 09:55:49AM -0800, Cesar Philippidis wrote:
So, you are going to deal with gang parsing incrementally?
Fine with me.
> +/* OpenACC 2.0:
> + tile ( size-expr-list ) */
> +
> +static tree
> +c_parser_oacc_clause_tile (c_parser *parser, tree list)
> +{
> + tree c, expr = error_mark_node;
> + location_t loc, expr_loc;
> + tree tile = NULL_TREE;
> +
> + check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile");
> +
> + loc = c_parser_peek_token (parser)->location;
> + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
> + return list;
> +
> + vec<tree, va_gc> *tvec = make_tree_vector ();
Seems I've misread your patch, thought you are using TREE_VEC, while
you are actually using TREE_LIST, but populating it in a weird way.
I think more efficient would be just to
tree tile = NULL_TREE;
here, then:
> +
> + vec_safe_push (tvec, expr);
tile = tree_cons (NULL_TREE, expr, tile);
> + if (c_parser_next_token_is (parser, CPP_COMMA))
> + c_parser_consume_token (parser);
> + }
> + while (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN));
> +
> + /* Consume the trailing ')'. */
> + c_parser_consume_token (parser);
> +
> + c = build_omp_clause (loc, OMP_CLAUSE_TILE);
> + tile = build_tree_list_vec (tvec);
tile = nreverse (tile);
> + OMP_CLAUSE_TILE_LIST (c) = tile;
> + OMP_CLAUSE_CHAIN (c) = list;
> + release_tree_vector (tvec);
and remove the release_tree_vector calls.
> +static tree
> +cp_parser_oacc_clause_tile (cp_parser *parser, location_t clause_loc, tree
> list)
This is already too long line.
> + case OMP_CLAUSE_TILE:
> + {
> + tree list = OMP_CLAUSE_TILE_LIST (c);
> +
> + while (list)
I'd say
for (tree list = OMP_CLAUSE_TILE_LIST (c);
list; list = TREE_CHAIN (list))
would be more readable.
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -6995,9 +6995,18 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> remove = true;
> break;
>
> + case OMP_CLAUSE_TILE:
> + for (tree list = OMP_CLAUSE_TILE_LIST (c); !remove && list;
> + list = TREE_CHAIN (list))
> + {
> + if (gimplify_expr (&TREE_VALUE (list), pre_p, NULL,
> + is_gimple_val, fb_rvalue) == GS_ERROR)
> + remove = true;
> + }
After all, you are already using for here ;)
Otherwise LGTM, but please clear with Thomas, I think he had some issues
with the patch.
Jakub
