#928: openmp parallelization within Exact broken ------------------------------------+--------------------------------------- Reporter: knarf | Owner: Type: defect | Status: confirmed Priority: minor | Milestone: ET_2012_11 Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by rhaas):
The attached patch replaces the SAVE variable with THREADPRIVATE variables (ie. the per-thread equivalent). I also did this substitution in boost.F77 where currently a CRITICAL section is used. I believe that the way the the critical section was used does not properly address the race condition (and I think I sprinkled a couple such constructs through the Cactus code). The code and reasoning go like this:
{{{ logical,save :: firstcall = .true. integer,save :: somevalue
if (firstcall) then $omp critical if (firstcall) then somevalue = 42 firstcall = .false. end if $end critical end if }}}
with the aim in skipping the whole critical section once somevalue has been initialized and firstcall set to true on the first go. The two if statements on the same condition are seens as a first speculative one the allows a quick skip and only if we think we might have to do something (ie if firstcall.eqv..true.) do we enter the expensive section of code. The critical section is intended to ensure we really only do so once.
The mistake now is that as far as I know, an optimizing compiler is free to actually execute the following code: {{{ logical,save :: firstcall = .true. integer,save :: somevalue
if (firstcall) then $omp critical if (firstcall) then ! order of statements reversed firstcall = .false. somevalue = 42 end if $end critical end if }}}
since there is no (visible to the compiler) ordering required. This can go wrong in the following way 1. thread 0 sets firstcall=.false. and flushes this value to memory. 1. before thread 0 sets somevalue, thread 1 comes along, encounters the outer if statement, finds that firstcall=.false. 1. thread 1 proceeds with the wrong value of somevalue.
There seems to be no way to force a compiler (C or Fortran) to *not* change a value temporarily (or to explicitly enforce an ordering on the statements). C's volatile attribute (think of using it to access a hardware register) seems the closest (and C also defines synchronization points where the compiler must ensure that all variable data is in memory, eg. before function calls, though optimization might do funny things as well if the compiler thinks it can be sure that a variable is not visible to anyone else). None of this helps with Fortran code of course.