[1/4] Add function for pretty-printing OpenACC clause names
diff mbox series

Message ID 20191006223237.81842-2-julian@codesourcery.com
State New
Headers show
Series
  • OpenACC 2.6 manual deep copy (repost)
Related show

Commit Message

Julian Brown Oct. 6, 2019, 10:32 p.m. UTC
This patch adds a function to pretty-print OpenACC clause names from
OMP_CLAUSE_MAP_KINDs, for error output. The function is used by subsequent
patches.

Previously approved as part of:

  https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01292.html

FAOD, OK for trunk?

Julian

ChangeLog

	gcc/
	* c-family/c-common.h (c_omp_map_clause_name): Add prototype.
	* c-family/c-omp.c (c_omp_map_clause_name): New function.
---
 gcc/c-family/c-common.h |  1 +
 gcc/c-family/c-omp.c    | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Thomas Schwinge Oct. 18, 2019, 12:28 p.m. UTC | #1
Hi Julian!

On 2019-10-06T15:32:34-0700, Julian Brown <julian@codesourcery.com> wrote:
> This patch adds a function to pretty-print OpenACC clause names from
> OMP_CLAUSE_MAP_KINDs, for error output.

Indeed talking about (OpenMP) 'map' clauses in an OpenACC context is not
quite ideal -- that's what PR65095 is about, so please mention that one
in your ChangeLog updates.

> The function is used by subsequent
> patches.

Ah, I somehow had assumed you'd also adapt existing code to use it.  ;-)

> Previously approved as part of:
>
>   https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01292.html
>
> FAOD, OK for trunk?

Still fine.  To record the review effort, please include "Reviewed-by:
Thomas Schwinge <thomas@codesourcery.com>" in the commit log, see
<https://gcc.gnu.org/wiki/Reviewed-by>.


A few more comments, for later:

>  gcc/c-family/c-common.h |  1 +
>  gcc/c-family/c-omp.c    | 33 +++++++++++++++++++++++++++++++++

As I'd mentioned before: 'Eventually (that is, later), this should move
into generic code, next to the other "clause printing".  Also to be
shared with Fortran.'

> --- a/gcc/c-family/c-omp.c
> +++ b/gcc/c-family/c-omp.c

> +/* For OpenACC, the OMP_CLAUSE_MAP_KIND of an OMP_CLAUSE_MAP is used internally
> +   to distinguish clauses as seen by the user.  Return the "friendly" clause
> +   name for error messages etc., where possible.  See also
> +   c/c-parser.c:c_parser_oacc_data_clause and
> +   cp/parser.c:cp_parser_oacc_data_clause.  */
> +
> +const char *
> +c_omp_map_clause_name (tree clause, bool oacc)
> +{
> +  if (oacc && OMP_CLAUSE_CODE (clause) == OMP_CLAUSE_MAP)
> +    switch (OMP_CLAUSE_MAP_KIND (clause))
> +    {
> +    case GOMP_MAP_FORCE_ALLOC:
> +    case GOMP_MAP_ALLOC: return "create";
> +    case GOMP_MAP_FORCE_TO:
> +    case GOMP_MAP_TO: return "copyin";
> +    case GOMP_MAP_FORCE_FROM:
> +    case GOMP_MAP_FROM: return "copyout";
> +    case GOMP_MAP_FORCE_TOFROM:
> +    case GOMP_MAP_TOFROM: return "copy";
> +    case GOMP_MAP_RELEASE: return "delete";
> +    case GOMP_MAP_FORCE_PRESENT: return "present";
> +    case GOMP_MAP_ATTACH: return "attach";
> +    case GOMP_MAP_FORCE_DETACH:
> +    case GOMP_MAP_DETACH: return "detach";
> +    case GOMP_MAP_DEVICE_RESIDENT: return "device_resident";
> +    case GOMP_MAP_LINK: return "link";
> +    case GOMP_MAP_FORCE_DEVICEPTR: return "deviceptr";
> +    default: break;
> +    }
> +  return omp_clause_code_name[OMP_CLAUSE_CODE (clause)];
> +}

Indeed nearby (after) the 'omp_clause_code_name' definition in
'gcc/tree.c' would probably be a better place for this, as that's where
the current clause names are coming from.

I did wonder whether we need to explicitly translate from (OpenMP) "'map'
clause" into (OpenACC) "'create' clause" etc., or if a generic (OpenACC)
"data clause" would be sufficient?  (After all, in diagnostics we also
print out the original code, so the user can then see which specific data
clause is being complained about.  But -- somewhat funnily! -- the way
you're doing this might actually be better in terms of translatability in
diagnostics printing: "%qs clause" might require a different translation
when its "%s" can be "'map'" (doesn't get translated) vs. "data" (gets
translated), but remains the same when "%s" is "'map'" vs. "'create'"
etc.

Do we at all still generate 'GOMP_MAP_FORCE_*' anywhere, or should these
in fact be 'gcc_unreachable'?

Generally, I prefer if all possible 'case's are listed explicitly, and
then the 'default' (and here OpenMP-only ones, too) be 'gcc_unreachable',
so that we easily catch the case that new 'GOMP_MAP_*' get added but such
functions not updated, for example.


Grüße
 Thomas

Patch
diff mbox series

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 1e13aaa16fc..6fddae6b9e5 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1189,6 +1189,7 @@  extern tree c_omp_declare_simd_clauses_to_numbers (tree, tree);
 extern void c_omp_declare_simd_clauses_to_decls (tree, tree);
 extern bool c_omp_predefined_variable (tree);
 extern enum omp_clause_default_kind c_omp_predetermined_sharing (tree);
+extern const char *c_omp_map_clause_name (tree, bool);
 
 /* Return next tree in the chain for chain_next walking of tree nodes.  */
 static inline tree
diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index 0048289b691..1bd91b9ebe8 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -2120,3 +2120,36 @@  c_omp_predetermined_sharing (tree decl)
 
   return OMP_CLAUSE_DEFAULT_UNSPECIFIED;
 }
+
+/* For OpenACC, the OMP_CLAUSE_MAP_KIND of an OMP_CLAUSE_MAP is used internally
+   to distinguish clauses as seen by the user.  Return the "friendly" clause
+   name for error messages etc., where possible.  See also
+   c/c-parser.c:c_parser_oacc_data_clause and
+   cp/parser.c:cp_parser_oacc_data_clause.  */
+
+const char *
+c_omp_map_clause_name (tree clause, bool oacc)
+{
+  if (oacc && OMP_CLAUSE_CODE (clause) == OMP_CLAUSE_MAP)
+    switch (OMP_CLAUSE_MAP_KIND (clause))
+    {
+    case GOMP_MAP_FORCE_ALLOC:
+    case GOMP_MAP_ALLOC: return "create";
+    case GOMP_MAP_FORCE_TO:
+    case GOMP_MAP_TO: return "copyin";
+    case GOMP_MAP_FORCE_FROM:
+    case GOMP_MAP_FROM: return "copyout";
+    case GOMP_MAP_FORCE_TOFROM:
+    case GOMP_MAP_TOFROM: return "copy";
+    case GOMP_MAP_RELEASE: return "delete";
+    case GOMP_MAP_FORCE_PRESENT: return "present";
+    case GOMP_MAP_ATTACH: return "attach";
+    case GOMP_MAP_FORCE_DETACH:
+    case GOMP_MAP_DETACH: return "detach";
+    case GOMP_MAP_DEVICE_RESIDENT: return "device_resident";
+    case GOMP_MAP_LINK: return "link";
+    case GOMP_MAP_FORCE_DEVICEPTR: return "deviceptr";
+    default: break;
+    }
+  return omp_clause_code_name[OMP_CLAUSE_CODE (clause)];
+}