Hi Cesar!
Can you please help me understand/verify a part of your patch:
On Thu, 15 Sep 2016 07:56:58 -0700, Cesar Philippidis <ce...@codesourcery.com>
wrote:
> Currently gfortran largely lacks support for fortran common blocks in
> OpenACC. The notable exception is acc declare link which does support
> common block arguments to some extent. This patch does two things:
>
> 1) Adds support for common blocks in the appropriate OpenACC data
> clauses.
>
> 2) Privatizes the underlying common block struct during gimplification.
> It also teaches the gimplifier to how to defer the expansion of
> DECL_VALUE_EXPR for common block decls until omp lowering.
>
> The first item allows allows common block names to be listed in data
> clauses. Such names need to be surrounded by slashes. E.g.
>
> common /BLOCK/ a, b, c
>
> !$acc enter data copyin(/BLOCK/)
>
> Note that common block names are treated in a similar manner to OpenMP
> common block arguments; gfc_match_omp_map_clauses expands the common
> block names to individual data clauses for each variable in the common
> block.
>
> The second item updates how common blocks behave on the accelerator.
> Using the BLOCK example from above, if an OpenACC offloading region only
> utilized, say, variable 'b', the gimplifier will now only transfer and
> remap 'b' on the accelerator. The actual common block struct will have a
> private clause. Without this patch, both the common block struct and the
> individual variable were transferred to the accelerator separately, and
> that would result in duplicate data mapping errors at runtime.
>
> The second item also defers the expansion of DECL_VALUE_EXPR because
> otherwise the privatized common block data would be used instead of one
> that was explicitly or implicitly transferred to the accelerator.
> gcc/
> * gimplify.c (oacc_default_clause): Privatize fortran common blocks.
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -6102,14 +6102,19 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx,
> tree decl, unsigned flags)
> {
> const char *rkind;
> bool on_device = false;
> + bool is_private = false;
So the intention here is that by default everything stays the same as
before; "is_private == false". This property is satisfied in the
following code.
> tree type = TREE_TYPE (decl);
>
> if (lang_hooks.decls.omp_privatize_by_reference (decl))
> type = TREE_TYPE (type);
>
> + if (RECORD_OR_UNION_TYPE_P (type))
> + is_private = lang_hooks.decls.omp_disregard_value_expr (decl, false);
> +
> if ((ctx->region_type & (ORT_ACC_PARALLEL | ORT_ACC_KERNELS)) != 0
> && is_global_var (decl)
> - && device_resident_p (decl))
> + && device_resident_p (decl)
> + && !is_private)
> {
> on_device = true;
> flags |= GOVD_MAP_TO_ONLY;
| }
For "is_private == true" we will not possibly enter this block.
| [ORT_ACC_KERNELS]
> /* Scalars are default 'copy' under kernels, non-scalars are default
> 'present_or_copy'. */
> flags |= GOVD_MAP;
> - if (!AGGREGATE_TYPE_P (type))
> + if (!AGGREGATE_TYPE_P (type) && !is_private)
> flags |= GOVD_MAP_FORCE;
For "is_private == true" we will not possibly enter this block, which
means in this case we will map both scalars and aggregates as
"present_or_copy".
> case ORT_ACC_PARALLEL:
> {
> - if (on_device || AGGREGATE_TYPE_P (type))
> + if (!is_private && (on_device || AGGREGATE_TYPE_P (type)))
> /* Aggregates default to 'present_or_copy'. */
> flags |= GOVD_MAP;
> else
| /* Scalars default to 'firstprivate'. */
| flags |= GOVD_FIRSTPRIVATE;
For "is_private == true" we will not possibly enter the "if" block, so we
will always enter the "else" block, which means in this case we will map
both scalars and aggregates as "firstprivate".
Is that all correct?
Grüße
Thomas
