Re: [patch] cleanup *finish_omp_clauses

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Apr 27, 2016 at 07:37:17PM -0700, Cesar Philippidis wrote:
> This patch replaces all of the bool argument to c_finish_omp_clauses and
> finish_omp_clauses in the c and c++ front ends, respectively. Right now
> there are three bool arguments, one for is_omp/allow_fields,
> declare_simd and is_cilk, the latter two have default values set.
> OpenACC will require some special handling in *finish_omp_clauses in the
> near future, too, so rather than add an is_oacc argument, I introduced
> an enum c_omp_region_type, similar to the one in gimplify.c.
> 
> Is this patch ok for trunk? I'll make use of C_ORT_ACC shortly in a
> follow up patch.

I've been long wanting to use just tree_code there, but as we don't have one
e.g. for DECLARE_SIMD, perhaps a separate enum is better.

> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1261,6 +1261,17 @@ enum c_omp_clause_split
>    C_OMP_CLAUSE_SPLIT_TASKLOOP = C_OMP_CLAUSE_SPLIT_FOR
>  };
>  
> +enum c_omp_region_type
> +{
> +  C_ORT_NONE		= 0,
> +  C_ORT_OMP		= 1 << 0,
> +  C_ORT_SIMD		= 1 << 1,
> +  C_ORT_CILK		= 1 << 2,
> +  C_ORT_ACC		= 1 << 3,
> +  C_ORT_OMP_SIMD	= C_ORT_OMP | C_ORT_SIMD,
> +  C_ORT_OMP_CILK	= C_ORT_OMP | C_ORT_CILK

That said, the above names are just weird, it is non-obvious
what they mean at all.  What is C_ORT_NONE for?  We surely don't
have any clauses that aren't OpenMP, nor Cilk+, nor OpenACC
(ok, maybe the simd attribute, but donno if it ever calls the
*finish_omp_clauses functions).
So, IMHO the originating specification should be one thing, so
C_ORT_OMP, C_ORT_CILK, C_ORT_ACC.
And, beyond that the C FE cares about whether it is a clause
on #pragma omp declare simd or its Cilk+ counterpart (vector attribute),
so you want C_ORT_DECLARE_SIMD possibly ored with the language
(note, not C_ORT_SIMD, that is way too confusing - we have
#pragma omp simd (OpenMP), #pragma simd (Cilk+), and we certainly do not
want the declare simd behavior for those.
Perhaps #pragma omp declare target is another construct that is handled
differently and could be visible in the bitmask too.

The C++ finish_omp_clauses also cares about whether fields (meaning
non-static members) are allowed, i.e. whether
struct S { int p; void foo () {
...
#pragma omp ... private (p)
...
}};
should be allowed or not.  That can be derived from the language and
the other construct bits though, I believe right now only OpenMP constructs
should handle it, and declare simd should not, and similarly declare target
should not.

	Jakub