Created attachment 39028 [details] Fortran test case The Fortran OpenACC routine directive doesn't diagnose if there are clauses specifying conflicting levels of parallelism (such as: "!$acc routine gang worker"). Also, 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. (Note that unless Cesar's recent <http://news.gmane.org/find-root.php?message_id=%3C5776D55A.4030002%40codesourcery.com%3E> "[PATCH] OpenACC routines in fortran modules" is applied, a routine directive for intrinsic functions (such as: "!$acc routine (ABORT) seq") will be generally flagged as invalid.)
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
Confirmed.
Created attachment 39064 [details] 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.
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.
(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.
(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.
<https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00932.html>. (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.
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
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
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
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
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
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
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