Hi! Regarding my comments, please keep in mind that I don't have a lot of Fortran experience; neither as a user nor as an implementor ;-) in the GCC front end, so don't hesitate to tell me if I'm misunderstanding something. As I suggested, it may make sense to CC Fortran front end maintainers for such patches.
When I say "again" in the following, that refers to things I
asked/suggested in »[PATCH 4/6] OpenACC GENERIC nodes«.
On Thu, 23 Jan 2014 22:03:09 +0400, Ilmir Usmanov <i.usma...@samsung.com> wrote:
> Subject: [PATCH 1/6] OpenACC fortran FE part 1
> --- a/gcc/fortran/dump-parse-tree.c
> +++ b/gcc/fortran/dump-parse-tree.c
> @@ -1230,6 +1230,194 @@ show_omp_node (int level, gfc_code *c)
> fprintf (dumpfile, " (%s)", c->ext.omp_name);
> }
>
> +/* Show a single OpenACC directive node and everything underneath it
> + if necessary. */
> +
> +static void
> +show_oacc_node (int level, gfc_code *c)
Is this something that accurately needs to reproduce valid Fortran code?
Or, could we again re-use more of the existing OpenMP code, and get
nearly-valid Fortran, that differs in how some of the data clauses are
displayed, for example?
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -1025,16 +1031,29 @@ gfc_namelist;
> enum
> {
> OMP_LIST_PRIVATE,
> + OACC_LIST_PRIVATE = OMP_LIST_PRIVATE,
> OMP_LIST_FIRSTPRIVATE,
> + OACC_LIST_FIRSTPRIVATE = OMP_LIST_FIRSTPRIVATE,
Again, possible to simply re-use the existing OMP_* ones?
> @@ -1047,7 +1066,29 @@ enum
> OMP_LIST_IOR,
> OMP_LIST_IEOR,
> OMP_LIST_REDUCTION_LAST = OMP_LIST_IEOR,
> - OMP_LIST_NUM
> + OACC_LIST_REDUCTION_LAST = OMP_LIST_REDUCTION_LAST,
> + OMP_LIST_NUM,
> +
> + OACC_LIST_COPY = OMP_LIST_NUM,
> + OACC_LIST_FIRST = OACC_LIST_COPY,
> + OACC_LIST_DATA_CLAUSE_FIRST = OACC_LIST_COPY,
> + OACC_LIST_COPYIN,
> + OACC_LIST_COPYOUT,
> + OACC_LIST_CREATE,
> + OACC_LIST_DELETE,
> + OACC_LIST_PRESENT,
> + OACC_LIST_PRESENT_OR_COPY,
> + OACC_LIST_PRESENT_OR_COPYIN,
> + OACC_LIST_PRESENT_OR_COPYOUT,
> + OACC_LIST_PRESENT_OR_CREATE,
> + OACC_LIST_DEVICEPTR,
> + OACC_LIST_DATA_CLAUSE_LAST = OACC_LIST_DEVICEPTR,
> + OACC_LIST_USE_DEVICE,
> + OACC_LIST_DEVICE_RESIDENT,
> + OACC_LIST_HOST,
> + OACC_LIST_DEVICE,
> + OACC_LIST_CACHE,
> + OACC_LIST_NUM
> };
Again, for conformity, maybe stick with OMP_LIST_* names
(OMP_LIST_OACC_*?).
Do we still need the OMP_LIST_NUM value if we now got OACC_LIST_NUM to
mark the number of items, and should the latter one in fact be named
OMP_LIST_NUM then?)
Again, can we re-use the existing infrastructure for OpenMP's memory
mapping instead of adding OACC_LIST_COPY* (and similar) here?
> @@ -1077,17 +1118,42 @@ typedef struct gfc_omp_clauses
> + /* OpenACC. */
> + bool is_acc;
Can we avoid the is_acc member and instead make it clear from the context
whether this is OpenACC or OpenMP? (Just an idea.)
> + [...]
> }
> gfc_omp_clauses;
>
> #define gfc_get_omp_clauses() XCNEW (gfc_omp_clauses)
>
> +typedef gfc_omp_clauses gfc_oacc_clauses;
Again, I'd just use the existing gfc_omp_* name.
> --- a/gcc/fortran/parse.c
> +++ b/gcc/fortran/parse.c
> @@ -3643,6 +3875,113 @@ parse_omp_atomic (void)
> }
>
>
> +/* Parse the statements of an OpenACC structured block. */
> +
> +static void
> +parse_oacc_structured_block (gfc_statement acc_st)
> +{
> +[...]
> --- a/gcc/fortran/parse.h
> +++ b/gcc/fortran/parse.h
> @@ -29,7 +29,8 @@ typedef enum
> COMP_BLOCK_DATA, COMP_INTERFACE, COMP_DERIVED, COMP_DERIVED_CONTAINS,
> COMP_BLOCK, COMP_ASSOCIATE, COMP_IF,
> COMP_DO, COMP_SELECT, COMP_FORALL, COMP_WHERE, COMP_CONTAINS, COMP_ENUM,
> - COMP_SELECT_TYPE, COMP_OMP_STRUCTURED_BLOCK, COMP_CRITICAL,
> COMP_DO_CONCURRENT
> + COMP_SELECT_TYPE, COMP_OMP_STRUCTURED_BLOCK, COMP_CRITICAL,
> COMP_DO_CONCURRENT,
> + COMP_OACC_STRUCTURED_BLOCK
> }
> gfc_compile_state;
In Fortran, how is an OpenACC structured block different from an OpenMP
one? In C they aren't; see the third patch in
<http://news.gmane.org/find-root.php?message_id=%3C87r49kuass.fsf%40kepler.schwinge.homeip.net%3E>,
for the C front end/middle end, which is pending approval. Or is this
something else in the Fortran front end?
Grüße,
Thomas
pgpCcAPrVfrQA.pgp
Description: PGP signature
