Hi, Thomas!
This is really great!
I've looked at your changes and in most of front-end parts it looks reasonable
to me. As you know, we're like-minded with you about how OpenACC's front-ends
should look like. So, I think it's good that you have working flow for your
implementation. Now we can start to merge front-ends from openacc-1_0-branch to
GOMP, while keeping all understandings of ACC in sync and working.
Jakub, don't you mind if I prepare a few front-end patches likewise for review
to extend Thomas' vision with the things that was already done by my colleagues
on the ACC branch? I hope it'll much speed up the development.
> the last patch adds GOACC_parallel, which so far simply branches to
> GOMP_target. There's more to come.
I've reviewed ACC-related code from [openacc-1_0-branch] and from the patches,
I noticed there are some mess around marking OpenACC specific code. There are
ACC, GACC, OACC, GOACC abbreviations used across code, which one is the best? I
prefer ACC, this is not critical, I'm just curious how to resolve this.
> Due to the conceptual similarity compared to OpenMP, and that (later)
> it is reasonable to expect to embed OpenACC directives into OpenMP
> ones, the approach I've chosen is to directly embed OpenACC handling
> into the existing OpenMP infrastructure/passes.
Regarding front-ends, not only from view of OMP/ACC, but front-ends itself, it
makes sense to move some parsing routines out of gomp/acc context. This will be
useful to implement not only ACC, but also HMPP, if someone is interested, or
other similar technology. And of cause, these functions should be used behind
the existing facilities - not to break current implementation and other
dependencies. So it won't not introduce any difficulties for back porting,
while generalizing front-ends a bit.
For now, I prefer to extend the approved way of developing, but keep it in
mind, please.
> This patch series doesn't contain any substantial rumtime library work
> yet;
Can you share some technical details/ideas about further implementation, it's
not going to be trivial. On the [openacc-1_0-branch] we resolved it with
OpenCL-driven runtime. Also, OpenACC 2.0 standard declares some vendor-specific
routines, this should be noted too.
> This is in contrast to Samsung's work, who are implementing OpenACC
> separately from the existing OpenMP support.
I'm not fully agree that this in contrast to things are taking place in
[openacc-1_0-branch]. The main reason for keeping middle-end and back-end
solutions far from GOMP is the understanding of OpenACC semantics and unclear
understanding of what should be done to generalize support of accelerators in
GCC. OpenACC and OpenOMP has some semantically differences, but such things
matters only for middle-end and further code generation. The main one, is the
memory concept. The
> Yet, I hope we'll be able to share/re-use at least front end and some
middle end code.
I absolutely agree, it's needed to merge our efforts regarding front-ends.
Many things from ACC in [openacc-1_0-branch] is already done, so it doesn't
necessary to re-implement same things. Maybe we can polish front-ends together
and then commit changes to GOMP?
> We directly strive for OpenACC 2.0 support, skipping OpenACC 1. We're
> focussing on the C front end implementation first, following on with
> C++ and Fortran later on.
How do you think, does it make sense to adopt current Fortran implementation
from the OpenACC branch? This can be done quite soon. Also, I may help to
extend C and C++ front-end the same way it's done now, by adding other
implemented directives, too.
Regarding proposed patches:
[8/9] c_parser_omp_all_clauses and cp_parser_omp_all_clauses
- c_parser_error (parser, "expected %<#pragma omp%> clause");
+ c_parser_error (parser, "expected clause");
- cp_parser_error (parser, "expected %<#pragma omp%> clause");
+ cp_parser_error (parser, "expected clause");
Does it really needed to remove %<#pragma omp%> in these cases? We handle both,
I think.
[9/9]
* gimple.h (gimple_build_oacc_*): New declaration.
gimple_oacc_parallel_* and other functions like that. Let's move these
functions out of gimple.* to gimple-oacc.*! There are going to be much more of
the stuff like this, maybe it'll make sense to keep it externally? May be not
only gimple manipulation functions but also pretty-print Also, have a look on
openacc_1-0_branch please, for gimple-oacc.*, is OK if I'll add some of this?
Handling clauses.
+ GIMPLE_CHECK (gs, GIMPLE_OACC_PARALLEL);
+ gs->gimple_omp_parallel.clauses = clauses;
I think, we can freely add some kind of flag to gimple_omp_parallel to indicate
that we're are working with ACC.
Or even add new gimple_acc_* statements.
On gimplification, lowering stuff and further, I think it's okay if targeting
GOMP_target, but on my view, this is going to prevent us from emiting OpenCL or
do some extended analysis of the code for error analysis to be offloaded. May
be introduce some switch that turns on OpenCL generation or GOMP_targeting
dependently on the state?
I hope my colleagues will correct me, if I forgot something.
-
Thanks,
Evgeny.
