Your patch fixes the issues. In attachment patch, test case and changelog. Thanks!
2015-12-07 11:06 GMT+01:00 Tobias Burnus <tobias.bur...@physik.fu-berlin.de>: > I wrote: >> I wonder whether using >> >> __asm__ __volatile__ ("":::"memory"); >> >> would be sufficient as it has a way lower overhead than >> __sync_synchronize(). > > Namely, something like the attached patch. > > Regarding the original patch submission: Is there a reason that you didn't > include the test case of Deepak from > https://gcc.gnu.org/ml/fortran/2015-04/msg00062.html > It should work as -fcoarray=lib -lcaf_single "dg-do run" test. > > Tobias
commit 69e650945454491bbaf81513a1eed10b5b6b0f45 Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it> Date: Mon Dec 7 15:46:10 2015 +0100 Introducing __asm__ __volatile__ (:::memory) after image control statements, send and get diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 21efe44..25ff311 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -1222,6 +1222,15 @@ gfc_conv_intrinsic_caf_get (gfc_se *se, gfc_expr *expr, tree lhs, tree lhs_kind, se->expr = res_var; if (array_expr->ts.type == BT_CHARACTER) se->string_length = argse.string_length; + + /* It guarantees memory consistency within the same segment */ + tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + gfc_add_expr_to_block (&se->pre, tmp); + } @@ -1390,6 +1399,15 @@ conv_caf_send (gfc_code *code) { gfc_add_expr_to_block (&block, tmp); gfc_add_block_to_block (&block, &lhs_se.post); gfc_add_block_to_block (&block, &rhs_se.post); + + /* It guarantees memory consistency within the same segment */ + tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + gfc_add_expr_to_block (&block, tmp); + return gfc_finish_block (&block); } diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 3df483a..b7e1faa 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -818,6 +818,15 @@ gfc_trans_lock_unlock (gfc_code *code, gfc_exec_op op) errmsg, errmsg_len); gfc_add_expr_to_block (&se.pre, tmp); + /* It guarantees memory consistency within the same segment */ + tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + + gfc_add_expr_to_block (&se.pre, tmp); + if (stat2 != NULL_TREE) gfc_add_modify (&se.pre, stat2, fold_convert (TREE_TYPE (stat2), stat)); @@ -995,6 +1004,14 @@ gfc_trans_event_post_wait (gfc_code *code, gfc_exec_op op) errmsg, errmsg_len); gfc_add_expr_to_block (&se.pre, tmp); + /* It guarantees memory consistency within the same segment */ + tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + gfc_add_expr_to_block (&se.pre, tmp); + if (stat2 != NULL_TREE) gfc_add_modify (&se.pre, stat2, fold_convert (TREE_TYPE (stat2), stat)); @@ -1080,6 +1097,18 @@ 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 = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + gfc_add_expr_to_block (&se.pre, tmp); + } + if (flag_coarray != GFC_FCOARRAY_LIB) { /* Set STAT to zero. */ diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index 001db41..1993743 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -746,6 +746,14 @@ gfc_allocate_using_lib (stmtblock_t * block, tree pointer, tree size, TREE_TYPE (pointer), pointer, fold_convert ( TREE_TYPE (pointer), tmp)); gfc_add_expr_to_block (block, tmp); + + /* It guarantees memory consistency within the same segment */ + tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + gfc_add_expr_to_block (block, tmp); } @@ -1356,6 +1364,14 @@ gfc_deallocate_with_status (tree pointer, tree status, tree errmsg, token, pstat, errmsg, errlen); gfc_add_expr_to_block (&non_null, tmp); + /* It guarantees memory consistency within the same segment */ + tmp = gfc_build_string_const (strlen ("memory")+1, "memory"), + tmp = build5_loc (input_location, ASM_EXPR, void_type_node, + gfc_build_string_const (1, ""), NULL_TREE, NULL_TREE, + tree_cons (NULL_TREE, tmp, NULL_TREE), NULL_TREE); + ASM_VOLATILE_P (tmp) = 1; + gfc_add_expr_to_block (&non_null, tmp); + if (status != NULL_TREE) { tree stat = build_fold_indirect_ref_loc (input_location, status); diff --git a/gcc/testsuite/gfortran.dg/coarray_40.f90 b/gcc/testsuite/gfortran.dg/coarray_40.f90 new file mode 100644 index 0000000..84af50e --- /dev/null +++ b/gcc/testsuite/gfortran.dg/coarray_40.f90 @@ -0,0 +1,25 @@ +! { dg-do run } +! { dg-options "-fcoarray=lib -lcaf_single" } +! +! Run-time test for memory consistency +! +! Contributed by Deepak Eachempati + +program cp_bug + implicit none + integer :: v1, v2, u[*] + integer :: me + + me = this_image() + + u = 0 + v1 = 10 + + v1 = u[me] + + ! v2 should get value in u (0) + v2 = v1 + + if(v2 /= u) call abort() + +end program
commit 3c36054cbeb23353d9fd81a9101861c812af35d5 Author: Alessandro Fanfarillo <fanfari...@ing.uniroma2.it> Date: Mon Dec 7 15:15:45 2015 +0100 Introducing __asm__ __volatile__ (:::memory) after image control statements, send and get diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog index ba176a1..3f0cef0 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,15 @@ +2015-12-07 Tobias Burnus <bur...@net-b.de> + Alessandro Fanfarillo <fanfarillo....@gmail.com> + + * trans.c (gfc_allocate_using_lib,gfc_deallocate_with_status): + Introducing __asm__ __volatile__ ("":::"memory") + after image control statements. + * trans-stmt.c (gfc_trans_sync,gfc_trans_event_post_wait) + (gfc_trans_lock_unlock): Ditto. + * trans-intrinsic.c (gfc_conv_intrinsic_caf_get, + conv_caf_send): Introducing __asm__ __volatile__ ("":::"memory") + after send, get and sendget. + 2015-12-05 Paul Thomas <pa...@gcc.gnu.org> PR fortran/68676 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 188ed2b..0ac8836 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2015-12-07 Tobias Burnus <bur...@net-b.de> + Alessandro Fanfarillo <fanfarillo....@gmail.com> + + * gfortran.dg/coarray_40.f90: New. + 2015-12-07 Kirill Yukhin <kirill.yuk...@intel.com> PR target/68627