Re: [OpenACC] declare directive

[James Norris <jnorris@codesourcery.com>]

Jakub

On 11/12/2015 03:09 AM, Jakub Jelinek wrote:
> On Wed, Nov 11, 2015 at 07:07:58PM -0600, James Norris wrote:
>> +		  oacc_declare_returns->remove (t);
>> +
>> +		  if (oacc_declare_returns->elements () == 0)
>> +		    {
>> +		      delete oacc_declare_returns;
>> +		      oacc_declare_returns = NULL;
>> +		    }
>
> Something for incremental patch:
> 1) might be nice to have some assertion that at the end of gimplify_body
>     or so oacc_declare_returns is NULL
> 2) what happens if you refer to automatic variables of other functions
>     (C or Fortran nested functions, maybe C++ lambdas); shall those be
>     unmapped at the end of the (nested) function's body?
>

Ok. Thanks! Will put on my TODO list.

>> @@ -5858,6 +5910,10 @@ omp_default_clause (struct gimplify_omp_ctx *ctx, tree decl,
>>         flags |= GOVD_FIRSTPRIVATE;
>>         break;
>>       case OMP_CLAUSE_DEFAULT_UNSPECIFIED:
>> +      if (is_global_var (decl)
>> +	  && ctx->region_type & (ORT_ACC_PARALLEL | ORT_ACC_KERNELS)
>
> Please put this condition as cheapest first.  I'd also surround
> it into (), just to make it clear that the bitwise & is intentional.
> Perhaps () != 0.
>
>> +	  && device_resident_p (decl))
>> +	flags |= GOVD_MAP_TO_ONLY | GOVD_MAP;
>
>> +	  case GOMP_MAP_FROM:
>> +	    kinds[i] = GOMP_MAP_FORCE_FROM;
>> +	    GOACC_enter_exit_data (device, 1, &hostaddrs[i], &sizes[i],
>> +				       &kinds[i], 0, 0);
>
> Wrong indentation.
>

Fixed.

> Ok with those two changes and please think about the incremental stuff.

Again, thanks for taking the time for the review.

Jim