On Oct 30, 2013, at 7:24 , Ian Hinder ian.hinder@aei.mpg.de wrote:

On 30 Oct 2013, at 01:53, Erik Schnetter schnetter@cct.lsu.edu wrote:

...

I have just found several instances of "OpenMP parallel do" constructs in GRHydro that did not mark loop variables as private. This is a severe parallelization error that can lead to wrong results in ways that are very difficult to debug.

I wonder why the compiler does not flag this. I suggest a code review before the release.

While it is true that the loop index of the loop that is parallelized (usually only the outermost loop) does, in Fortran, not have to be declared as private, all other loop indices still have to be declared as private. In C, all loop indices have to be declared as private. Personally, I find it simplest to just declare all loop indices as private.

This is correct: !$OMP PARALLEL DO PRIVATE(i,j,k) do k=1,nz do j=1,ny do i=1,nx

And this would be wrong: !$OMP PARALLEL DO do k=1,nz do j=1,ny do i=1,nx

Surely this would give results so wrong that they would be noticed? Is it possible that the compiler is automatically declaring the variables as private, without emitting a warning? Are the routines with the bugs actually used for physics yet, and have they been validated?

Without a flush statement (or a statement that implies a flush, such as e.g. a barrier), there is no guarantee that shared variables are actually communicated. In a simple loop, the compiler may decided to privatize the variables, and to communicate them only when required, e.g. by keeping them in registers. In this case, the code would happen to work correctly.

-erik

PS: #include <usual rant about lack of a reproducible correctness-testing framework for the Einstein Toolkit>

-- Ian Hinder http://numrel.aei.mpg.de/people/hinder

On Wed, Oct 30, 2013 at 08:54:08AM -0400, Yosef Zlochower wrote:

I though that for C code, the (outermost) loop index was automatically private. Has this changed in the openmp standard?

I was under the same impression, and this may well be the case, but I found this statement about automatic private variables only within the Fortran section of the 'for' construct, not the C section.

Having said that, assume a construct like:

#pragma omp parallel for for (int i=0; i<N; i++) { }

Can you here actually specify private(i)? 'i' isn't even declared at the point of the pragma yet.

Further assuming that you use the same syntax (inline declaration) for inner loops, you should be fine:

#pragma omp parallel for for (int i=0; i<N; i++) for (int j=0; j<N; j++) { }

Frank

On Oct 30, 2013, at 16:49 , Roland Haas roland.haas@physics.gatech.edu wrote:

Signed PGP part Hello all,

...

On Wed, Oct 30, 2013 at 08:54:08AM -0400, Yosef Zlochower wrote:

...

I though that for C code, the (outermost) loop index was automatically private. Has this changed in the openmp standard?

Philipp and I did some more reading of the OpenMP specs (http://www.openmp.org/mp-documents/spec30.pdf%E2%80%8E) and while we could not find a helpful definition of "associated loops" (since those are the ones whose controlling variables are automatically private in FOTRAN) we *did* find an example in the specs that does exactly what we do. Example A.7.1f (on page 168) has: $OMP DO DO 300 I = 1,10 DO 300 J = 1,10 CALL WORK(I,J) 300 CONTINUE !$OMP ENDDO which would indicate that that *both* nested loops are seen as one and that both i and j are private. Note that the 2.0 specs (which have no COLLAPSE yet) also speak of do_loops and inner and outer loops so this all points towards the standard requiring that all nested loops in FORTRAN have private loop variables.

In this case, i and j are both private since the OpenMP parallel construct is in the routine calling this routine, so that the routine-local declarations of i and j make them private (since declared inside the parallel region). This has nothing to do with the loops; only the i loop is parallelized, while the j loop is not.

However, I stand corrected: Apparently, in Fortran all loop indices are private, as described in the example following the one you mention.

-erik

HOWEVER we certainly should NOT do this in C since no such guarantee is given there and I believe we SHOULD do as Erik suggested and explicitly declare all loop control variables as private (in FORTRAN) since the standard is confusing and possibly misleading. Appendix A.8 of the specs give more information on this.

...

Having said that, assume a construct like:

#pragma omp parallel for for (int i=0; i<N; i++) { }

Can you here actually specify private(i)? 'i' isn't even declared at the point of the pragma yet.

This is fine and actually the recommended method since it declares "i" inside of the parallel region so it is automatically private. It is (up to the lifetime of "i" after the loop ends) the same as: #pragma omp parallel for { int i; for (i=0; i<N; i++) { } }

Yours, Roland

-- My email is as private as my paper mail. I therefore support encrypting and signing email messages. Get my PGP key from http://keys.gnupg.net.

---

Users mailing list Users@einsteintoolkit.org http://lists.einsteintoolkit.org/mailman/listinfo/users