[openacc] tile, independent, default, private and firstprivate support in c/++

Jakub Jelinek jakub@redhat.com
Wed Nov 4 10:24:00 GMT 2015


On Tue, Nov 03, 2015 at 02:16:59PM -0800, Cesar Philippidis wrote:
> --- a/gcc/c-family/c-omp.c
> +++ b/gcc/c-family/c-omp.c
> @@ -709,12 +709,21 @@ c_oacc_split_loop_clauses (tree clauses, tree *not_loop_clauses)
>  
>        switch (OMP_CLAUSE_CODE (clauses))
>          {
> +	  /* Loop clauses.  */
>  	case OMP_CLAUSE_COLLAPSE:
> -	case OMP_CLAUSE_REDUCTION:
> +	case OMP_CLAUSE_TILE:
> +	case OMP_CLAUSE_GANG:
> +	case OMP_CLAUSE_WORKER:
> +	case OMP_CLAUSE_VECTOR:
> +	case OMP_CLAUSE_AUTO:
> +	case OMP_CLAUSE_SEQ:
> +	case OMP_CLAUSE_INDEPENDENT:
>  	  OMP_CLAUSE_CHAIN (clauses) = loop_clauses;
>  	  loop_clauses = clauses;
>  	  break;
>  
> +	  /* Parallel/kernels clauses.  */
> +

Why the extra empty line where you don't have it above COLLAPSE?
>  	default:
>  	  OMP_CLAUSE_CHAIN (clauses) = *not_loop_clauses;
>  	  *not_loop_clauses = clauses;

> @@ -10577,7 +10584,10 @@ c_parser_omp_clause_default (c_parser *parser, tree list)
>    else
>      {
>      invalid_kind:
> -      c_parser_error (parser, "expected %<none%> or %<shared%>");
> +      if (is_oacc)
> +	c_parser_error (parser, "expected %<none%>");
> +	else
> +	  c_parser_error (parser, "expected %<none%> or %<shared%>");

The indentation is wrong above (last two lines), doesn't -Wmisleading-indentation warn about
this?
>      }
>    c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>");
>  
> @@ -11376,6 +11386,83 @@ c_parser_oacc_clause_async (c_parser *parser, tree list)
>    return list;
>  }
>  
> +/* 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;
> +  vec<tree, va_gc> *tvec = make_tree_vector ();
> +
> +  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 %<(%>"))
> +    {
> +      release_tree_vector (tvec);
> +      return list;
> +    }

Better move tvec = make_tree_vector (); below this if and remove
release_tree_vector call?

> +
> +  do
> +    {
> +      if (c_parser_next_token_is (parser, CPP_MULT))
> +	{
> +	  c_parser_consume_token (parser);
> +	  expr = integer_minus_one_node;
> +	}
> +      else

Is this right?  If it is either * or (assignment) expression, then
I'd expect to parse only CPP_MULT followed by CPP_CLOSE_PAREN
or CPP_COMMA that way (C parser has 2 tokens look-ahead, so it should be
fine), so that
tile (a + b + c, *)
is parsed as
(a + b + c); -1
and so is
tile (*, a + b)
as
-1; (a + b)
while
tile (*a, *b)
is
*a; *b.

Guess the gang clause parsing that went into the trunk already has the
same bug,
gang (static: *)
or
gang (static: *, num: 5)
should be special, but
gang (static: *ptr)
should be
gang (static: (*ptr))

> +	{
> +	  expr_loc = c_parser_peek_token (parser)->location;
> +	  expr = c_parser_expr_no_commas (parser, NULL).value;
> +
> +	  if (expr == error_mark_node)
> +	    goto cleanup_error;
> +
> +	  if (!INTEGRAL_TYPE_P (TREE_TYPE (expr)))
> +	    {
> +	      c_parser_error (parser, "%<tile%> value must be integral");
> +	      return list;
> +	    }
> +
> +	  mark_exp_read (expr);
> +	  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)
> +	    {
> +	      warning_at (expr_loc, 0,"%<tile%> value must be positive");
> +	      expr = integer_one_node;
> +	    }
> +	}
> +
> +      vec_safe_push (tvec, expr);
> +      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);
> +  OMP_CLAUSE_TILE_LIST (c) = tile;
> +  OMP_CLAUSE_CHAIN (c) = list;
> +  release_tree_vector (tvec);
> +  return c;
> +
> + cleanup_error:
> +  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>");
> +  release_tree_vector (tvec);
> +  return list;
> +}
> +
>  /* OpenACC:
>     wait ( int-expr-list ) */
>  
> @@ -12576,6 +12663,10 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
>  	  clauses = c_parser_oacc_data_clause (parser, c_kind, clauses);
>  	  c_name = "delete";
>  	  break;
> +	case PRAGMA_OMP_CLAUSE_DEFAULT:
> +	  clauses = c_parser_omp_clause_default (parser, clauses, true);
> +	  c_name = "default";
> +	  break;
>  	case PRAGMA_OACC_CLAUSE_DEVICE:
>  	  clauses = c_parser_oacc_data_clause (parser, c_kind, clauses);
>  	  c_name = "device";
> @@ -12601,6 +12692,11 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
>  	  clauses = c_parser_omp_clause_if (parser, clauses, false);
>  	  c_name = "if";
>  	  break;
> +	case PRAGMA_OACC_CLAUSE_INDEPENDENT:
> +	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_INDEPENDENT,
> +						clauses);
> +	  c_name = "independent";
> +	  break;
>  	case PRAGMA_OACC_CLAUSE_NUM_GANGS:
>  	  clauses = c_parser_omp_clause_num_gangs (parser, clauses);
>  	  c_name = "num_gangs";
> @@ -12646,6 +12742,10 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
>  						clauses);
>  	  c_name = "seq";
>  	  break;
> +	case PRAGMA_OACC_CLAUSE_TILE:
> +	  clauses = c_parser_oacc_clause_tile (parser, clauses);
> +	  c_name = "tile";
> +	  break;
>  	case PRAGMA_OACC_CLAUSE_VECTOR:
>  	  c_name = "vector";
>  	  clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_VECTOR,
> @@ -12727,7 +12827,7 @@ c_parser_omp_all_clauses (c_parser *parser, omp_clause_mask mask,
>  	  c_name = "copyprivate";
>  	  break;
>  	case PRAGMA_OMP_CLAUSE_DEFAULT:
> -	  clauses = c_parser_omp_clause_default (parser, clauses);
> +	  clauses = c_parser_omp_clause_default (parser, clauses, false);
>  	  c_name = "default";
>  	  break;
>  	case PRAGMA_OMP_CLAUSE_FIRSTPRIVATE:
> @@ -13140,13 +13240,15 @@ c_parser_oacc_enter_exit_data (c_parser *parser, bool enter)
>  
>  #define OACC_LOOP_CLAUSE_MASK						\
>  	( (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COLLAPSE)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_PRIVATE)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_REDUCTION)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_GANG)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_WORKER)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_VECTOR)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_AUTO)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_INDEPENDENT) 	\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_SEQ)			\
> -	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_REDUCTION) )
> -
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_TILE) )
>  static tree
>  c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name,
>  		    omp_clause_mask mask, tree *cclauses)
> @@ -13191,6 +13293,7 @@ c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name,
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYIN)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYOUT)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_CREATE)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEFAULT)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEVICEPTR)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_IF)			\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_PRESENT)		\
> @@ -13206,8 +13309,11 @@ c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name,
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYIN)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYOUT)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_CREATE)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEFAULT)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEVICEPTR)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_IF)			\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_PRIVATE)		\
> +	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_FIRSTPRIVATE)	\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_NUM_GANGS)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_NUM_WORKERS)		\
>  	| (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_PRESENT)		\
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 2363b9b..24cedf3 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -12946,10 +12946,12 @@ c_finish_omp_clauses (tree clauses, bool is_omp, bool declare_simd)
>  	case OMP_CLAUSE_ASYNC:
>  	case OMP_CLAUSE_WAIT:
>  	case OMP_CLAUSE_AUTO:
> +	case OMP_CLAUSE_INDEPENDENT:
>  	case OMP_CLAUSE_SEQ:
>  	case OMP_CLAUSE_GANG:
>  	case OMP_CLAUSE_WORKER:
>  	case OMP_CLAUSE_VECTOR:
> +	case OMP_CLAUSE_TILE:
>  	  pc = &OMP_CLAUSE_CHAIN (c);
>  	  continue;
>  
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index a90bf3b..200bf0c 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -29120,6 +29120,8 @@ cp_parser_omp_clause_name (cp_parser *parser)
>  	case 'i':
>  	  if (!strcmp ("inbranch", p))
>  	    result = PRAGMA_OMP_CLAUSE_INBRANCH;
> +	  else if (!strcmp ("independent", p))
> +	    result = PRAGMA_OACC_CLAUSE_INDEPENDENT;
>  	  else if (!strcmp ("is_device_ptr", p))
>  	    result = PRAGMA_OMP_CLAUSE_IS_DEVICE_PTR;
>  	  break;
> @@ -29214,6 +29216,8 @@ cp_parser_omp_clause_name (cp_parser *parser)
>  	    result = PRAGMA_OMP_CLAUSE_THREAD_LIMIT;
>  	  else if (!strcmp ("threads", p))
>  	    result = PRAGMA_OMP_CLAUSE_THREADS;
> +	  else if (!strcmp ("tile", p))
> +	    result = PRAGMA_OACC_CLAUSE_TILE;
>  	  else if (!strcmp ("to", p))
>  	    result = PRAGMA_OMP_CLAUSE_TO;
>  	  break;
> @@ -29714,6 +29718,58 @@ cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind,
>    return list;
>  }
>  
> +/* OpenACC 2.0:
> +   tile ( size-expr-list ) */
> +
> +static tree
> +cp_parser_oacc_clause_tile (cp_parser *parser, tree list, location_t here)

For consistency, please use cp_parser *parser, location_t clause_loc, tree list
parameter order and naming.
> +{
> +  tree c, expr = error_mark_node;
> +  location_t loc;
> +  tree tile = NULL_TREE;
> +  vec<tree, va_gc> *tvec = make_tree_vector ();
> +
> +  check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile", here);
> +
> +  loc = cp_lexer_peek_token (parser->lexer)->location;
> +  if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
> +    goto cleanup_error;

See above.
> +
> +  do
> +    {
> +      if (cp_lexer_next_token_is (parser->lexer, CPP_MULT))
> +	{
> +	  cp_lexer_consume_token (parser->lexer);
> +	  expr = integer_minus_one_node;
> +	}

See above.
> +      else
> +	expr = cp_parser_assignment_expression (parser, NULL, false, false);
> +
> +      if (expr == error_mark_node)
> +	goto cleanup_error;
> +
> +      vec_safe_push (tvec, expr);
> +
> +      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));
> +
> +  /* Consume the trailing ')'.  */
> +  cp_lexer_consume_token (parser->lexer);
> +
> +  c = build_omp_clause (loc, OMP_CLAUSE_TILE);
> +  tile = build_tree_list_vec (tvec);
> +  OMP_CLAUSE_TILE_LIST (c) = tile;
> +  OMP_CLAUSE_CHAIN (c) = list;
> +  release_tree_vector (tvec);
> +  return c;
> +
> + cleanup_error:
> +  release_tree_vector (tvec);
> +  return list;
> +}
> +
>  /* OpenACC:
>     vector_length ( expression ) */
>  
> @@ -29860,10 +29916,14 @@ cp_parser_omp_clause_collapse (cp_parser *parser, tree list, location_t location
>  }
>  
>  /* OpenMP 2.5:
> -   default ( shared | none ) */
> +   default ( shared | none )
> +
> +   OpenACC 2.0
> +   default (none) */
>  
>  static tree
> -cp_parser_omp_clause_default (cp_parser *parser, tree list, location_t location)
> +cp_parser_omp_clause_default (cp_parser *parser, tree list,
> +			      location_t location, bool is_oacc)
>  {
>    enum omp_clause_default_kind kind = OMP_CLAUSE_DEFAULT_UNSPECIFIED;
>    tree c;
> @@ -29884,7 +29944,7 @@ cp_parser_omp_clause_default (cp_parser *parser, tree list, location_t location)
>  	  break;
>  
>  	case 's':
> -	  if (strcmp ("shared", p) != 0)
> +	  if (strcmp ("shared", p) != 0 || is_oacc)
>  	    goto invalid_kind;
>  	  kind = OMP_CLAUSE_DEFAULT_SHARED;
>  	  break;
> @@ -29898,7 +29958,10 @@ cp_parser_omp_clause_default (cp_parser *parser, tree list, location_t location)
>    else
>      {
>      invalid_kind:
> -      cp_parser_error (parser, "expected %<none%> or %<shared%>");
> +      if (is_oacc)
> +	cp_parser_error (parser, "expected %<none%>");
> +      else
> +	cp_parser_error (parser, "expected %<none%> or %<shared%>");
>      }
>  
>    if (!cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN))
> @@ -31467,6 +31530,10 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
>  	  clauses = cp_parser_oacc_data_clause (parser, c_kind, clauses);
>  	  c_name = "delete";
>  	  break;
> +	case PRAGMA_OMP_CLAUSE_DEFAULT:
> +	  clauses = cp_parser_omp_clause_default (parser, clauses, here, true);
> +	  c_name = "default";
> +	  break;
>  	case PRAGMA_OACC_CLAUSE_DEVICE:
>  	  clauses = cp_parser_oacc_data_clause (parser, c_kind, clauses);
>  	  c_name = "device";
> @@ -31475,6 +31542,11 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
>  	  clauses = cp_parser_oacc_data_clause_deviceptr (parser, clauses);
>  	  c_name = "deviceptr";
>  	  break;
> +	case PRAGMA_OACC_CLAUSE_FIRSTPRIVATE:
> +	  clauses = cp_parser_omp_var_list
> +	    (parser, OMP_CLAUSE_FIRSTPRIVATE, clauses);

Please put the ( on the same line as the fn call, either

	  clauses = cp_parser_omp_var_list (parser, OMP_CLAUSE_FIRSTPRIVATE,
					    clauses);

fits on the same number of lines.

> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index c73dcd0..14d006b 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -6704,9 +6704,51 @@ finish_omp_clauses (tree clauses, bool allow_fields, bool declare_simd)
>  	case OMP_CLAUSE_DEFAULTMAP:
>  	case OMP_CLAUSE__CILK_FOR_COUNT_:
>  	case OMP_CLAUSE_AUTO:
> +	case OMP_CLAUSE_INDEPENDENT:
>  	case OMP_CLAUSE_SEQ:
>  	  break;
>  
> +	case OMP_CLAUSE_TILE:
> +	  {
> +	    tree list = OMP_CLAUSE_TILE_LIST (c);
> +
> +	    while (list)
> +	      {
> +		t = TREE_VALUE (list);
> +
> +		if (t == error_mark_node)
> +		  remove = true;
> +		else if (!type_dependent_expression_p (t)
> +			 && !INTEGRAL_TYPE_P (TREE_TYPE (t)))
> +		  {
> +		    error ("%<tile%> value must be integral");
> +		    remove = true;
> +		  }
> +		else
> +		  {
> +		    t = mark_rvalue_use (t);
> +		    if (!processing_template_decl)
> +		      {
> +			t = maybe_constant_value (t);
> +			if (TREE_CODE (t) == INTEGER_CST
> +			    && tree_int_cst_sgn (t) != 1
> +			    && t != integer_minus_one_node)
> +			  {
> +			    warning_at (OMP_CLAUSE_LOCATION (c), 0,
> +					"%<tile%> value must be positive");
> +			    t = integer_one_node;
> +			  }
> +		      }
> +		    t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
> +		  }
> +
> +		/* Update list item.  */
> +		TREE_VALUE (list) = t;
> +		list = TREE_CHAIN (list);
> +	      }
> +	  }
> +	  break;

Have you verified pt.c does the right thing when instantiating the
OMP_CLAUSE_TILE clause (I mean primarily the TREE_VEC in there)?
There really should be testcases for that.

> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 03203c0..08b192d 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -6997,7 +6997,6 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
>  
>  	case OMP_CLAUSE_DEVICE_RESIDENT:
>  	case OMP_CLAUSE_USE_DEVICE:
> -	case OMP_CLAUSE_INDEPENDENT:
>  	  remove = true;
>  	  break;
>  
> @@ -7007,6 +7006,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
>  	case OMP_CLAUSE_COLLAPSE:
>  	case OMP_CLAUSE_AUTO:
>  	case OMP_CLAUSE_SEQ:
> +	case OMP_CLAUSE_INDEPENDENT:
>  	case OMP_CLAUSE_MERGEABLE:
>  	case OMP_CLAUSE_PROC_BIND:
>  	case OMP_CLAUSE_SAFELEN:
> @@ -7014,6 +7014,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
>  	case OMP_CLAUSE_NOGROUP:
>  	case OMP_CLAUSE_THREADS:
>  	case OMP_CLAUSE_SIMD:
> +	case OMP_CLAUSE_TILE:
>  	  break;

No gimplification of the expressions in the tile clause?

>  	case OMP_CLAUSE_DEFAULTMAP:
> @@ -7482,6 +7483,7 @@ gimplify_adjust_omp_clauses (gimple_seq *pre_p, tree *list_p,
>  	case OMP_CLAUSE_VECTOR:
>  	case OMP_CLAUSE_AUTO:
>  	case OMP_CLAUSE_SEQ:
> +	case OMP_CLAUSE_TILE:
>  	  break;
>  
>  	default:

	Jakub



More information about the Gcc-patches mailing list