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

[Tobias Burnus <burnus@net-b.de>]

Alessandro Fanfarillo wrote:
> in attachment the new patch. I also checked the behavior with
> move_alloc: it synchronizes right after the deregistration of the
> destination.
> I also noticed that __asm__ __volatile__ ("":::"memory") is called
> before sync all and not after. It shouldn't be a problem, right?

The patch looks mostly good to me, except:

> @@ -1222,6 +1230,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); */
> +
>   }

Do not add out-commented code. Please remove.


>  gfc_trans_critical (gfc_code *code)
>  {
>    stmtblock_t block;
>    tree tmp, token = NULL_TREE;
>
>    gfc_start_block (&block);
>
>    if (flag_coarray == GFC_FCOARRAY_LIB)
>      {
>        token = gfc_get_symbol_decl (code->resolved_sym);
>        token = GFC_TYPE_ARRAY_CAF_TOKEN (TREE_TYPE (token));
>        tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_lock, 7,
>                                  token, integer_zero_node, 
> integer_one_node,
>                                  null_pointer_node, null_pointer_node,
>                                  null_pointer_node, integer_zero_node);
>        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);
> +
>    tmp = gfc_trans_code (code->block->next);
>    gfc_add_expr_to_block (&block, tmp);
>
>    if (flag_coarray == GFC_FCOARRAY_LIB)
>      {
>        tmp = build_call_expr_loc (input_location, 
> gfor_fndecl_caf_unlock, 6,
>                                  token, integer_zero_node, 
> integer_one_node,
>                                  null_pointer_node, null_pointer_node,
>                                  integer_zero_node);
>        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);
>
>    return gfc_finish_block (&block);
>  }

Please move the two new code additions into the associated "if 
(flag_coarray == GFC_FCOARRAY_LIB)" blocks - and keep an eye on the 
indentation: for the current code location, the second block is wrongly 
idented.


With the two issues fixed: LGTM.

Cheers,

Tobias

PS: I assume you can still commit yourself. I am asking because Steve 
did commit the recent EVENT patch for you.