[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #14 from Thomas Schwinge  ---
Author: tschwinge
Date: Thu Mar 21 20:13:44 2019
New Revision: 269858

URL: https://gcc.gnu.org/viewcvs?rev=269858&root=gcc&view=rev
Log:
[PR72741] Properly handle clauses specifying the level of parallelism for
'external' Fortran OpenACC routines

..., so as to also for these enable the generic middle end OMP code to verify
proper nesting of loops/routines regarding their levels of parallelism.

gcc/fortran/
PR fortran/72741
* openmp.c (gfc_match_oacc_routine): Set the level of parallelism
for all variants.
(gfc_resolve_oacc_routines): Call gfc_add_omp_declare_target.
gcc/testsuite/
PR fortran/72741
* c-c++-common/goacc/routine-3-extern.c: New file.
* c-c++-common/goacc/routine-3.c: Adjust.
* c-c++-common/goacc/routine-4-extern.c: New file.
* c-c++-common/goacc/routine-4.c: Adjust.
* gfortran.dg/goacc/routine-module-3.f90: New file.
* gfortran.dg/goacc/routine-external-level-of-parallelism-1.f: New
file.
* gfortran.dg/goacc/routine-external-level-of-parallelism-2.f:
Likewise.

Added:
trunk/gcc/testsuite/c-c++-common/goacc/routine-3-extern.c
  - copied, changed from r269857,
trunk/gcc/testsuite/c-c++-common/goacc/routine-3.c
trunk/gcc/testsuite/c-c++-common/goacc/routine-4-extern.c
  - copied, changed from r269857,
trunk/gcc/testsuite/c-c++-common/goacc/routine-4.c
   
trunk/gcc/testsuite/gfortran.dg/goacc/routine-external-level-of-parallelism-1.f
   
trunk/gcc/testsuite/gfortran.dg/goacc/routine-external-level-of-parallelism-2.f
trunk/gcc/testsuite/gfortran.dg/goacc/routine-module-3.f90
Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/openmp.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/c-c++-common/goacc/routine-3.c
trunk/gcc/testsuite/c-c++-common/goacc/routine-4.c

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #13 from Thomas Schwinge  ---
Author: tschwinge
Date: Thu Mar 21 19:54:51 2019
New Revision: 269856

URL: https://gcc.gnu.org/viewcvs?rev=269856&root=gcc&view=rev
Log:
[PR72741] The name in a Fortran OpenACC 'routine' directive refers to the
containing subroutine or function

gcc/fortran/
PR fortran/72741
* openmp.c (gfc_match_oacc_routine): Clarify.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/routine-module-mod-1.f90: Update.

Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/openmp.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #12 from Thomas Schwinge  ---
Author: tschwinge
Date: Thu Mar 21 19:44:45 2019
New Revision: 269855

URL: https://gcc.gnu.org/viewcvs?rev=269855&root=gcc&view=rev
Log:
[PR72741] Encode OpenACC 'routine' directive's level of parallelism inside
Fortran module files

If 'use'ing with an old GCC a new module file (with OpenACC 'routine'
directive's level of parallelism encoded), then that expectedly fails as
follows:

f951: Fatal Error: Reading module 'routine_module_mod_1' at line 27 column
21: find_enum(): Enum not found

If 'use'ing with a new GCC an old module file (without OpenACC 'routine'
directive's level of parallelism encoded), then that (silently) continues to
accept the module file, and will proceed with the previous, erroneous behavior.

These seem to be acceptable compromises, instead of incrementing 'MOD_VERSION'.

gcc/fortran/
PR fortran/72741
* module.c (verify_OACC_ROUTINE_LOP_NONE): New function.
(enum ab_attribute): Add AB_OACC_ROUTINE_LOP_GANG,
AB_OACC_ROUTINE_LOP_WORKER, AB_OACC_ROUTINE_LOP_VECTOR,
AB_OACC_ROUTINE_LOP_SEQ.
(attr_bits): Add these.
(mio_symbol_attribute): Handle these.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/routine-module-1.f90: New file.
* gfortran.dg/goacc/routine-module-2.f90: Likewise.
* gfortran.dg/goacc/routine-module-mod-1.f90: Likewise.

Added:
trunk/gcc/testsuite/gfortran.dg/goacc/routine-module-1.f90
trunk/gcc/testsuite/gfortran.dg/goacc/routine-module-2.f90
trunk/gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90
Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/module.c
trunk/gcc/testsuite/ChangeLog

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #10 from Thomas Schwinge  ---
Author: tschwinge
Date: Thu Feb 28 20:31:23 2019
New Revision: 269286

URL: https://gcc.gnu.org/viewcvs?rev=269286&root=gcc&view=rev
Log:
[PR72741] For all Fortran OpenACC 'routine' directive variants check for
multiple clauses specifying the level of parallelism

gcc/fortran/
PR fortran/72741
* gfortran.h (enum oacc_routine_lop): Add OACC_ROUTINE_LOP_ERROR.
* openmp.c (gfc_oacc_routine_lop, gfc_match_oacc_routine): Use it.
* trans-decl.c (add_attributes_to_decl): Likewise.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90: New file.

Added:
trunk/gcc/testsuite/gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90
Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/gfortran.h
trunk/gcc/fortran/openmp.c
trunk/gcc/fortran/trans-decl.c
trunk/gcc/testsuite/ChangeLog

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #11 from Thomas Schwinge  ---
Author: tschwinge
Date: Thu Feb 28 20:31:36 2019
New Revision: 269287

URL: https://gcc.gnu.org/viewcvs?rev=269287&root=gcc&view=rev
Log:
[PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive

gcc/fortran/
PR fortran/72741
PR fortran/89433
* openmp.c (gfc_match_oacc_routine): Handle repeated use of the
Fortran OpenACC 'routine' directive.
gcc/testsuite/
PR fortran/72741
PR fortran/89433
* gfortran.dg/goacc/routine-multiple-directives-1.f90: New file.
* gfortran.dg/goacc/routine-multiple-directives-2.f90: Likewise.

Added:
trunk/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-1.f90
trunk/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-2.f90
Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/openmp.c
trunk/gcc/testsuite/ChangeLog

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #9 from Thomas Schwinge  ---
Author: tschwinge
Date: Thu Feb 28 20:31:01 2019
New Revision: 269285

URL: https://gcc.gnu.org/viewcvs?rev=269285&root=gcc&view=rev
Log:
[PR72741, PR89433] Accept intrinsic symbols in Fortran OpenACC 'routine'
directives

gcc/fortran/
PR fortran/72741
PR fortran/89433
* openmp.c (gfc_match_oacc_routine): Accept intrinsic symbols.
gcc/testsuite/
PR fortran/72741
PR fortran/89433
* gfortran.dg/goacc/routine-6.f90: Update
* gfortran.dg/goacc/routine-intrinsic-1.f: New file.
* gfortran.dg/goacc/routine-intrinsic-2.f: Likewise.

Added:
trunk/gcc/testsuite/gfortran.dg/goacc/routine-intrinsic-1.f
trunk/gcc/testsuite/gfortran.dg/goacc/routine-intrinsic-2.f
Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/openmp.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gfortran.dg/goacc/routine-6.f90

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #8 from Thomas Schwinge  ---
Author: tschwinge
Date: Fri Feb 22 10:50:35 2019
New Revision: 269105

URL: https://gcc.gnu.org/viewcvs?rev=269105&root=gcc&view=rev
Log:
[PR72741] Use 'oacc_build_routine_dims' for Fortran OpenACC 'routine'
directives, too

... instead of having an incomplete local implementation.

With these changes in place, we can then also revert the work-around r267213
"[nvptx] Unify C/Fortran routine handling in nvptx_goacc_validate_dims".

gcc/fortran/
PR fortran/72741
* gfortran.h (oacc_routine_lop): New enum.
(symbol_attribute): Use it.
* openmp.c (gfc_oacc_routine_dims): Replace with...
(gfc_oacc_routine_lop): ... this new function.
(gfc_match_oacc_routine): Adjust.
* trans-decl.c (add_attributes_to_decl): Likewise.
gcc/
PR fortran/72741
* omp-general.c (oacc_replace_fn_attrib): Mostly split out into...
(oacc_replace_fn_attrib_attr): ... this new function.
* omp-general.h (oacc_replace_fn_attrib_attr): New prototype.
* config/nvptx/nvptx.c (nvptx_goacc_validate_dims_1): Revert
workaround.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/classify-routine.f95: Adjust.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/nvptx/nvptx.c
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/gfortran.h
trunk/gcc/fortran/openmp.c
trunk/gcc/fortran/trans-decl.c
trunk/gcc/omp-general.c
trunk/gcc/omp-general.h
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gfortran.dg/goacc/classify-routine.f95

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

Thomas Schwinge  changed:

   What|Removed |Added

   Assignee|cesar at gcc dot gnu.org   |tschwinge at gcc dot 
gnu.org

--- Comment #7 from Thomas Schwinge  ---
.

(In reply to cesar from comment #6)
> a) The fortran pretty printer doesn't know about generic tree notes.
> Consequently, I don't think build_oacc_routine_dims would be able to report
> any errors unless we add support for tree notes in that pretty printer.

I've not (yet?) into any such problems.  Phew.  ;-)

> b) The fortran FE handle errors slightly differently from the c FE. Instead
> of catching the error early and aborting immediately, you're method will
> defer the error handling to after the FE has parsed everything. I'm not sure
> if this is a problem or not.

It seems to work fine, and, as discussed, something like that will be needed
anyway, once we get to support the OpenACC device_type clause.

> c) That oacc_function information needs to be captured for fortran module
> .mod files, and those .mod files don't require tree node attributes.
> Postponing that attribute requires separate functions to manipulate the
> routine clauses.

ACK.  Still very much TODO in my WIP patch.

> d) gfc_oacc_routine_dims is already creating an oacc_function attribute for
> routines. This attribute is a single enum instead of the individual clauses.
> I like that because its more self contained.

As discussed, it actually seems easier conceptually (maybe not in terms of
"boilerplate code") to handle these separately, and we need to do that for
being able to generate the desired compile-time diagnostics.

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[cesar at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #6 from cesar at gcc dot gnu.org ---
(In reply to Thomas Schwinge from comment #5)
> (In reply to cesar from comment #4)

> Are you saying that's not how the Fortran front end operates, and the
> "SUBROUTINE r_w" and the later "PROGRAM main" do see completly detached
> "decl spaces"?  Then indeed, we can't verify this in the Fortran front end,
> but...

Yes, the address spaces are completely detached in separate, external function.
Internal functions, i.e. those that are places after "contains:" share the same
scope as the main program/module block. 

Keep in mind that fortran has a lot of intrinsic procedures, so those find
function/subroutine functions mostly resolve those intrinsic procedures or
procedures explicitly declared by the user.

> > especially if lto is not enabled. If lto
> > is enabled, we could add some more checking in pass oacc_device_lower.
> 
> ..., as discussed before, something like that is the final goal that I'm
> working on, and for that...
> 
> > But I
> > don't think there's anything else to do in the fortran front end.
> 
> ..., please implement the changes I asked you to implement in Comment 3,
> #c3, or elaborate why that doesn't make sense.

That's a good idea in principle, but there are a couple of reasons why I'm
hesitant to pursue it.

a) The fortran pretty printer doesn't know about generic tree notes.
Consequently, I don't think build_oacc_routine_dims would be able to report any
errors unless we add support for tree notes in that pretty printer.

b) The fortran FE handle errors slightly differently from the c FE. Instead of
catching the error early and aborting immediately, you're method will defer the
error handling to after the FE has parsed everything. I'm not sure if this is a
problem or not.

c) That oacc_function information needs to be captured for fortran module .mod
files, and those .mod files don't require tree node attributes. Postponing that
attribute requires separate functions to manipulate the routine clauses.

d) gfc_oacc_routine_dims is already creating an oacc_function attribute for
routines. This attribute is a single enum instead of the individual clauses. I
like that because its more self contained.

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #5 from Thomas Schwinge  ---
(In reply to cesar from comment #4)
> I could be mistaken, but I don't think there's anything we can do about that
> test case because fortran doesn't have file scope. Specifically, in your
> example,
> 
> SUBROUTINE r_w
>   IMPLICIT NONE
>   INTEGER :: i
>   !$ACC ROUTINE WORKER
> 
>   !$ACC LOOP
>   DO i = 1, 12345
>   END DO
> END SUBROUTINE r_w
> 
> PROGRAM main
>   IMPLICIT NONE
>   EXTERNAL :: r_w
>   !$ACC ROUTINE (r_w) GANG
> 
>   !$ACC PARALLEL
>   CALL r_w
>   !$ACC END PARALLEL
> END PROGRAM main
> 
> from program main's perspective, r_w should be an acc routine with a gang
> attribute, but that's only because the end user included an explicit
> 'external' function declaration. I'm not sure how to catch mismatched
> function declarations in fortran

Maybe I'm assuming too much about the Fortran front end, but I assumed that the
"SUBROUTINE r_w" would create some kind of decl, and the later "!$ACC ROUTINE
(r_w)" would look up some kind of decl (that is, the
gfc_find_function/gfc_find_subroutine/gfc_find_symtree stuff in
gfc_match_oacc_routine), and these decls both have some kind of "attributes"
attached to them, and once these two decls are "paired"/"merged", we can then
see whether these "attributes" match or conflict.  I assume some very similar
verification is done in the Fortran front end for other checking between
definition and use of any kind of routines, for example?

Are you saying that's not how the Fortran front end operates, and the
"SUBROUTINE r_w" and the later "PROGRAM main" do see completly detached "decl
spaces"?  Then indeed, we can't verify this in the Fortran front end, but...

> especially if lto is not enabled. If lto
> is enabled, we could add some more checking in pass oacc_device_lower.

..., as discussed before, something like that is the final goal that I'm
working on, and for that...

> But I
> don't think there's anything else to do in the fortran front end.

..., please implement the changes I asked you to implement in Comment 3, #c3,
or elaborate why that doesn't make sense.

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[cesar at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #4 from cesar at gcc dot gnu.org ---
I could be mistaken, but I don't think there's anything we can do about that
test case because fortran doesn't have file scope. Specifically, in your
example,

SUBROUTINE r_w
  IMPLICIT NONE
  INTEGER :: i
  !$ACC ROUTINE WORKER

  !$ACC LOOP
  DO i = 1, 12345
  END DO
END SUBROUTINE r_w

PROGRAM main
  IMPLICIT NONE
  EXTERNAL :: r_w
  !$ACC ROUTINE (r_w) GANG

  !$ACC PARALLEL
  CALL r_w
  !$ACC END PARALLEL
END PROGRAM main

from program main's perspective, r_w should be an acc routine with a gang
attribute, but that's only because the end user included an explicit 'external'
function declaration. I'm not sure how to catch mismatched function
declarations in fortran, especially if lto is not enabled. If lto is enabled,
we could add some more checking in pass oacc_device_lower. But I don't think
there's anything else to do in the fortran front end.

It should be noted that if the user wants to avoid these types of problems, acc
routines should be placed inside modules. That way, each procedure sees the
same acc routine attributes.

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #3 from Thomas Schwinge  ---
Created attachment 39064
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39064&action=edit
Fortran routine directive with a name

Cesar, thanks for your initial fix.

Please have a look at the new file I'm attaching.  Clearly, the two OpenACC
routine directives are conflicting; this illustrates my earlier point 'from a
quick look it seems as if for a routine directive with a name (such as: "!$acc
routine (function_1) gang"), any clauses specified are generally not evaluated
to determine the specified function's level of parallelism'.  No compiler
diagnostic is being created for that.

Instead of fixing just that specific problem only, please (also) verify and
implement my following ideas (separate patch?).  In early parsing in the front
end, do not check for routine dimension errors (we can't do that anyway, if you
take the device_type clause into account).  That is, remove
gcc/fortran/openmp.c:gfc_oacc_routine_dims.  Instead of the "dims" you compute
from the clauses in gcc/fortran/openmp.c:gfc_match_oacc_routine and save in
attr.oacc_function, just store all clauses (gfc_omp_clauses) in that "attr". 
If that is not feasible (I don't know how "attr" is processed in the following,
also regarding Fortran modules etc.), then have "attr" contain *separate*
members for all clauses that apply to the OpenACC routine directive: gang,
worker, vector, seq, bind, nohost.  Then, move the "multiple loop axes"
checking into gcc/fortran/trans-decl.c:add_attributes_to_decl.  As this
function has access to all gfc_omp_clauses corresponding to the decl (or, at
least all individual OpenACC routine clauses), this should then ideally just
call gcc/fortran/trans-openmp.c:gfc_trans_omp_clauses to translate the Fortran
OMP clauses representation into OMP_CLAUSE trees.  (Notice that on
gomp-4_0-branch, gcc/fortran/trans-decl.c:add_attributes_to_decl already
contains some incomplete, ad-hoc code to handle the nohost clause; you can
ignore that stuff for the moment -- if you put the infrastructure in place for
the clauses specifying the level of parallelism, we can then later add
bind/nohost handling.)  Instead of the manual "oacc function" attribute
construction, then call gcc/omp-low.c:build_oacc_routine_dims and
gcc/omp-low.c:replace_oacc_fn_attrib, as I had commented earlier.

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

Thomas Schwinge  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |cesar at gcc dot gnu.org

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

Dominique d'Humieres  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-07-30
 Ever confirmed|0   |1

--- Comment #2 from Dominique d'Humieres  ---
Confirmed.

[Bug fortran/72741] Fortran OpenACC routine directive doesn't properly handle clauses specifying the level of parallelism

[cesar at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72741

--- Comment #1 from cesar at gcc dot gnu.org ---
Author: cesar
Date: Fri Jul 29 04:20:00 2016
New Revision: 238847

URL: https://gcc.gnu.org/viewcvs?rev=238847&root=gcc&view=rev
Log:
PR fortran/72741
gcc/fortran/
* openmp.c (gfc_oacc_routine_dims): Move gfc_error to
gfc_match_oacc_routine.  Return OACC_FUNCTION_NONE on error.
(gfc_match_oacc_routine): Call gfc_oacc_routine_dims for all
routines directives.  Propagate error as appropriate.

gcc/testsuite/
* gfortran.dg/goacc/pr72741.f90: New test.


Added:
branches/gomp-4_0-branch/gcc/testsuite/gfortran.dg/goacc/pr72741.f90
Modified:
branches/gomp-4_0-branch/gcc/fortran/ChangeLog.gomp
branches/gomp-4_0-branch/gcc/fortran/openmp.c
branches/gomp-4_0-branch/gcc/testsuite/ChangeLog.gomp