[PATCH 1/6] [GOMP4] OpenACC 1.0+ support in fortran front-end
Tobias Burnus
burnus@net-b.de
Sun Feb 9 22:22:00 GMT 2014
Ilmir Usmanov wrote:
> OpenACC 1.0 support to fortran FE -- core.
> --- a/gcc/fortran/dump-parse-tree.c
> +++ b/gcc/fortran/dump-parse-tree.c
> @@ -1031,9 +1031,22 @@ show_omp_node (int level, gfc_code *c)
> {
> gfc_omp_clauses *omp_clauses = NULL;
> const char *name = NULL;
> + bool is_oacc = false;
I'd also update the comment before the function and mention OpenACC
there. It currently reads:
/* Show a single OpenMP directive node and everything underneath it
if necessary. */
> + fprintf (dumpfile, "!$%s %s", is_oacc?"ACC":"OMP", name);
Add spaces around "?" and ":"
> @@ -1215,7 +1334,7 @@ show_omp_node (int level, gfc_code *c)
> - fprintf (dumpfile, "!$OMP END %s", name);
> + fprintf (dumpfile, "!$%s END %s", is_oacc?"ACC":"OMP", name);
Ditto.
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
>
> +/* Likewise to gfc_namelist, but contains expressions. */
> +typedef struct gfc_exprlist
> +{
> + struct gfc_expr *expr;
> + struct gfc_exprlist *next;
> +}
> +gfc_exprlist;
I don't feel strong about it, but I think it is more GCC / gfortran
style to use "gfc_expr_list" instead of "gfc_exprlist".
> + /* OpenACC. */
> + struct gfc_expr *async_expr;
> + struct gfc_expr *gang_expr;
> + struct gfc_expr *worker_expr;
> + struct gfc_expr *vector_expr;
> + struct gfc_expr *num_gangs_expr;
> + struct gfc_expr *num_workers_expr;
> + struct gfc_expr *vector_length_expr;
> + struct gfc_expr *non_clause_wait_expr;
> + gfc_exprlist *waitlist;
> + gfc_exprlist *tilelist;
> + bool async, gang, worker, vector, seq, independent;
> + bool wait, par_auto, gang_static;
I wonder whether it would make sense to use bit fields here.
> --- a/gcc/fortran/match.c
> +++ b/gcc/fortran/match.c
> @@ -2595,6 +2595,33 @@ match_exit_cycle (gfc_statement st, gfc_exec_op op)
> if (cnt > 0
> && o != NULL
> && o->state == COMP_OMP_STRUCTURED_BLOCK
> + && (o->head->op == EXEC_OACC_LOOP
> + || o->head->op == EXEC_OACC_PARALLEL_LOOP))
> + {
> + int collapse = 1;
> + gcc_assert (o->head->next != NULL
> + && (o->head->next->op == EXEC_DO
> + || o->head->next->op == EXEC_DO_WHILE)
Two questions:
a) Does this part work well when both -fopenmp and -fopenacc is used? I
mean: "!$acc loop" followed/preceded by "!$omp do"? I could imagine that
there could be clashes, especially when - e.g. - collapse doesn't match.
b) Do you also handle DO CONCURRENT - either by rejecting it or by
accepting it? Namely,
!$acc loop
do concurrent(i=1:5)
end do
!$acc end loop
end
Side remark, with -fopenmp, it does ICE:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60127
Talking about "!$acc loop": I vaguely remember that OpenACC 1.0's spec
doesn't have "!$acc end loop" while I have seen OpenACC programs which
use it. How do you handle "!$acc end loop"?
Looking at parse.c, it seems to simply error out. I wonder whether one
should be a bit more graceful. For instance, the following examples use
"!$acc end loop":
http://devblogs.nvidia.com/parallelforall/openacc-example-part-2/ [If I
remember correctly, pgf95 and Cray ftn silently accepts "end loop" while
pathf95 accepts it with a warning.]
And looking at the spec of OpenACC 1.0 and 2.0a, the "end loop" seems to
be invalid. How about following PathScale's ENZO and accepting "end
loop" with a warning? Or at least error out with a good error message.
> + if (st == ST_EXIT && cnt <= collapse)
> + {
> + gfc_error ("EXIT statement at %C terminating !$ACC LOOP loop");
> + return MATCH_ERROR;
> + }
> + if (st == ST_CYCLE && cnt < collapse)
> + {
> + gfc_error ("CYCLE statement at %C to non-innermost collapsed"
> + " !$ACC LOOP loop");
> + return MATCH_ERROR;
> + }
> + }
I wonder whether one should include for OpenMP and OpenACC some
additional checks as done for DO CONCURRENT or whether those aren't
required. I think image controll statements like Fortran 2008's CRITICAL
block might cause problems with OpenACC/OpenMP concurrency. (Given that
OpenACC/OpenMP likely ignores Fortran 2008's coarrays, one might defer
this until OpenACC/OpenMP has been updated for Fortran 2008.)
> + if (gfc_pure (NULL))
> + {
> + gfc_error_now ("OpenACC directives at %C may not appear in PURE "
> + "or ELEMENTAL procedures");
Using gfc_pure() you do not check for ELEMENTAL: Since Fortran 2008,
there are also IMPURE ELEMENTAL procedures. I don't know the spec, but I
don't really see a reason why OpenACC shouldn't be permitted in IMPURE
ELEMENTAL procedures. (BTW: "ELEMENTAL" implies PURE unless an explicit
IMPURE is used.)
In any case, either drop "or ELEMENTAL" or also check for the elemental
attribute.
> + if (gfc_implicit_pure (NULL))
> + gfc_current_ns->proc_name->attr.implicit_pure = 0;
I believe that will fail with BLOCK - cf. gfc_implicit_pure()
real function foo(n)
integer, value :: n
BLOCK
integer i
real sum
!$acc loop reduce(+:sum)
do i=1, n
sum += sin(real(i))
end do
END BLOCK
end
> + /* All else has failed, so give up. See if any of the matchers has
> + stored an error message of some sort. */
Indenting of the second line looks wrong. And "all else" sounds language
wise strange to me.
> +static void
> +verify_token_free (const char* token, int length, bool last_was_use_stmt)
> +{
This function should have a comment describing what it does.
> +static bool
> +verify_token_fixed (const char *token, int length, bool last_was_use_stmt)
Also here, a short comment line before the function would be helpful.
> --- a/gcc/fortran/scanner.c
> +++ b/gcc/fortran/scanner.c
> +static int openmp_flag, openacc_flag; /* If !$omp/!&acc occurred in current comment line */
Is this really !&acc and not !$acc?
> + else if (gfc_option.gfc_flag_openmp&& !gfc_option.gfc_flag_openacc)
Space before "&&".
> + /* If -fopenmp/-fopenacc, we need to handle here 2 things:
> + 1) don't treat !$omp/!$acc|c$omp/c$acc|*$omp / *$acc as comments,
Is there a reason for the spaces around the last "/"?
> + gcc_assert(gfc_wide_tolower (c) == (unsigned char ) "!$acc"[i]);
Spurious " " before ")".
Tobias
More information about the Gcc-patches
mailing list