On 2016/11/18 7:23 PM, Jakub Jelinek wrote:
> On Thu, Nov 17, 2016 at 05:34:34PM +0800, Chung-Lin Tang wrote:
>> Updated C/C++ front-end patches, adjusted as reviewed.
>
> Jason is right, finish_omp_clauses will verify the tile operands
> when !processing_template_decl are non-negative host INTEGER_CSTs,
> so can't you just tsubst it like OMP_CLAUSE_COLLAPSE? If the operand
> is not a constant expression, presumably it will not be INTEGER_CST.
Yeah, it appears that way will work. Updated C/C++ FE patch as attached.
> On the other side, OMP_CLAUSE_TILE has now 3 operands instead of just 1,
> don't you need to do something during instantiation for the other 2
> operands?
>
> Jakub
The other two operands are used only in omp-low, they're not programmer
defined operands.
Thanks,
Chung-Lin
Index: c/c-parser.c
===================================================================
--- c/c-parser.c (revision 241809)
+++ c/c-parser.c (working copy)
@@ -11010,6 +11010,7 @@ c_parser_omp_clause_collapse (c_parser *parser, tr
location_t loc;
check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse");
+ 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 %<(%>"))
@@ -11920,10 +11921,11 @@ static tree
c_parser_oacc_clause_tile (c_parser *parser, tree list)
{
tree c, expr = error_mark_node;
- location_t loc, expr_loc;
+ location_t loc;
tree tile = NULL_TREE;
check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile");
+ check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse");
loc = c_parser_peek_token (parser)->location;
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
@@ -11931,16 +11933,19 @@ c_parser_oacc_clause_tile (c_parser *parser, tree
do
{
+ if (tile && !c_parser_require (parser, CPP_COMMA, "expected %<,%>"))
+ return list;
+
if (c_parser_next_token_is (parser, CPP_MULT)
&& (c_parser_peek_2nd_token (parser)->type == CPP_COMMA
|| c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_PAREN))
{
c_parser_consume_token (parser);
- expr = integer_minus_one_node;
+ expr = integer_zero_node;
}
else
{
- expr_loc = c_parser_peek_token (parser)->location;
+ location_t expr_loc = c_parser_peek_token (parser)->location;
c_expr cexpr = c_parser_expr_no_commas (parser, NULL);
cexpr = convert_lvalue_to_rvalue (expr_loc, cexpr, false, true);
expr = cexpr.value;
@@ -11952,28 +11957,19 @@ c_parser_oacc_clause_tile (c_parser *parser, tree
return list;
}
- if (!INTEGRAL_TYPE_P (TREE_TYPE (expr)))
- {
- c_parser_error (parser, "%<tile%> value must be integral");
- return list;
- }
-
expr = c_fully_fold (expr, false, NULL);
- /* Attempt to statically determine when expr isn't positive. */
- c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, expr,
- build_int_cst (TREE_TYPE (expr), 0));
- protected_set_expr_location (c, expr_loc);
- if (c == boolean_true_node)
+ if (!INTEGRAL_TYPE_P (TREE_TYPE (expr))
+ || !tree_fits_shwi_p (expr)
+ || tree_to_shwi (expr) <= 0)
{
- warning_at (expr_loc, 0,"%<tile%> value must be positive");
- expr = integer_one_node;
+ error_at (expr_loc, "%<tile%> argument needs positive"
+ " integral constant");
+ expr = integer_zero_node;
}
}
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));
@@ -14899,11 +14895,17 @@ c_parser_omp_for_loop (location_t loc, c_parser *p
bool fail = false, open_brace_parsed = false;
int i, collapse = 1, ordered = 0, count, nbraces = 0;
location_t for_loc;
+ bool tiling = false;
vec<tree, va_gc> *for_block = make_tree_vector ();
for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
+ else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_TILE)
+ {
+ tiling = true;
+ collapse = list_length (OMP_CLAUSE_TILE_LIST (cl));
+ }
else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_ORDERED
&& OMP_CLAUSE_ORDERED_EXPR (cl))
{
@@ -14933,7 +14935,7 @@ c_parser_omp_for_loop (location_t loc, c_parser *p
pc = &OMP_CLAUSE_CHAIN (*pc);
}
- gcc_assert (collapse >= 1 && ordered >= 0);
+ gcc_assert (tiling || (collapse >= 1 && ordered >= 0));
count = ordered ? ordered : collapse;
declv = make_tree_vec (count);
Index: cp/pt.c
===================================================================
--- cp/pt.c (revision 241809)
+++ cp/pt.c (working copy)
@@ -14742,6 +14742,7 @@ tsubst_omp_clauses (tree clauses, enum c_omp_regio
= tsubst_omp_clause_decl (OMP_CLAUSE_DECL (oc), args, complain,
in_decl);
break;
+ case OMP_CLAUSE_TILE:
case OMP_CLAUSE_IF:
case OMP_CLAUSE_NUM_THREADS:
case OMP_CLAUSE_SCHEDULE:
@@ -14836,19 +14837,6 @@ tsubst_omp_clauses (tree clauses, enum c_omp_regio
case OMP_CLAUSE_AUTO:
case OMP_CLAUSE_SEQ:
break;
- case OMP_CLAUSE_TILE:
- {
- tree lnc, loc;
- for (lnc = OMP_CLAUSE_TILE_LIST (nc),
- loc = OMP_CLAUSE_TILE_LIST (oc);
- loc;
- loc = TREE_CHAIN (loc), lnc = TREE_CHAIN (lnc))
- {
- TREE_VALUE (lnc) = tsubst_expr (TREE_VALUE (loc), args,
- complain, in_decl, false);
- }
- }
- break;
default:
gcc_unreachable ();
}
Index: cp/parser.c
===================================================================
--- cp/parser.c (revision 241809)
+++ cp/parser.c (working copy)
@@ -30885,30 +30885,33 @@ cp_parser_oacc_clause_tile (cp_parser *parser, loc
tree c, expr = error_mark_node;
tree tile = NULL_TREE;
+ /* Collapse and tile are mutually exclusive. (The spec doesn't say
+ so, but the spec authors never considered such a case and have
+ differing opinions on what it might mean, including 'not
+ allowed'.) */
check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile", clause_loc);
+ check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse",
+ clause_loc);
if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
return list;
do
{
+ if (tile && !cp_parser_require (parser, CPP_COMMA, RT_COMMA))
+ return list;
+
if (cp_lexer_next_token_is (parser->lexer, CPP_MULT)
&& (cp_lexer_nth_token_is (parser->lexer, 2, CPP_COMMA)
|| cp_lexer_nth_token_is (parser->lexer, 2, CPP_CLOSE_PAREN)))
{
cp_lexer_consume_token (parser->lexer);
- expr = integer_minus_one_node;
+ expr = integer_zero_node;
}
else
- expr = cp_parser_assignment_expression (parser, NULL, false, false);
+ expr = cp_parser_constant_expression (parser);
- if (expr == error_mark_node)
- return list;
-
tile = tree_cons (NULL_TREE, expr, tile);
-
- if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA))
- cp_lexer_consume_token (parser->lexer);
}
while (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN));
@@ -31021,6 +31024,7 @@ cp_parser_omp_clause_collapse (cp_parser *parser,
}
check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse", location);
+ check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile", location);
c = build_omp_clause (loc, OMP_CLAUSE_COLLAPSE);
OMP_CLAUSE_CHAIN (c) = list;
OMP_CLAUSE_COLLAPSE_EXPR (c) = num;
@@ -34027,10 +34031,16 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr
int i, collapse = 1, ordered = 0, count, nbraces = 0;
vec<tree, va_gc> *for_block = make_tree_vector ();
auto_vec<tree, 4> orig_inits;
+ bool tiling = false;
for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
+ else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_TILE)
+ {
+ tiling = true;
+ collapse = list_length (OMP_CLAUSE_TILE_LIST (cl));
+ }
else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_ORDERED
&& OMP_CLAUSE_ORDERED_EXPR (cl))
{
@@ -34060,7 +34070,7 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr
pc = &OMP_CLAUSE_CHAIN (*pc);
}
- gcc_assert (collapse >= 1 && ordered >= 0);
+ gcc_assert (tiling || (collapse >= 1 && ordered >= 0));
count = ordered ? ordered : collapse;
declv = make_tree_vec (count);
@@ -34079,13 +34089,15 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr
if (code != CILK_FOR
&& !cp_lexer_next_token_is_keyword (parser->lexer, RID_FOR))
{
- cp_parser_error (parser, "for statement expected");
+ if (!collapse_err)
+ cp_parser_error (parser, "for statement expected");
return NULL;
}
if (code == CILK_FOR
&& !cp_lexer_next_token_is_keyword (parser->lexer, RID_CILK_FOR))
{
- cp_parser_error (parser, "_Cilk_for statement expected");
+ if (!collapse_err)
+ cp_parser_error (parser, "_Cilk_for statement expected");
return NULL;
}
loc = cp_lexer_consume_token (parser->lexer)->location;
@@ -34245,7 +34257,7 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr
nested. Hopefully the final version clarifies this.
For now handle (multiple) {'s and empty statements. */
cp_parser_parse_tentatively (parser);
- do
+ for (;;)
{
if (cp_lexer_next_token_is_keyword (parser->lexer, RID_FOR))
break;
@@ -34260,14 +34272,13 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tr
else
{
loc = cp_lexer_peek_token (parser->lexer)->location;
- error_at (loc, "not enough collapsed for loops");
+ error_at (loc, "not enough for loops to collapse");
collapse_err = true;
cp_parser_abort_tentative_parse (parser);
declv = NULL_TREE;
break;
}
}
- while (1);
if (declv)
{
Index: cp/semantics.c
===================================================================
--- cp/semantics.c (revision 241809)
+++ cp/semantics.c (working copy)
@@ -7086,7 +7086,8 @@ finish_omp_clauses (tree clauses, enum c_omp_regio
else if (!type_dependent_expression_p (t)
&& !INTEGRAL_TYPE_P (TREE_TYPE (t)))
{
- error ("%<tile%> value must be integral");
+ error_at (OMP_CLAUSE_LOCATION (c),
+ "%<tile%> argument needs integral type");
remove = true;
}
else
@@ -7094,14 +7095,16 @@ finish_omp_clauses (tree clauses, enum c_omp_regio
t = mark_rvalue_use (t);
if (!processing_template_decl)
{
+ /* Zero is used to indicate '*', we permit you
+ to get there via an ICE of value zero. */
t = maybe_constant_value (t);
- if (TREE_CODE (t) == INTEGER_CST
- && tree_int_cst_sgn (t) != 1
- && t != integer_minus_one_node)
+ if (!tree_fits_shwi_p (t)
+ || tree_to_shwi (t) < 0)
{
- warning_at (OMP_CLAUSE_LOCATION (c), 0,
- "%<tile%> value must be positive");
- t = integer_one_node;
+ error_at (OMP_CLAUSE_LOCATION (c),
+ "%<tile%> argument needs positive "
+ "integral constant");
+ remove = true;
}
}
t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
@@ -8000,11 +8003,19 @@ finish_omp_for (location_t locus, enum tree_code c
gcc_assert (TREE_VEC_LENGTH (declv) == TREE_VEC_LENGTH (incrv));
if (TREE_VEC_LENGTH (declv) > 1)
{
- tree c = find_omp_clause (clauses, OMP_CLAUSE_COLLAPSE);
+ tree c;
+
+ c = find_omp_clause (clauses, OMP_CLAUSE_TILE);
if (c)
- collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));
- if (collapse != TREE_VEC_LENGTH (declv))
- ordered = TREE_VEC_LENGTH (declv);
+ collapse = list_length (OMP_CLAUSE_TILE_LIST (c));
+ else
+ {
+ c = find_omp_clause (clauses, OMP_CLAUSE_COLLAPSE);
+ if (c)
+ collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));
+ if (collapse != TREE_VEC_LENGTH (declv))
+ ordered = TREE_VEC_LENGTH (declv);
+ }
}
for (i = 0; i < TREE_VEC_LENGTH (declv); i++)
{
