- changed status to open
- removed comment
handling of OMP continuation lines in f_file_processor.pl
Issue #388
closed
OpenMP requires continuation lines in Fortran 90 (and possibly Fortran 77 as well though I have not checked that) to start with the OMP sentinel (https://computing.llnl.gov/tutorials/openMP/#Directives).
The attached patch to f_file_processor.pl ensures that a sentinel OMP or HPF is repeated when a line starting with that sentinel is broken to limit its length to 132 characters.
It also adds a case-insensitive flag to the check for "!" like comments and adds extra backslashes to the standard_comments regex to properly match $OMP after all metacharacters have been handled.
Tested with LoopControl's Fortran interface on gcc 4.5 and Intel 11.1 (using LoopControl's Fortran interface, see ticket #387).
Keyword:
Comments (12)
-
reporter
-
- removed comment
The patch looks good to me (didn't test it).
-
reporter
- removed comment
Thank you. Since this changes something rather fundamental in Cactus, I'd feel much more comfortable if someone other than me tested this :-) Plus I cannot possibly apply the patch myself since I am not a Cactus maintainer (I think). We care about the PGI compiler on Kraken, do we? I should probably test it there as well (unless Ian would like to... I remember compiling on Kraken being rather painful).
-
- removed comment
I can test it on Kraken. Is there a testsuite that should be run, or some other parameter file?
-
reporter
- removed comment
Replying to [comment:4 hinder]:
I can test it on Kraken. Is there a testsuite that should be run, or some other parameter file?
Greatly appreciated. There is no real testsuite since it only modifies a Cactus internal file used to produce the Fortran source that the compiler sees (the .f90 files in build/ThornName). However any of the GRHydro tests would likely do. In particular GRHydro/test/test/GRHydro_test_tov_ppm_ML.par with the following changes to GRHydrdo_Con2Prim.F90 (to actually trigger any non-trivial changes). I had tested that it compiles (and runs) with LoopControl for Fortran (which generates long OMP lines).
Index: src/GRHydro_Con2Prim.F90
--- src/GRHydro_Con2Prim.F90 (revision 226) +++ src/GRHydro_Con2Prim.F90 (working copy) @@ -1154,7 +1154,7 @@ !!$ do j = GRHydro_stencil + 1, ny - GRHydro_stencil !!$ do i = GRHydro_stencil + 1, nx - GRHydro_stencil !$OMP PARALLEL DO PRIVATE(i,j,itracer,& - !$OMP uxx, uxy, uxz, uyy, uyz, uzz, det) + !$OMP uxx, uxy, uxz, uyy, uyz, uzz, det) do k = 1, nz do j = 1, ny do i = 1, nx
-
- removed comment
I have run all the testsuites on Kraken with the two patches that you sent, and they make no difference to the testsuites. However, the test case that you mentioned was failing on Kraken before, so this is not a good validation. In
#393, Erik noticed that these were failing on Hopper which also uses the PGI compiler and upped the tolerance. I will update and rerun both sets of tests. -
reporter
- removed comment
Ian: when you do, would you mind using v2 of the patch (see above), please? The first version contained a bug where the size of the $OMP sentinel was not taken into account in the new line length calculation (which made it produce lines that were slightly too long when breaking up OMP "comment" lines).
-
- removed comment
I have rerun the test with an updated checkout and the test now passes on Kraken (since Erik's increase of the tolerance) both with and without the patches. All testsuites pass and fail in the same way with and without the patch, so I would say it is OK to commit it.
-
- removed comment
Yes, please commit it.
-
reporter
- removed comment
Very good. Thank you. I'd love to apply it myself, but I do not have commit rights to the Cactus repository (https://svn.cactuscode.org/flesh/trunk) it seems. Below is my intended commit message (for the v2 version of the patch!). If someone with commit rights would want to apply it.
OpenMP requires continuation lines in Fortran 90 (and possibly Fortran 77 as well though I have not checked that) to start with the OMP sentinel (https://computing.llnl.gov/tutorials/openMP/#Directives).
This patch ensures that a sentinel OMP or HPF is repeated when a line starting with that sentinel is broken to limit its length to 132 characters.
It also adds a case-insensitive flag to the check for "!" like comments and adds extra backslashes to the standard_comments regex to properly match $OMP after all metacharacters have been handled.
Tested with LoopControl's Fortran interface on gcc 4.5,Intel 11.1 and PGI.
-
- changed status to resolved
- removed comment
This patch has been applied by Frank Löffler.
-
reporter
- changed status to closed
- edited description
- Log in to comment
