Message ID | 20191006223237.81842-2-julian@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | OpenACC 2.6 manual deep copy (repost) | expand |
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
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)]; +}