* [PATCH, Fortran] Sync memory action delegated to OpenCoarrays
@ 2015-03-06 18:38 Alessandro Fanfarillo
2015-03-06 20:01 ` Tobias Burnus
[not found] ` <54F9FDEE.6090002@net-b.de>
0 siblings, 2 replies; 5+ messages in thread
From: Alessandro Fanfarillo @ 2015-03-06 18:38 UTC (permalink / raw)
To: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 325 bytes --]
Dear all,
so far a "sync memory" statement is translated into a local
"__sync_synchronize ()".
The attached draft patch delegates the action for sync memory (when
-fcoarray=lib is used) to the external function
_gfortran_caf_sync_memory() implemented in the OpenCoarrays library.
Any suggestions?
Best regards
Alessandro
[-- Attachment #2: raw-patch-sync_memory.diff --]
[-- Type: text/plain, Size: 3907 bytes --]
commit fdb560e3f548fe60edf26717b8de6bb92f3fd555
Author: Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>
Date: Fri Mar 6 10:25:56 2015 -0800
Sync memory is translated into an invokation to _gfortran_caf_sync_memory()
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 3664824..a66c25c 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -153,6 +153,7 @@ tree gfor_fndecl_caf_get;
tree gfor_fndecl_caf_send;
tree gfor_fndecl_caf_sendget;
tree gfor_fndecl_caf_sync_all;
+tree gfor_fndecl_caf_sync_memory;
tree gfor_fndecl_caf_sync_images;
tree gfor_fndecl_caf_error_stop;
tree gfor_fndecl_caf_error_stop_str;
@@ -3451,6 +3452,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX("caf_sync_all")), ".WW", void_type_node,
3, pint_type, pchar_type_node, integer_type_node);
+ gfor_fndecl_caf_sync_memory = gfc_build_library_function_decl_with_spec (
+ get_identifier (PREFIX("caf_sync_memory")), ".WW", void_type_node,
+ 3, pint_type, pchar_type_node, integer_type_node);
+
gfor_fndecl_caf_sync_images = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("caf_sync_images")), ".RRWW", void_type_node,
5, integer_type_node, pint_type, pint_type,
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 505f905..dce0e87 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -768,8 +768,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
else
stat = null_pointer_node;
- if (code->expr3 && flag_coarray == GFC_FCOARRAY_LIB
- && type != EXEC_SYNC_MEMORY)
+ if (code->expr3 && flag_coarray == GFC_FCOARRAY_LIB)
{
gcc_assert (code->expr3->expr_type == EXPR_VARIABLE);
gfc_init_se (&argse, NULL);
@@ -778,7 +777,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
errmsg = gfc_build_addr_expr (NULL, argse.expr);
errmsglen = argse.string_length;
}
- else if (flag_coarray == GFC_FCOARRAY_LIB && type != EXEC_SYNC_MEMORY)
+ else if (flag_coarray == GFC_FCOARRAY_LIB)
{
errmsg = null_pointer_node;
errmsglen = build_int_cst (integer_type_node, 0);
@@ -822,13 +821,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
gfc_add_expr_to_block (&se.pre, tmp);
}
- if (flag_coarray != GFC_FCOARRAY_LIB || type == EXEC_SYNC_MEMORY)
+ if (flag_coarray != GFC_FCOARRAY_LIB)
{
/* Set STAT to zero. */
if (code->expr2)
gfc_add_modify (&se.pre, stat, build_int_cst (TREE_TYPE (stat), 0));
}
- else if (type == EXEC_SYNC_ALL)
+ else if (type == EXEC_SYNC_ALL || type == EXEC_SYNC_MEMORY)
{
/* SYNC ALL => stat == null_pointer_node
SYNC ALL(stat=s) => stat has an integer type
@@ -840,8 +839,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
if (TREE_TYPE (stat) == integer_type_node)
stat = gfc_build_addr_expr (NULL, stat);
- tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
- 3, stat, errmsg, errmsglen);
+ if(type == EXEC_SYNC_MEMORY)
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_memory,
+ 3, stat, errmsg, errmsglen);
+ else
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+ 3, stat, errmsg, errmsglen);
+
gfc_add_expr_to_block (&se.pre, tmp);
}
else
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index bd1520a..3ba2f88 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -731,6 +731,7 @@ extern GTY(()) tree gfor_fndecl_caf_get;
extern GTY(()) tree gfor_fndecl_caf_send;
extern GTY(()) tree gfor_fndecl_caf_sendget;
extern GTY(()) tree gfor_fndecl_caf_sync_all;
+extern GTY(()) tree gfor_fndecl_caf_sync_memory;
extern GTY(()) tree gfor_fndecl_caf_sync_images;
extern GTY(()) tree gfor_fndecl_caf_error_stop;
extern GTY(()) tree gfor_fndecl_caf_error_stop_str;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays
2015-03-06 18:38 [PATCH, Fortran] Sync memory action delegated to OpenCoarrays Alessandro Fanfarillo
@ 2015-03-06 20:01 ` Tobias Burnus
[not found] ` <54F9FDEE.6090002@net-b.de>
1 sibling, 0 replies; 5+ messages in thread
From: Tobias Burnus @ 2015-03-06 20:01 UTC (permalink / raw)
To: Alessandro Fanfarillo, gfortran, gcc-patches
Dear Alessandro,
Alessandro Fanfarillo wrote:
> so far a "sync memory" statement is translated into a local
> "__sync_synchronize ()".
> The attached draft patch delegates the action for sync memory (when
> -fcoarray=lib is used) to the external function
> _gfortran_caf_sync_memory() implemented in the OpenCoarrays library.
Looks good to me. However, you should add a test case with 'dg-options
"-fdump-tree-original -fcoarray=lib"' to check that this works; cf.
gcc/testsuite/gfortran.dg/coarray*.f90 for examples.
And you have to provide a stub implementation in
libgfortran/caf/{libcaf.h,single.c}.
Tobias
PS: I wonder whether it makes sense to remove the __sync_synchronize for
-fcoarray=single and replace it by the equivalent to "|asm volatile (""
: : : "memory")". It almost certainly does.
|
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays
[not found] ` <54F9FDEE.6090002@net-b.de>
@ 2015-03-10 3:48 ` Alessandro Fanfarillo
2015-03-10 7:20 ` Tobias Burnus
0 siblings, 1 reply; 5+ messages in thread
From: Alessandro Fanfarillo @ 2015-03-10 3:48 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]
Thanks for the quick response.
Patch built and regtested on x86_64-unknown-linux-gnu.
Currently the stub for sync memory in single.c invokes "asm volatile
("" : : : "memory")" but _gfortran_caf_sync_memory is not called when
the code is compiled with -fcoarray=single.
Please let me know if I have to change the patch for -fcoarray=single.
2015-03-06 11:20 GMT-08:00 Tobias Burnus <burnus@net-b.de>:
> Dear Alessandro,
>
> Alessandro Fanfarillo wrote:
>
> so far a "sync memory" statement is translated into a local
> "__sync_synchronize ()".
> The attached draft patch delegates the action for sync memory (when
> -fcoarray=lib is used) to the external function
> _gfortran_caf_sync_memory() implemented in the OpenCoarrays library.
>
>
> Looks good to me. However, you should add a test case with 'dg-options
> "-fdump-tree-original -fcoarray=lib"' to check that this works; cf.
> gcc/testsuite/gfortran.dg/coarray*.f90 for examples.
>
> And you have to provide a stub implementation in
> libgfortran/caf/{libcaf.h,single.c}.
>
> Tobias
>
> PS: I wonder whether it makes sense to remove the __sync_synchronize for
> -fcoarray=single and replace it by the equivalent to "asm volatile ("" : : :
> "memory")". It almost certainly does.
[-- Attachment #2: patch_sync_memory.diff --]
[-- Type: text/plain, Size: 5570 bytes --]
2015-03-09 Alessandro Fanfarillo fanfarillo.gcc@gmail.com
trans-decl.c: Add function declaration caf_sync_memory
trans.h: Ditto
trans-stmt.c: Add caf_sync_memory invocation
coarray_38.f90: Test case
single.c: caf_sync_memory implementation
libcaf.h: caf_sync_memory prototype
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 3664824..a66c25c 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -153,6 +153,7 @@ tree gfor_fndecl_caf_get;
tree gfor_fndecl_caf_send;
tree gfor_fndecl_caf_sendget;
tree gfor_fndecl_caf_sync_all;
+tree gfor_fndecl_caf_sync_memory;
tree gfor_fndecl_caf_sync_images;
tree gfor_fndecl_caf_error_stop;
tree gfor_fndecl_caf_error_stop_str;
@@ -3451,6 +3452,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX("caf_sync_all")), ".WW", void_type_node,
3, pint_type, pchar_type_node, integer_type_node);
+ gfor_fndecl_caf_sync_memory = gfc_build_library_function_decl_with_spec (
+ get_identifier (PREFIX("caf_sync_memory")), ".WW", void_type_node,
+ 3, pint_type, pchar_type_node, integer_type_node);
+
gfor_fndecl_caf_sync_images = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("caf_sync_images")), ".RRWW", void_type_node,
5, integer_type_node, pint_type, pint_type,
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 505f905..dce0e87 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -768,8 +768,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
else
stat = null_pointer_node;
- if (code->expr3 && flag_coarray == GFC_FCOARRAY_LIB
- && type != EXEC_SYNC_MEMORY)
+ if (code->expr3 && flag_coarray == GFC_FCOARRAY_LIB)
{
gcc_assert (code->expr3->expr_type == EXPR_VARIABLE);
gfc_init_se (&argse, NULL);
@@ -778,7 +777,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
errmsg = gfc_build_addr_expr (NULL, argse.expr);
errmsglen = argse.string_length;
}
- else if (flag_coarray == GFC_FCOARRAY_LIB && type != EXEC_SYNC_MEMORY)
+ else if (flag_coarray == GFC_FCOARRAY_LIB)
{
errmsg = null_pointer_node;
errmsglen = build_int_cst (integer_type_node, 0);
@@ -822,13 +821,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
gfc_add_expr_to_block (&se.pre, tmp);
}
- if (flag_coarray != GFC_FCOARRAY_LIB || type == EXEC_SYNC_MEMORY)
+ if (flag_coarray != GFC_FCOARRAY_LIB)
{
/* Set STAT to zero. */
if (code->expr2)
gfc_add_modify (&se.pre, stat, build_int_cst (TREE_TYPE (stat), 0));
}
- else if (type == EXEC_SYNC_ALL)
+ else if (type == EXEC_SYNC_ALL || type == EXEC_SYNC_MEMORY)
{
/* SYNC ALL => stat == null_pointer_node
SYNC ALL(stat=s) => stat has an integer type
@@ -840,8 +839,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
if (TREE_TYPE (stat) == integer_type_node)
stat = gfc_build_addr_expr (NULL, stat);
- tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
- 3, stat, errmsg, errmsglen);
+ if(type == EXEC_SYNC_MEMORY)
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_memory,
+ 3, stat, errmsg, errmsglen);
+ else
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+ 3, stat, errmsg, errmsglen);
+
gfc_add_expr_to_block (&se.pre, tmp);
}
else
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index bd1520a..3ba2f88 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -731,6 +731,7 @@ extern GTY(()) tree gfor_fndecl_caf_get;
extern GTY(()) tree gfor_fndecl_caf_send;
extern GTY(()) tree gfor_fndecl_caf_sendget;
extern GTY(()) tree gfor_fndecl_caf_sync_all;
+extern GTY(()) tree gfor_fndecl_caf_sync_memory;
extern GTY(()) tree gfor_fndecl_caf_sync_images;
extern GTY(()) tree gfor_fndecl_caf_error_stop;
extern GTY(()) tree gfor_fndecl_caf_error_stop_str;
diff --git a/gcc/testsuite/gfortran.dg/coarray_38.f90 b/gcc/testsuite/gfortran.dg/coarray_38.f90
new file mode 100644
index 0000000..01b5b31
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_38.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! { dg-options "-fcoarray=lib -fdump-tree-original" }
+!
+! Coarray sync memory managed by external library
+!
+implicit none
+integer :: n
+sync memory
+end
+! { dg-final { scan-tree-dump-times "_gfortran_caf_sync_memory \\(0B, 0B, 0\\);" 1 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }
diff --git a/libgfortran/caf/libcaf.h b/libgfortran/caf/libcaf.h
index bd5c9c6..27e0d62 100644
--- a/libgfortran/caf/libcaf.h
+++ b/libgfortran/caf/libcaf.h
@@ -99,6 +99,7 @@ void *_gfortran_caf_register (size_t, caf_register_t, caf_token_t *, int *,
char *, int);
void _gfortran_caf_deregister (caf_token_t *, int *, char *, int);
+void _gfortran_caf_sync_memory(int *, char *, int);
void _gfortran_caf_sync_all (int *, char *, int);
void _gfortran_caf_sync_images (int, int[], int *, char *, int);
diff --git a/libgfortran/caf/single.c b/libgfortran/caf/single.c
index 7405c91..8eb61c3 100644
--- a/libgfortran/caf/single.c
+++ b/libgfortran/caf/single.c
@@ -156,6 +156,17 @@ _gfortran_caf_deregister (caf_token_t *token, int *stat,
*stat = 0;
}
+void
+_gfortran_caf_sync_memory(int *stat,
+ char *errmsg __attribute__ ((unused)),
+ int errmsg_len __attribute__ ((unused)))
+{
+ if (stat)
+ *stat = 0;
+
+ asm volatile("" ::: "memory");
+}
+
void
_gfortran_caf_sync_all (int *stat,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays
2015-03-10 3:48 ` Alessandro Fanfarillo
@ 2015-03-10 7:20 ` Tobias Burnus
2015-03-10 15:30 ` Alessandro Fanfarillo
0 siblings, 1 reply; 5+ messages in thread
From: Tobias Burnus @ 2015-03-10 7:20 UTC (permalink / raw)
To: Alessandro Fanfarillo; +Cc: gfortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2967 bytes --]
Hi Alessandro,
Alessandro Fanfarillo wrote:
> Thanks for the quick response. Patch built and regtested on x86_64-unknown-linux-gnu.
Actually, I also was about to send a reworked patch myself - see
attachment. Additional changes compared to your last patch:
* Add the assembler statement to libcaf_single also for the other syncs.
* Tested also the passing of the other arguments in the test case.
* Removed the call to sync_synchronize in the compiler - leaving it
completely to the library. (Existed for end of program, STOP and SYNC
...; was only issued with -fcoarray=lib)
Are the changes okay from your side? Then I'd prefer my patch - and I'll
commit it this evening.
* * *
> Currently the stub for sync memory in single.c invokes "asm volatile
> ("" : : : "memory")" but _gfortran_caf_sync_memory is not called when
> the code is compiled with -fcoarray=single.
I misremembered that we do call __sync_synchronize for -fcoarray=single.
As it turned out, we only call it for -fcoarray=lib - and it makes sense
(in my opinion) to leave such calls to the library.
As you, I also added the "asm" to the library - which probably has no
effect - except when the library and the program are both compiled with
LTO. Still, it somehow looks more complete.
Regarding: Looks good to me - except that I prefer the additional items
of my patch - and that the ChangeLog doesn't conform to the GCC style:
- The file names shall be relative to the change log, i.e.
"caf/single.c" (as the ChangeLog file is in libgfortran/ChangeLog),
similarly a "gfortran.dg/" is missing for the test case.
- There should be a single "<tab>* " before the file names, not two tabs
and no "* ".
- After the file name, the changed symbols/function names shall be put
in parentheses.
- The item describing the change should end with a full stop.
- The email address should be enclosed in <>.
(Often missed, but not by you: date-name-email separation is two spaces.)
Tobias
> Please let me know if I have to change the patch for -fcoarray=single.
>
>
>
> 2015-03-06 11:20 GMT-08:00 Tobias Burnus <burnus@net-b.de>:
>> Dear Alessandro,
>>
>> Alessandro Fanfarillo wrote:
>>
>> so far a "sync memory" statement is translated into a local
>> "__sync_synchronize ()".
>> The attached draft patch delegates the action for sync memory (when
>> -fcoarray=lib is used) to the external function
>> _gfortran_caf_sync_memory() implemented in the OpenCoarrays library.
>>
>>
>> Looks good to me. However, you should add a test case with 'dg-options
>> "-fdump-tree-original -fcoarray=lib"' to check that this works; cf.
>> gcc/testsuite/gfortran.dg/coarray*.f90 for examples.
>>
>> And you have to provide a stub implementation in
>> libgfortran/caf/{libcaf.h,single.c}.
>>
>> Tobias
>>
>> PS: I wonder whether it makes sense to remove the __sync_synchronize for
>> -fcoarray=single and replace it by the equivalent to "asm volatile ("" : : :
>> "memory")". It almost certainly does.
[-- Attachment #2: syncmem.diff --]
[-- Type: text/x-patch, Size: 8217 bytes --]
2015-03-09 Alessandro Fanfarillo fanfarillo.gcc@gmail.com
Tobias Burnus <burnus@net-b.de>
* trans.h (caf_sync_memory): New function decl tree.
* trans-decl.c (gfc_build_builtin_function_decls): Define it.
(create_main_function): Don't call sync_synchronize and leave
it to the CAF library.
* trans-stmt.c (gfc_trans_stop): Ditto.
(gfc_trans_sync): Ditto; add call library call for sync memory.
* coarray_sync_memory.f90: New.
* caf/libcaf.h (_gfortran_caf_sync_memory): New prototype.
* caf/single.c (_gfortran_caf_sync_memory): Implement.
(_gfortran_caf_sync_all, _gfortran_caf_sync_image): Add
__asm__ __volatile___ ("":::"memory").
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 3664824..769d487 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -153,6 +153,7 @@ tree gfor_fndecl_caf_get;
tree gfor_fndecl_caf_send;
tree gfor_fndecl_caf_sendget;
tree gfor_fndecl_caf_sync_all;
+tree gfor_fndecl_caf_sync_memory;
tree gfor_fndecl_caf_sync_images;
tree gfor_fndecl_caf_error_stop;
tree gfor_fndecl_caf_error_stop_str;
@@ -3451,6 +3452,10 @@ gfc_build_builtin_function_decls (void)
get_identifier (PREFIX("caf_sync_all")), ".WW", void_type_node,
3, pint_type, pchar_type_node, integer_type_node);
+ gfor_fndecl_caf_sync_memory = gfc_build_library_function_decl_with_spec (
+ get_identifier (PREFIX("caf_sync_memory")), ".WW", void_type_node,
+ 3, pint_type, pchar_type_node, integer_type_node);
+
gfor_fndecl_caf_sync_images = gfc_build_library_function_decl_with_spec (
get_identifier (PREFIX("caf_sync_images")), ".RRWW", void_type_node,
5, integer_type_node, pint_type, pint_type,
@@ -5583,12 +5588,6 @@ create_main_function (tree fndecl)
/* Coarray: Call _gfortran_caf_finalize(void). */
if (flag_coarray == GFC_FCOARRAY_LIB)
{
- /* Per F2008, 8.5.1 END of the main program implies a
- SYNC MEMORY. */
- tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
- tmp = build_call_expr_loc (input_location, tmp, 0);
- gfc_add_expr_to_block (&body, tmp);
-
tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_finalize, 0);
gfc_add_expr_to_block (&body, tmp);
}
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 505f905..6450a0e 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -639,17 +639,6 @@ gfc_trans_stop (gfc_code *code, bool error_stop)
gfc_init_se (&se, NULL);
gfc_start_block (&se.pre);
- if (flag_coarray == GFC_FCOARRAY_LIB && !error_stop)
- {
- /* Per F2008, 8.5.1 STOP implies a SYNC MEMORY. */
- tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
- tmp = build_call_expr_loc (input_location, tmp, 0);
- gfc_add_expr_to_block (&se.pre, tmp);
-
- tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_finalize, 0);
- gfc_add_expr_to_block (&se.pre, tmp);
- }
-
if (code->expr1 == NULL)
{
tmp = build_int_cst (gfc_int4_type_node, 0);
@@ -768,8 +757,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
else
stat = null_pointer_node;
- if (code->expr3 && flag_coarray == GFC_FCOARRAY_LIB
- && type != EXEC_SYNC_MEMORY)
+ if (code->expr3 && flag_coarray == GFC_FCOARRAY_LIB)
{
gcc_assert (code->expr3->expr_type == EXPR_VARIABLE);
gfc_init_se (&argse, NULL);
@@ -778,7 +766,7 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
errmsg = gfc_build_addr_expr (NULL, argse.expr);
errmsglen = argse.string_length;
}
- else if (flag_coarray == GFC_FCOARRAY_LIB && type != EXEC_SYNC_MEMORY)
+ else if (flag_coarray == GFC_FCOARRAY_LIB)
{
errmsg = null_pointer_node;
errmsglen = build_int_cst (integer_type_node, 0);
@@ -813,22 +801,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
fold_convert (integer_type_node, images));
}
- /* Per F2008, 8.5.1, a SYNC MEMORY is implied by calling the
- image control statements SYNC IMAGES and SYNC ALL. */
- if (flag_coarray == GFC_FCOARRAY_LIB)
- {
- tmp = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
- tmp = build_call_expr_loc (input_location, tmp, 0);
- gfc_add_expr_to_block (&se.pre, tmp);
- }
-
- if (flag_coarray != GFC_FCOARRAY_LIB || type == EXEC_SYNC_MEMORY)
+ if (flag_coarray != GFC_FCOARRAY_LIB)
{
/* Set STAT to zero. */
if (code->expr2)
gfc_add_modify (&se.pre, stat, build_int_cst (TREE_TYPE (stat), 0));
}
- else if (type == EXEC_SYNC_ALL)
+ else if (type == EXEC_SYNC_ALL || type == EXEC_SYNC_MEMORY)
{
/* SYNC ALL => stat == null_pointer_node
SYNC ALL(stat=s) => stat has an integer type
@@ -840,8 +819,13 @@ gfc_trans_sync (gfc_code *code, gfc_exec_op type)
if (TREE_TYPE (stat) == integer_type_node)
stat = gfc_build_addr_expr (NULL, stat);
- tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
- 3, stat, errmsg, errmsglen);
+ if(type == EXEC_SYNC_MEMORY)
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_memory,
+ 3, stat, errmsg, errmsglen);
+ else
+ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+ 3, stat, errmsg, errmsglen);
+
gfc_add_expr_to_block (&se.pre, tmp);
}
else
diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h
index bd1520a..3ba2f88 100644
--- a/gcc/fortran/trans.h
+++ b/gcc/fortran/trans.h
@@ -731,6 +731,7 @@ extern GTY(()) tree gfor_fndecl_caf_get;
extern GTY(()) tree gfor_fndecl_caf_send;
extern GTY(()) tree gfor_fndecl_caf_sendget;
extern GTY(()) tree gfor_fndecl_caf_sync_all;
+extern GTY(()) tree gfor_fndecl_caf_sync_memory;
extern GTY(()) tree gfor_fndecl_caf_sync_images;
extern GTY(()) tree gfor_fndecl_caf_error_stop;
extern GTY(()) tree gfor_fndecl_caf_error_stop_str;
diff --git a/gcc/testsuite/gfortran.dg/coarray_sync_memory.f90 b/gcc/testsuite/gfortran.dg/coarray_sync_memory.f90
new file mode 100644
index 0000000..6e1aee3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_sync_memory.f90
@@ -0,0 +1,19 @@
+! { dg-do compile }
+! { dg-options "-fdump-tree-original -fcoarray=lib" }
+!
+! Coarray sync memory managed by the external library
+!
+implicit none
+integer :: stat
+character(len=42) :: msg
+sync memory
+sync memory(stat=stat)
+sync memory(errmsg=msg)
+sync memory(errmsg=msg, stat=stat)
+end
+
+! { dg-final { scan-tree-dump-times "_gfortran_caf_sync_memory \\(0B, 0B, 0\\);" 1 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_sync_memory \\(&stat, 0B, 0\\);" 1 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_sync_memory \\(0B, &&msg, 42\\);" 1 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_sync_memory \\(&stat, &&msg, 42\\);" 1 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }
diff --git a/libgfortran/caf/libcaf.h b/libgfortran/caf/libcaf.h
index bd5c9c6..660bd7c 100644
--- a/libgfortran/caf/libcaf.h
+++ b/libgfortran/caf/libcaf.h
@@ -100,6 +100,7 @@ void *_gfortran_caf_register (size_t, caf_register_t, caf_token_t *, int *,
void _gfortran_caf_deregister (caf_token_t *, int *, char *, int);
void _gfortran_caf_sync_all (int *, char *, int);
+void _gfortran_caf_sync_memory (int *, char *, int);
void _gfortran_caf_sync_images (int, int[], int *, char *, int);
void _gfortran_caf_error_stop_str (const char *, int32_t)
diff --git a/libgfortran/caf/single.c b/libgfortran/caf/single.c
index 7405c91..daef281 100644
--- a/libgfortran/caf/single.c
+++ b/libgfortran/caf/single.c
@@ -162,6 +162,18 @@ _gfortran_caf_sync_all (int *stat,
char *errmsg __attribute__ ((unused)),
int errmsg_len __attribute__ ((unused)))
{
+ __asm__ __volatile__ ("":::"memory");
+ if (stat)
+ *stat = 0;
+}
+
+
+void
+_gfortran_caf_sync_memory (int *stat,
+ char *errmsg __attribute__ ((unused)),
+ int errmsg_len __attribute__ ((unused)))
+{
+ __asm__ __volatile__ ("":::"memory");
if (stat)
*stat = 0;
}
@@ -186,6 +198,7 @@ _gfortran_caf_sync_images (int count __attribute__ ((unused)),
}
#endif
+ __asm__ __volatile__ ("":::"memory");
if (stat)
*stat = 0;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays
2015-03-10 7:20 ` Tobias Burnus
@ 2015-03-10 15:30 ` Alessandro Fanfarillo
0 siblings, 0 replies; 5+ messages in thread
From: Alessandro Fanfarillo @ 2015-03-10 15:30 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gfortran, gcc-patches
Totally fine with me. Thanks.
2015-03-10 0:20 GMT-07:00 Tobias Burnus <burnus@net-b.de>:
> Hi Alessandro,
>
> Alessandro Fanfarillo wrote:
>>
>> Thanks for the quick response. Patch built and regtested on
>> x86_64-unknown-linux-gnu.
>
>
> Actually, I also was about to send a reworked patch myself - see attachment.
> Additional changes compared to your last patch:
> * Add the assembler statement to libcaf_single also for the other syncs.
> * Tested also the passing of the other arguments in the test case.
> * Removed the call to sync_synchronize in the compiler - leaving it
> completely to the library. (Existed for end of program, STOP and SYNC ...;
> was only issued with -fcoarray=lib)
>
> Are the changes okay from your side? Then I'd prefer my patch - and I'll
> commit it this evening.
>
> * * *
>
>> Currently the stub for sync memory in single.c invokes "asm volatile
>> ("" : : : "memory")" but _gfortran_caf_sync_memory is not called when
>> the code is compiled with -fcoarray=single.
>
>
> I misremembered that we do call __sync_synchronize for -fcoarray=single. As
> it turned out, we only call it for -fcoarray=lib - and it makes sense (in my
> opinion) to leave such calls to the library.
>
> As you, I also added the "asm" to the library - which probably has no effect
> - except when the library and the program are both compiled with LTO. Still,
> it somehow looks more complete.
>
>
> Regarding: Looks good to me - except that I prefer the additional items of
> my patch - and that the ChangeLog doesn't conform to the GCC style:
> - The file names shall be relative to the change log, i.e. "caf/single.c"
> (as the ChangeLog file is in libgfortran/ChangeLog), similarly a
> "gfortran.dg/" is missing for the test case.
> - There should be a single "<tab>* " before the file names, not two tabs and
> no "* ".
> - After the file name, the changed symbols/function names shall be put in
> parentheses.
> - The item describing the change should end with a full stop.
> - The email address should be enclosed in <>.
> (Often missed, but not by you: date-name-email separation is two spaces.)
>
> Tobias
>
>
>> Please let me know if I have to change the patch for -fcoarray=single.
>>
>>
>>
>> 2015-03-06 11:20 GMT-08:00 Tobias Burnus <burnus@net-b.de>:
>>>
>>> Dear Alessandro,
>>>
>>> Alessandro Fanfarillo wrote:
>>>
>>> so far a "sync memory" statement is translated into a local
>>> "__sync_synchronize ()".
>>> The attached draft patch delegates the action for sync memory (when
>>> -fcoarray=lib is used) to the external function
>>> _gfortran_caf_sync_memory() implemented in the OpenCoarrays library.
>>>
>>>
>>> Looks good to me. However, you should add a test case with 'dg-options
>>> "-fdump-tree-original -fcoarray=lib"' to check that this works; cf.
>>> gcc/testsuite/gfortran.dg/coarray*.f90 for examples.
>>>
>>> And you have to provide a stub implementation in
>>> libgfortran/caf/{libcaf.h,single.c}.
>>>
>>> Tobias
>>>
>>> PS: I wonder whether it makes sense to remove the __sync_synchronize for
>>> -fcoarray=single and replace it by the equivalent to "asm volatile ("" :
>>> : :
>>> "memory")". It almost certainly does.
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-10 15:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-06 18:38 [PATCH, Fortran] Sync memory action delegated to OpenCoarrays Alessandro Fanfarillo
2015-03-06 20:01 ` Tobias Burnus
[not found] ` <54F9FDEE.6090002@net-b.de>
2015-03-10 3:48 ` Alessandro Fanfarillo
2015-03-10 7:20 ` Tobias Burnus
2015-03-10 15:30 ` Alessandro Fanfarillo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).