[4/5,OpenACC] Allow optional arguments to be used in the use_device OpenACC clause
diff mbox series

Message ID 2c2c6f22-9482-699c-0088-7b00f8969611@codesourcery.com
State New
Headers show
Series
  • Add support for Fortran optional arguments in OpenACC
Related show

Commit Message

Kwok Cheung Yeung July 12, 2019, 11:38 a.m. UTC
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;
  	  }

Comments

Kwok Cheung Yeung July 29, 2019, 9 p.m. UTC | #1
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;
  	  }
Jakub Jelinek July 31, 2019, 12:54 p.m. UTC | #2
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

Patch
diff mbox series

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++);