| Message ID | 2c2c6f22-9482-699c-0088-7b00f8969611@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for Fortran optional arguments in OpenACC | expand |
On 12/07/2019 12:38 pm, Kwok Cheung Yeung wrote: > This patch fixes a similar situation that occurs with the use_device > clause, where the lowering would result in a null dereference if applied > to a non-present optional argument. This patch builds a conditional > check that skips the dereference if the argument is non-present, and > ensures that optional arguments are treated like references. With the updated patch in part 2 of this series, this patch can be simplified slightly by reducing the number of calls to omp_is_optional_argument. Kwok gcc/ * omp-low.c (lower_omp_target): For use_device clauses that take optional arguments, generate conditional statements to avoid dereferences of non-present arguments. --- gcc/omp-low.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 1c7b43b..9c40100 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -11636,9 +11636,40 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) tkind = GOMP_MAP_FIRSTPRIVATE_INT; type = TREE_TYPE (ovar); if (TREE_CODE (type) == ARRAY_TYPE) - var = build_fold_addr_expr (var); + { + var = build_fold_addr_expr (var); + gimplify_assign (x, var, &ilist); + } else { + tree opt_arg_label; + bool optional_arg_p + = TREE_CODE (TREE_TYPE (ovar)) == POINTER_TYPE + && omp_is_optional_argument (ovar); + + if (optional_arg_p) + { + tree null_label + = create_artificial_label (UNKNOWN_LOCATION); + tree notnull_label + = create_artificial_label (UNKNOWN_LOCATION); + opt_arg_label + = create_artificial_label (UNKNOWN_LOCATION); + tree new_x = copy_node (x); + gcond *cond = gimple_build_cond (EQ_EXPR, ovar, + null_pointer_node, + null_label, + notnull_label); + gimple_seq_add_stmt (&ilist, cond); + gimple_seq_add_stmt (&ilist, + gimple_build_label (null_label)); + gimplify_assign (new_x, null_pointer_node, &ilist); + gimple_seq_add_stmt (&ilist, + gimple_build_goto (opt_arg_label)); + gimple_seq_add_stmt (&ilist, + gimple_build_label (notnull_label)); + } + if (omp_is_reference (ovar)) { type = TREE_TYPE (type); @@ -11646,8 +11677,12 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) var = build_simple_mem_ref (var); var = fold_convert (TREE_TYPE (x), var); } + + gimplify_assign (x, var, &ilist); + if (optional_arg_p) + gimple_seq_add_stmt (&ilist, + gimple_build_label (opt_arg_label)); } - gimplify_assign (x, var, &ilist); s = size_int (0); purpose = size_int (map_idx++); CONSTRUCTOR_APPEND_ELT (vsize, purpose, s); @@ -11829,11 +11864,43 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) { tree type = TREE_TYPE (var); tree new_var = lookup_decl (var, ctx); + tree opt_arg_label = NULL_TREE; + if (omp_is_reference (var)) { type = TREE_TYPE (type); if (TREE_CODE (type) != ARRAY_TYPE) { + if (omp_is_optional_argument (var)) + { + tree null_label + = create_artificial_label (UNKNOWN_LOCATION); + tree notnull_label + = create_artificial_label (UNKNOWN_LOCATION); + opt_arg_label + = create_artificial_label (UNKNOWN_LOCATION); + glabel *null_glabel + = gimple_build_label (null_label); + glabel *notnull_glabel + = gimple_build_label (notnull_label); + ggoto *opt_arg_ggoto + = gimple_build_goto (opt_arg_label); + gcond *cond; + + gimplify_expr (&x, &new_body, NULL, is_gimple_val, + fb_rvalue); + cond = gimple_build_cond (EQ_EXPR, x, + null_pointer_node, + null_label, + notnull_label); + gimple_seq_add_stmt (&new_body, cond); + gimple_seq_add_stmt (&new_body, null_glabel); + gimplify_assign (new_var, null_pointer_node, + &new_body); + gimple_seq_add_stmt (&new_body, opt_arg_ggoto); + gimple_seq_add_stmt (&new_body, notnull_glabel); + } + tree v = create_tmp_var_raw (type, get_name (var)); gimple_add_tmp_var (v); TREE_ADDRESSABLE (v) = 1; @@ -11850,6 +11917,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) gimplify_expr (&x, &new_body, NULL, is_gimple_val, fb_rvalue); gimple_seq_add_stmt (&new_body, gimple_build_assign (new_var, x)); + + if (opt_arg_label != NULL_TREE) + gimple_seq_add_stmt (&new_body, + gimple_build_label (opt_arg_label)); } break; }
On Mon, Jul 29, 2019 at 10:00:53PM +0100, Kwok Cheung Yeung wrote: > --- a/gcc/omp-low.c > +++ b/gcc/omp-low.c > @@ -11636,9 +11636,40 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, > omp_context *ctx) > tkind = GOMP_MAP_FIRSTPRIVATE_INT; > type = TREE_TYPE (ovar); > if (TREE_CODE (type) == ARRAY_TYPE) > - var = build_fold_addr_expr (var); > + { > + var = build_fold_addr_expr (var); > + gimplify_assign (x, var, &ilist); > + } > else > { > + tree opt_arg_label; > + bool optional_arg_p > + = TREE_CODE (TREE_TYPE (ovar)) == POINTER_TYPE > + && omp_is_optional_argument (ovar); This should be also wrapped in ()s for emacs, like: bool optional_arg_p = (TREE_CODE (TREE_TYPE (ovar)) == POINTER_TYPE && omp_is_optional_argument (ovar)); > + > + if (optional_arg_p) > + { > + tree null_label > + = create_artificial_label (UNKNOWN_LOCATION); > + tree notnull_label > + = create_artificial_label (UNKNOWN_LOCATION); > + opt_arg_label > + = create_artificial_label (UNKNOWN_LOCATION); > + tree new_x = copy_node (x); Please use unshare_expr instead of copy_node here. Otherwise LGTM. On the OpenMP side this isn't sufficient, because it only handles mapping clauses, not the data sharing, so I guess I'll need to go through all data sharing clauses on all constructs, write testcases and see if what OpenMP spec says (just a general rule): "If a list item that appears in a directive or clause is an optional dummy argument that is not present, the directive or clause for that list item is ignored. If the variable referenced inside a construct is an optional dummy argument that is not present, any explicitly determined, implicitly determined, or predetermined data-sharing and data-mapping attribute rules for that variable are ignored. Otherwise, if the variable is an optional dummy argument that is present, it is present inside the construct." is handled right. Jakub
diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 625df1e..2dfeca5 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -11635,18 +11635,51 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) tkind = GOMP_MAP_FIRSTPRIVATE_INT; type = TREE_TYPE (ovar); if (TREE_CODE (type) == ARRAY_TYPE) - var = build_fold_addr_expr (var); + { + var = build_fold_addr_expr (var); + gimplify_assign (x, var, &ilist); + } else { - if (omp_is_reference (ovar)) + tree opt_arg_label; + bool optional_arg_p = omp_is_optional_argument (ovar); + + if (optional_arg_p) + { + tree null_label + = create_artificial_label (UNKNOWN_LOCATION); + tree notnull_label + = create_artificial_label (UNKNOWN_LOCATION); + opt_arg_label + = create_artificial_label (UNKNOWN_LOCATION); + tree new_x = copy_node (x); + gcond *cond = gimple_build_cond (EQ_EXPR, ovar, + null_pointer_node, + null_label, + notnull_label); + gimple_seq_add_stmt (&ilist, cond); + gimple_seq_add_stmt (&ilist, + gimple_build_label (null_label)); + gimplify_assign (new_x, null_pointer_node, &ilist); + gimple_seq_add_stmt (&ilist, + gimple_build_goto (opt_arg_label)); + gimple_seq_add_stmt (&ilist, + gimple_build_label (notnull_label)); + } + + if (omp_is_reference (ovar) || optional_arg_p) { type = TREE_TYPE (type); if (TREE_CODE (type) != ARRAY_TYPE) var = build_simple_mem_ref (var); var = fold_convert (TREE_TYPE (x), var); } + + gimplify_assign (x, var, &ilist); + if (optional_arg_p) + gimple_seq_add_stmt (&ilist, + gimple_build_label (opt_arg_label)); } - gimplify_assign (x, var, &ilist); s = size_int (0); purpose = size_int (map_idx++);
This patch fixes a similar situation that occurs with the use_device clause, where the lowering would result in a null dereference if applied to a non-present optional argument. This patch builds a conditional check that skips the dereference if the argument is non-present, and ensures that optional arguments are treated like references. gcc/ * omp-low.c (lower_omp_target): For use_device clauses, generate conditional statements to treat Fortran optional arguments like references if non-null, or propogate null arguments into offloaded code otherwise. --- gcc/omp-low.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 4 deletions(-) CONSTRUCTOR_APPEND_ELT (vsize, purpose, s); @@ -11828,11 +11861,43 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) { tree type = TREE_TYPE (var); tree new_var = lookup_decl (var, ctx); - if (omp_is_reference (var)) + tree opt_arg_label = NULL_TREE; + + if (omp_is_reference (var) || omp_is_optional_argument (var)) { type = TREE_TYPE (type); if (TREE_CODE (type) != ARRAY_TYPE) { + if (omp_is_optional_argument (var)) + { + tree null_label + = create_artificial_label (UNKNOWN_LOCATION); + tree notnull_label + = create_artificial_label (UNKNOWN_LOCATION); + opt_arg_label + = create_artificial_label (UNKNOWN_LOCATION); + glabel *null_glabel + = gimple_build_label (null_label); + glabel *notnull_glabel + = gimple_build_label (notnull_label); + ggoto *opt_arg_ggoto + = gimple_build_goto (opt_arg_label); + gcond *cond; + + gimplify_expr (&x, &new_body, NULL, is_gimple_val, + fb_rvalue); + cond = gimple_build_cond (EQ_EXPR, x, + null_pointer_node, + null_label, + notnull_label); + gimple_seq_add_stmt (&new_body, cond); + gimple_seq_add_stmt (&new_body, null_glabel); + gimplify_assign (new_var, null_pointer_node, + &new_body); + gimple_seq_add_stmt (&new_body, opt_arg_ggoto); + gimple_seq_add_stmt (&new_body, notnull_glabel); + } + tree v = create_tmp_var_raw (type, get_name (var)); gimple_add_tmp_var (v); TREE_ADDRESSABLE (v) = 1; @@ -11849,6 +11914,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) gimplify_expr (&x, &new_body, NULL, is_gimple_val, fb_rvalue); gimple_seq_add_stmt (&new_body, gimple_build_assign (new_var, x)); + + if (opt_arg_label != NULL_TREE) + gimple_seq_add_stmt (&new_body, + gimple_build_label (opt_arg_label)); } break; }