[Einstein Toolkit] #388: handling of OMP continuation lines in f_file_processor.pl

List overview All Threads
Download

newer

older

[Einstein Toolkit] #338:...

[Einstein Toolkit] #398: --update...

Einstein Toolkit

5 Apr 2011 5 Apr '11

12:26 p.m.

#388: handling of OMP continuation lines in f_file_processor.pl --------------------+------------------------------------------------------- Reporter: rhaas | Owner: rhaas Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Keywords: | --------------------+------------------------------------------------------- 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).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Show replies by date

Einstein Toolkit

5 Apr 5 Apr

12:26 p.m.

#388: handling of OMP continuation lines in f_file_processor.pl ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: rhaas Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: Resolution: | Keywords: ---------------------+------------------------------------------------------ Changes (by rhaas):

* status: new => review

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:1 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

12:38 p.m.

Comment (by knarf):

The patch looks good to me (didn't test it).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:2 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

1:05 p.m.

Comment (by rhaas):

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).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:3 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

2:15 p.m.

Comment (by hinder):

I can test it on Kraken. Is there a testsuite that should be run, or some other parameter file?

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:4 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3:32 p.m.

Comment (by rhaas):

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 }}}

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:5 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

8 Apr 8 Apr

4:51 a.m.

Comment (by hinder):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:6 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5:51 a.m.

Comment (by rhaas):

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).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:7 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

11 Apr 11 Apr

10:41 a.m.

Comment (by hinder):

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.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:8 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

11:01 a.m.

Comment (by eschnett):

Yes, please commit it.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:9 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

12:27 p.m.

Comment (by rhaas):

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. }}}

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:10 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

13 Apr 13 Apr

1:36 p.m.

#388: handling of OMP continuation lines in f_file_processor.pl ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: rhaas Type: defect | Status: closed Priority: minor | Milestone: Component: Cactus | Version: Resolution: fixed | Keywords: ---------------------+------------------------------------------------------ Changes (by eschnett):

* status: review => closed * resolution: => fixed

Comment:

This patch has been applied by Frank Löffler.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/388#comment:11 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

5658

Age (days ago)

5666

Last active (days ago)

trac@lists.einsteintoolkit.org

11 comments

1 participants

tags (0)

participants (1)

Einstein Toolkit