[v3] OpenACC 2.7: default clause support for data constructs (was: [PATCH, OpenACC 2.7, v2] Implement default clause support for data constructs)
Hi!
On 2023-08-01T23:35:16+0800, Chung-Lin Tang wrote:
> this is v2 of the patch for implementing the OpenACC 2.7 addition of
> default(none|present) support for data constructs.
Thanks!
> Instead of propagating an additional 'oacc_default_kind' for OpenACC,
> this patch does it in a more complete way: it directly propagates the
> gimplify_omp_ctx* pointer of the inner most context where we found
> a default-clause.
Right -- but reviewing this, it came upon me that we don't need any such
new code at all, and instead may in 'gcc/gimplify.cc:oacc_default_clause'
simply look through the 'ctx's to find the 'default' clause information.
This centralizes the logic in the one place where it's relevant.
> This supports displaying the location/type of OpenACC
> construct where the default-clause is in the error messages.
This is preserved...
> The testcases also have the multiple nested data construct testing added,
> where we can now have messages referring precisely to the exact innermost
> default clause that was active at that program point.
..., but we should also still 'inform' about the compute construct, where
the user is expected to add explicit data clauses (if not adding to the
'data' construct where the 'default(none)' clause appears):
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -7785,16 +7809,20 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx,
> tree decl, unsigned flags)
> - else if (ctx->default_kind == OMP_CLAUSE_DEFAULT_NONE)
> + else if (default_kind == OMP_CLAUSE_DEFAULT_NONE)
> {
>error ("%qE not specified in enclosing OpenACC %qs construct",
> - DECL_NAME (lang_hooks.decls.omp_report_decl (decl)), rkind);
> - inform (ctx->location, "enclosing OpenACC %qs construct", rkind);
> -}
> - else if (ctx->default_kind == OMP_CLAUSE_DEFAULT_PRESENT)
> + DECL_NAME (lang_hooks.decls.omp_report_decl (decl)),
> + oacc_region_type_name (ctx->region_type));
> + inform (ctx->oacc_default_clause_ctx->location,
> + "enclosing OpenACC %qs construct",
> + oacc_region_type_name
> + (ctx->oacc_default_clause_ctx->region_type));
> +}
That is, we should keep here the original 'inform' for 'ctx->location',
and *add another* 'inform' for 'ctx->oacc_default_clause_ctx->location'.
Otherwise that's confusing to users.
Instead of requiring another iteration through you, I've now implemented
that, and with test cases enhanced some more, pushed to master branch
commit bed993884b149851fe930b43cf11cbcdf05f1578
"OpenACC 2.7: default clause support for data constructs", see attached.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From bed993884b149851fe930b43cf11cbcdf05f1578 Mon Sep 17 00:00:00 2001
From: Chung-Lin Tang
Date: Tue, 6 Jun 2023 03:46:29 -0700
Subject: [PATCH] OpenACC 2.7: default clause support for data constructs
This patch implements the OpenACC 2.7 addition of default(none|present) support
for data constructs.
Now, specifying "default(none|present)" on a data construct turns on same
default clause behavior for all lexically enclosed compute constructs (which
don't already themselves have a default clause).
gcc/c/ChangeLog:
* c-parser.cc (OACC_DATA_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_DEFAULT.
gcc/cp/ChangeLog:
* parser.cc (OACC_DATA_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_DEFAULT.
gcc/fortran/ChangeLog:
* openmp.cc (OACC_DATA_CLAUSES): Add OMP_CLAUSE_DEFAULT.
gcc/ChangeLog:
* gimplify.cc (oacc_region_type_name): New function.
(oacc_default_clause): If no 'default' clause appears on this
compute construct, see if one appears on a lexically containing
'data' construct.
(gimplify_scan_omp_clauses): Upon OMP_CLAUSE_DEFAULT case, set
ctx->oacc_default_clause_ctx to current context.
gcc/testsuite/ChangeLog:
* c-c++-common/goacc/default-3.c: Adjust testcase.
* c-c++-common/goacc/default-4.c: Adjust testcase.
* c-c++-common/goacc/default-5.c: Adjust testcase.
* gfortran.dg/goacc/default-3.f95: Adjust testcase.
* gfortran.dg/goacc/default-4.f: Adjust testcase.
* gfortran.dg/goacc/default-5.f: Adjust testcase.
Co-authored-by: Thomas Schwinge
---
gcc/c/c-parser.cc | 1 +
gcc/cp/parser.cc | 1 +
gcc/fortran/openmp.cc | 3 +-
gcc/gimplify.cc | 64 +++
gcc/testsuite/c-c++-common/goacc/default-3.c | 59 +-
gcc/testsuite/c-c++-common/goacc/default-4.c | 42 ++
gcc/testsuite/c-c++-common/goacc/default-5.c | 19 -
gcc/testsuite/gfortran.dg/goacc/default-3.f95 | 77 ++-
gcc/testsuite/gfortran.dg/goacc/default-4.f | 36 +
gcc/testsuite/gfortran.dg/goacc/default-5.f | 19 -
10
[PATCH, OpenACC 2.7, v2] Implement default clause support for data constructs
Hi Thomas, this is v2 of the patch for implementing the OpenACC 2.7 addition of default(none|present) support for data constructs. Instead of propagating an additional 'oacc_default_kind' for OpenACC, this patch does it in a more complete way: it directly propagates the gimplify_omp_ctx* pointer of the inner most context where we found a default-clause. This supports displaying the location/type of OpenACC construct where the default-clause is in the error messages. The testcases also have the multiple nested data construct testing added, where we can now have messages referring precisely to the exact innermost default clause that was active at that program point. Note, I got rid of the dummy OMP_CLAUSE_DEFAULT creation in this version, since it seemed not really needed. Re-tested on master on powerpc64le-linux/nvptx. Okay to commit? Thanks, Chung-Lin 2023-08-01 Chung-Lin Tang gcc/c/ChangeLog: * c-parser.cc (OACC_DATA_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_DEFAULT. gcc/cp/ChangeLog: * parser.cc (OACC_DATA_CLAUSE_MASK): Add PRAGMA_OACC_CLAUSE_DEFAULT. gcc/fortran/ChangeLog: * openmp.cc (OACC_DATA_CLAUSES): Add OMP_CLAUSE_DEFAULT. gcc/ChangeLog: * gimplify.cc (struct gimplify_omp_ctx): Add oacc_default_clause_ctx field. (new_omp_context): Initialize oacc_default_clause_ctx field. (oacc_region_type_name): New function. (oacc_default_clause): Lookup current default_kind value from ctx->oacc_default_clause_ctx, adjust default(none) error and inform message dumping. (gimplify_scan_omp_clauses): Upon OMP_CLAUSE_DEFAULT case, set ctx->oacc_default_clause_ctx to current context. gcc/testsuite/ChangeLog: * c-c++-common/goacc/default-3.c: Adjust testcase. * c-c++-common/goacc/default-5.c: Adjust testcase. * gfortran.dg/goacc/default-3.f95: Adjust testcase. * gfortran.dg/goacc/default-5.f: Adjust testcase.diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 24a6eb6e459..974f0132787 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -18196,6 +18196,7 @@ c_parser_oacc_cache (location_t loc, c_parser *parser) | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYIN) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYOUT) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_CREATE) \ + | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEFAULT) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEVICEPTR) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_IF) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_NO_CREATE) \ diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index d7ef5b34d42..bc59fbeac20 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -45860,6 +45860,7 @@ cp_parser_oacc_cache (cp_parser *parser, cp_token *pragma_tok) | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYIN) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_COPYOUT) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_CREATE) \ + | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEFAULT) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DETACH) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_DEVICEPTR) \ | (OMP_CLAUSE_MASK_1 << PRAGMA_OACC_CLAUSE_IF) \ diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index 2952cd300ac..c37f843ec3b 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -3802,7 +3802,8 @@ error: #define OACC_DATA_CLAUSES \ (omp_mask (OMP_CLAUSE_IF) | OMP_CLAUSE_DEVICEPTR | OMP_CLAUSE_COPY\ | OMP_CLAUSE_COPYIN | OMP_CLAUSE_COPYOUT | OMP_CLAUSE_CREATE \ - | OMP_CLAUSE_NO_CREATE | OMP_CLAUSE_PRESENT | OMP_CLAUSE_ATTACH) + | OMP_CLAUSE_NO_CREATE | OMP_CLAUSE_PRESENT | OMP_CLAUSE_ATTACH \ + | OMP_CLAUSE_DEFAULT) #define OACC_LOOP_CLAUSES \ (omp_mask (OMP_CLAUSE_COLLAPSE) | OMP_CLAUSE_GANG | OMP_CLAUSE_WORKER \ | OMP_CLAUSE_VECTOR | OMP_CLAUSE_SEQ | OMP_CLAUSE_INDEPENDENT \ diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index 320920ed74c..ec0ccc67da8 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -225,6 +225,7 @@ struct gimplify_omp_ctx vec loop_iter_var; location_t location; enum omp_clause_default_kind default_kind; + struct gimplify_omp_ctx *oacc_default_clause_ctx; enum omp_region_type region_type; enum tree_code code; bool combined_loop; @@ -459,6 +460,10 @@ new_omp_context (enum omp_region_type region_type) c->default_kind = OMP_CLAUSE_DEFAULT_SHARED; else c->default_kind = OMP_CLAUSE_DEFAULT_UNSPECIFIED; + if (gimplify_omp_ctxp) +c->oacc_default_clause_ctx = gimplify_omp_ctxp->oacc_default_clause_ctx; + else +c->oacc_default_clause_ctx = c; c->defaultmap[GDMK_SCALAR] =
