[PATCH][committed] Warn about inconsistent OpenACC nested reduction clauses

[Jakub Jelinek]

On Wed, Nov 06, 2019 at 01:41:47PM +0100, frederik@codesourcery.com wrote:
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -128,6 +128,12 @@ struct omp_context
>       corresponding tracking loop iteration variables.  */
>    hash_map<tree, tree> *lastprivate_conditional_map;
>  
> +  /* A tree_list of the reduction clauses in this context.  */
> +  tree local_reduction_clauses;
> +
> +  /* A tree_list of the reduction clauses in outer contexts.  */
> +  tree outer_reduction_clauses;

Could there be acc in the name to make it clear it is OpenACC only?

>    /* Nesting depth of this context.  Used to beautify error messages re
>       invalid gotos.  The outermost ctx is depth 1, with depth 0 being
>       reserved for the main body of the function.  */
> @@ -910,6 +916,8 @@ new_omp_context (gimple *stmt, omp_context *outer_ctx)
>        ctx->outer = outer_ctx;
>        ctx->cb = outer_ctx->cb;
>        ctx->cb.block = NULL;
> +      ctx->local_reduction_clauses = NULL;
> +      ctx->outer_reduction_clauses = ctx->outer_reduction_clauses;
>        ctx->depth = outer_ctx->depth + 1;
>      }
>    else
> @@ -925,6 +933,8 @@ new_omp_context (gimple *stmt, omp_context *outer_ctx)
>        ctx->cb.transform_call_graph_edges = CB_CGE_MOVE;
>        ctx->cb.adjust_array_error_bounds = true;
>        ctx->cb.dont_remap_vla_if_no_change = true;
> +      ctx->local_reduction_clauses = NULL;
> +      ctx->outer_reduction_clauses = NULL;

The = NULL assignments are unnecessary in all 3 cases, ctx is allocated with
XCNEW.
>        ctx->depth = 1;
>      }
>  
> @@ -1139,6 +1149,11 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
>  	  goto do_private;
>  
>  	case OMP_CLAUSE_REDUCTION:
> +	  if (is_oacc_parallel (ctx) || is_oacc_kernels (ctx))
> +	    ctx->local_reduction_clauses
> +	      = tree_cons (NULL, c, ctx->local_reduction_clauses);

I'm not sure it is a good idea to use a TREE_LIST in this case, vec would be
more natural, wouldn't it.
Or, wouldn't it be better to do this checking in the gimplifier instead of
omp-low.c?  There we have splay trees with GOVD_REDUCTION etc. for the
variables, so it wouldn't be O(#reductions^2) compile time?
It is true that the gimplifier doesn't record the reduction codes (after
all, OpenMP has UDRs and so there can be fairly arbitrary reductions).
Consider million reduction clauses on nested loops.
If gimplifier is not the right spot, then use a splay tree + vector instead?
splay tree for the outer ones, vector for the local ones, and put into both
the clauses, so you can compare reduction code etc.

	Jakub