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

On 11/04/2015 02:24 AM, Jakub Jelinek wrote:
> On Tue, Nov 03, 2015 at 02:16:59PM -0800, Cesar Philippidis wrote:

>> +
>> +  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))

Thanks for catching that. I'll fix the tile and shape scanners in both
front ends.

>> 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.

I don't think we have any support for templates in trunk yet. Should I
add it to this patch, or should I address that in a follow up patch?

By the way, template support for num_gangs, num_worker and vector_length
depend on this patch
<https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03455.html> because the
c++ front end is currently incorrectly trying to do type checking as it
scans those clauses.

>> 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?

Ultimately we're just ignoring it, but for completeness' sake I fixed this.

I'm still testing this patch, but something like this OK for trunk?

cesar

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

Commit Message

Comments

Patch