* [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
[parent not found: <54F9FDEE.6090002@net-b.de>]
* 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).