Hi Cesar!
On Wed, 5 Apr 2017 13:37:30 -0700, Cesar Philippidis <ce...@codesourcery.com>
wrote:
> On 04/05/2017 01:22 PM, Thomas Schwinge wrote:
>
> >> --- 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.
>
> Yes.
>
> >> 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.
So, is this an invalid scenario (and should thus get an "assert" or
"gcc_unreachable"), or is it correct, if "private", to not set
"on_device" and "GOVD_MAP_TO_ONLY" for "device_resident_p" DECLs?
> > | [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".
>
> Yes. Inside kernels regions, everything is pcopy, unless it's private.
But I'm saying/understanding the code so that inside kernels regions,
"private" stuff is "present_or_copy". Is that correct or not?
> Some private variables include, e.g., fortran array descriptors.
(I'd prefer if we had tree-dump scanning tests for such things.)
> >> 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?
>
> Yes. Is there something wrong with that behavior
Again: I'm confused as to why inside kernels regions, "private" stuff is
mapped "present_or_copy", but inside parallel regions, it's
"firstprivate".
> or is it just unclear?
In gomp-4_0-branch r246762, I committed the following patch, to make
better apparent the current behavior:
commit a87af0655eb2f803c330d807cdca698ab597b44e
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Fri Apr 7 14:55:56 2017 +0000
Clarify gcc/gimplify.c:oacc_default_clause
gcc/
* gimplify.c (oacc_default_clause): Clarify.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@246762
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/ChangeLog.gomp | 4 ++++
gcc/gimplify.c | 44 ++++++++++++++++++++++++++------------------
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git gcc/ChangeLog.gomp gcc/ChangeLog.gomp
index effcc05..811e190 100644
--- gcc/ChangeLog.gomp
+++ gcc/ChangeLog.gomp
@@ -1,3 +1,7 @@
+2017-04-07 Thomas Schwinge <tho...@codesourcery.com>
+
+ * gimplify.c (oacc_default_clause): Clarify.
+
2017-04-05 Cesar Philippidis <ce...@codesourcery.com>
* omp-low.c (scan_sharing_clauses): Update handling of OpenACC declare
diff --git gcc/gimplify.c gcc/gimplify.c
index 604a6cb..f2bb814 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -6129,30 +6129,38 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, tree
decl, unsigned flags)
switch (ctx->region_type)
{
- default:
- gcc_unreachable ();
-
case ORT_ACC_KERNELS:
- /* Scalars are default 'copy' under kernels, non-scalars are default
- 'present_or_copy'. */
- flags |= GOVD_MAP;
- if (!AGGREGATE_TYPE_P (type) && !is_private)
- flags |= GOVD_MAP_FORCE;
-
rkind = "kernels";
+
+ if (is_private)
+ flags |= GOVD_MAP;
+ else if (AGGREGATE_TYPE_P (type))
+ /* Aggregates default to 'present_or_copy'. */
+ flags |= GOVD_MAP;
+ else
+ /* Scalars default to 'copy'. */
+ flags |= GOVD_MAP | GOVD_MAP_FORCE;
+
break;
case ORT_ACC_PARALLEL:
- {
- if (!is_private && (on_device || AGGREGATE_TYPE_P (type) || declared))
- /* Aggregates default to 'present_or_copy'. */
- flags |= GOVD_MAP;
- else
- /* Scalars default to 'firstprivate'. */
- flags |= GOVD_FIRSTPRIVATE;
- rkind = "parallel";
- }
+ rkind = "parallel";
+
+ if (is_private)
+ flags |= GOVD_FIRSTPRIVATE;
+ else if (on_device || declared)
+ flags |= GOVD_MAP;
+ else if (AGGREGATE_TYPE_P (type))
+ /* Aggregates default to 'present_or_copy'. */
+ flags |= GOVD_MAP;
+ else
+ /* Scalars default to 'firstprivate'. */
+ flags |= GOVD_FIRSTPRIVATE;
+
break;
+
+ default:
+ gcc_unreachable ();
}
if (DECL_ARTIFICIAL (decl))
Grüße
Thomas
