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
