Re: [PING^3][PATCH, 12/16] Handle acc loop directive

[Tom de Vries <Tom_deVries at mentor dot com>]

On 26/01/16 13:49, Jakub Jelinek wrote:
> On Tue, Jan 26, 2016 at 01:38:39PM +0100, Tom de Vries wrote:
> > Ping^3. ( https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01089.html )
>
> First of all, I wonder if it wouldn't be far easier to handle these during
> gimplification rather than during omp expansion or during parsing.  Inside
> kernels, do you need to honor any clauses on the acc loop, like
> privatization etc., or can you just ignore it altogether (after parsing them
> to ensure they are valid)?

The oacc loop clauses are: gang, worker, vector, seq, auto, tile, 
device_type, independent, private, reduction.

AFAIU, there're all safe to ignore. That has largely been the approach 
in the gomp-4_0-branch, and sofar I haven't seen any failures due to 
ignoring a loop clause in a kernels region.

But we do want to be able to honor loop clauses in a kernels region at 
some point. F.i., supporting the independent clause would allow more 
test-cases to be parallelized.

At some point we had an implementation of the independent clause in the 
gomp-4_0-branch, but that had to be reverted ( 
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00696.html ).

Anyway, the implementation of the propagation of the independent 
property was to keep the loop directive with the independent clause 
until omp-expand (where we have cfg), and set a new field 
marked_independent in the corresponding struct loop.

If we want to do the expansion of the loop directive to a normal loop at 
gimplication, I see two issues:
- in general, we don't only check for correctness during parsing,
  there's also checking being done during scan_omp, which happens in
  pass_lower_omp, after gimplification.
- how do we mark the new loop as being independent?

> Handling this in expand_omp_for_generic is not really nice, because it will
> make already very complicated function even more complex.

An alternative would be to copy expand_omp_for_generic, apply the patch, 
and partially evaluate for the single call introduced in the patch.

Do you prefer this approach?

Thanks,
- Tom

>     gomp_ordered *ord_stmt;
> +
> +  /* True if this is nested inside an OpenACC kernels construct.  */
> +  bool inside_kernels_p;
>   };
>
> is bad placement, there are other bool/unsigned char fields earlier and the
> smaller fields should be adjacent for smaller padding of the struct.
>
> 	Jakub