Re: [Fortran, Patch] Memory sync after coarray image control statements and assignment

[Tobias Burnus <tobias.burnus@physik.fu-berlin.de>]

Dear Alessandro, dear all,

On Mon, Dec 07, 2015 at 03:48:17PM +0100, Alessandro Fanfarillo wrote:
> Your patch fixes the issues. In attachment patch, test case and changelog.

Regarding the ChangeLog: Please include the added lines, only, and not the
change as patch. gcc/testsuite/ChangeLog changes too often such that a patch
won't apply.


Regarding the patch, I wonder where the memory synchronization makes sense
and where it is not required. (cf. also my email to Matthew in this thread,
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00828.html)


I think it should be after all image control statements (8.5.1 in
http://j3-fortran.org/doc/year/15/15-007r2.pdf):
SYNC ALL, SYNC IMAGES, SYNC MEMORY, ALLOCATE, DEALLOCATE,
CRITICAL ... END CRITICAL, EVENT POST, EVENT WAIT, LOCK, UNLOCK,
MOVE_ALLOC.

Thus:
- SYNC ..., ALLOCATE/DEALLOCATE: I think those are all handled by the
  current patch
- MOVE_ALLOC: This one should be handled via the internal (de)allocate
  handling (please check!)
- EVENT WAIT, CRITICAL and LOCK: Obtaining a lock or receiving an event
  implies that quite likely some other process has changed something
  before. For those, the assembler statement really has to be added.
- EVENT POST, UNLOCK and END CRITICAL: While those are image control
  statements, I do not see how a remote image could modify a value in
  a well defined way, which is guaranteed to be available after that
  statement - but might not yet be available already at the previous
  segment (i.e. the previous image control statement).

Hence: I think you should update the patch to also handle 
EVENT WAIT, CRITICAL and LOCK - and to check MOVE_ALLOC.



Additionally, we need to handle the alias issue of:
  var = 5
  var[this_image()] = 42
  tmp = var

Both _gfortran_caf_send and _gfortran_caf_sendget can modify the
value of a variable; thus, calling the assembler after the function
makes sense.


However, _gfortran_caf_get does not modify the associated variable;
adding the assembler statement *after* _gfortran_caf_get. The
question is, however, whether one needs to take care of 'flushing'
the variable before the _gfortran_caf_get:
   var = 7
   ...
   var = 5
   tmp = var[this_image()]
   result = var + tmp
Here, one needs to prevent the compiler of keeping "5" only in a
register or moving "var = 5" after the _gfortran_caf_get call.


Thus, you have to move the assembler statement before the library
call in _gfortran_caf_get - and add another one at the beginning
of _gfortran_caf_sendget.

(For send/get, might might come up with something better than
""::"memory", but for now, it should do.)

Cheers,

Tobias