diff mbox series

[og8] Allow optional arguments to be used in the use_device OpenACC clause

Message ID 8e8bf339-42eb-1b65-4b4b-2df50a2bbcb9@codesourcery.com
State New
Headers show
Series [og8] Allow optional arguments to be used in the use_device OpenACC clause | expand

Commit Message

Kwok Cheung Yeung Jan. 31, 2019, 6:30 p.m. UTC 
This patch allows for the use of Fortran optional arguments in the 
use_device clause of a host_data directive.

I will push this into openacc-gcc-8-branch later today.

Kwok


Optional arguments should be treated as references rather than pointers
in the lowering.  However, for non-present arguments, this would result
in a null dereference, so conditionals need to be added to detect and
handle this.

	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.

Reviewed-by: Julian Brown <julian@codesourcery.com>
---
  gcc/ChangeLog.openacc |  7 +++++
  gcc/omp-low.c         | 77 
++++++++++++++++++++++++++++++++++++++++++++++++---
  2 files changed, 80 insertions(+), 4 deletions(-)

  	    purpose = size_int (map_idx++);
@@ -9137,11 +9170,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;
@@ -9158,6 +9223,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

Thomas Schwinge Feb. 1, 2019, 4:24 p.m. UTC | #1 
Hi Kwok!

On Thu, 31 Jan 2019 18:30:35 +0000, Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
> This patch allows for the use of Fortran optional arguments in the 
> use_device clause of a host_data directive.
> 
> I will push this into openacc-gcc-8-branch later today.

Per my testing, it unfortunately also introduces a number of regressions:

    [-PASS:-]{+FAIL:+} gfortran.dg/goacc/uninit-use-device-clause.f95   -O   (test for warnings, line 7)
    PASS: gfortran.dg/goacc/uninit-use-device-clause.f95   -O  (test for excess errors)

(This probably means that the clause argument is no longer
"evaluated/used".)

    PASS: libgomp.c/target-14.c (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.c/target-14.c execution test

    libgomp: cuCtxSynchronize error: an illegal memory access was encountered

    PASS: libgomp.c/target-18.c (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.c/target-18.c execution test

    libgomp: use_device_ptr pointer wasn't mapped

    PASS: libgomp.c++/target-9.C (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.c++/target-9.C execution test

    libgomp: use_device_ptr pointer wasn't mapped

    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  execution test

    libgomp: use_device_ptr pointer wasn't mapped

    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  execution test

No error message.

    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  execution test
    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  execution test
    PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  execution test

    host_data-6.exe: [...]/libgomp.oacc-c-c++-common/host_data-6.c:15: foo: Assertion `p == (float *) host_p' failed.
    
Same for C++, for "libgomp.oacc-c-c++-common/host_data-5.c", and
"libgomp.oacc-c-c++-common/host_data-6.c".

    PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O0  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O0  execution test
    PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O1  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O1  execution test
    PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  execution test
    PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
    PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -g  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -g  execution test
    PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -Os  (test for excess errors)
    [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -Os  execution test

    STOP 1


Grüße
 Thomas


> Optional arguments should be treated as references rather than pointers
> in the lowering.  However, for non-present arguments, this would result
> in a null dereference, so conditionals need to be added to detect and
> handle this.
> 
> 	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.
> 
> Reviewed-by: Julian Brown <julian@codesourcery.com>
> ---
>   gcc/ChangeLog.openacc |  7 +++++
>   gcc/omp-low.c         | 77 
> ++++++++++++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 80 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
> index 5104eaa..58258fd 100644
> --- a/gcc/ChangeLog.openacc
> +++ b/gcc/ChangeLog.openacc
> @@ -1,5 +1,12 @@
>   2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
> 
> +	* 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.
> +
> +2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
> +
>   	* omp-general.c (omp_is_optional_argument): Add comment.  Add extra
>   	check for Fortran language.
> 
> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
> index ef71704..3ae39c3 100644
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -8938,18 +8938,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);
>   	    decl_args = append_decl_arg (ovar, decl_args, ctx);
>   	    purpose = size_int (map_idx++);
> @@ -9137,11 +9170,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;
> @@ -9158,6 +9223,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;
>   	  }
Kwok Cheung Yeung Feb. 1, 2019, 6:02 p.m. UTC | #2 
There is an error in the logic here:

--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -8938,18 +8938,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) || optional_arg_p)
                   {
...
+                   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);

The gimplify_assign was hoisted into the two branches of the preceding 
if-else because I wanted to skip the assign if there was a non-present 
optional argument. However, in the else case, the assign only happens if 
omp_is_reference or optional_arg_p is true, when it should be unconditional.

I can confirm that fixing this allows at least 
libgomp.oacc-fortran/host_data-1.f90 to pass again. I will post the 
patch when I have double-checked the other cases.

Thanks

Kwok

On 01/02/2019 4:24 pm, Thomas Schwinge wrote:
> Hi Kwok!
> 
> On Thu, 31 Jan 2019 18:30:35 +0000, Kwok Cheung Yeung <kcy@codesourcery.com> wrote:
>> This patch allows for the use of Fortran optional arguments in the
>> use_device clause of a host_data directive.
>>
>> I will push this into openacc-gcc-8-branch later today.
> 
> Per my testing, it unfortunately also introduces a number of regressions:
> 
>      [-PASS:-]{+FAIL:+} gfortran.dg/goacc/uninit-use-device-clause.f95   -O   (test for warnings, line 7)
>      PASS: gfortran.dg/goacc/uninit-use-device-clause.f95   -O  (test for excess errors)
> 
> (This probably means that the clause argument is no longer
> "evaluated/used".)
> 
>      PASS: libgomp.c/target-14.c (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.c/target-14.c execution test
> 
>      libgomp: cuCtxSynchronize error: an illegal memory access was encountered
> 
>      PASS: libgomp.c/target-18.c (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.c/target-18.c execution test
> 
>      libgomp: use_device_ptr pointer wasn't mapped
> 
>      PASS: libgomp.c++/target-9.C (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.c++/target-9.C execution test
> 
>      libgomp: use_device_ptr pointer wasn't mapped
> 
>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  execution test
> 
>      libgomp: use_device_ptr pointer wasn't mapped
> 
>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  execution test
> 
> No error message.
> 
>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O0  execution test
>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 -foffload=nvptx-none  -O2  execution test
>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  execution test
> 
>      host_data-6.exe: [...]/libgomp.oacc-c-c++-common/host_data-6.c:15: foo: Assertion `p == (float *) host_p' failed.
>      
> Same for C++, for "libgomp.oacc-c-c++-common/host_data-5.c", and
> "libgomp.oacc-c-c++-common/host_data-6.c".
> 
>      PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O0  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O0  execution test
>      PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O1  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O1  execution test
>      PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  execution test
>      PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
>      PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -g  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 -g  execution test
>      PASS: libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -Os  (test for excess errors)
>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -Os  execution test
> 
>      STOP 1
> 
> 
> Grüße
>   Thomas
> 
> 
>> Optional arguments should be treated as references rather than pointers
>> in the lowering.  However, for non-present arguments, this would result
>> in a null dereference, so conditionals need to be added to detect and
>> handle this.
>>
>> 	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.
>>
>> Reviewed-by: Julian Brown <julian@codesourcery.com>
>> ---
>>    gcc/ChangeLog.openacc |  7 +++++
>>    gcc/omp-low.c         | 77
>> ++++++++++++++++++++++++++++++++++++++++++++++++---
>>    2 files changed, 80 insertions(+), 4 deletions(-)
>>
>> diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
>> index 5104eaa..58258fd 100644
>> --- a/gcc/ChangeLog.openacc
>> +++ b/gcc/ChangeLog.openacc
>> @@ -1,5 +1,12 @@
>>    2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
>>
>> +	* 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.
>> +
>> +2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
>> +
>>    	* omp-general.c (omp_is_optional_argument): Add comment.  Add extra
>>    	check for Fortran language.
>>
>> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
>> index ef71704..3ae39c3 100644
>> --- a/gcc/omp-low.c
>> +++ b/gcc/omp-low.c
>> @@ -8938,18 +8938,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);
>>    	    decl_args = append_decl_arg (ovar, decl_args, ctx);
>>    	    purpose = size_int (map_idx++);
>> @@ -9137,11 +9170,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;
>> @@ -9158,6 +9223,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;
>>    	  }
Kwok Cheung Yeung Feb. 1, 2019, 9:30 p.m. UTC | #3 
I have retested all the failing cases and they now pass with the 
attached patch. I will commit this to openacc-gcc-8-branch now as the 
fix is obvious.

Kwok

On 01/02/2019 6:02 pm, Kwok Cheung Yeung wrote:
> There is an error in the logic here:
> 
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -8938,18 +8938,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) || optional_arg_p)
>                    {
> ...
> +                   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);
> 
> The gimplify_assign was hoisted into the two branches of the preceding 
> if-else because I wanted to skip the assign if there was a non-present 
> optional argument. However, in the else case, the assign only happens if 
> omp_is_reference or optional_arg_p is true, when it should be 
> unconditional.
> 
> I can confirm that fixing this allows at least 
> libgomp.oacc-fortran/host_data-1.f90 to pass again. I will post the 
> patch when I have double-checked the other cases.
> 
> Thanks
> 
> Kwok
> 
> On 01/02/2019 4:24 pm, Thomas Schwinge wrote:
>> Hi Kwok!
>>
>> On Thu, 31 Jan 2019 18:30:35 +0000, Kwok Cheung Yeung 
>> <kcy@codesourcery.com> wrote:
>>> This patch allows for the use of Fortran optional arguments in the
>>> use_device clause of a host_data directive.
>>>
>>> I will push this into openacc-gcc-8-branch later today.
>>
>> Per my testing, it unfortunately also introduces a number of regressions:
>>
>>      [-PASS:-]{+FAIL:+} 
>> gfortran.dg/goacc/uninit-use-device-clause.f95   -O   (test for 
>> warnings, line 7)
>>      PASS: gfortran.dg/goacc/uninit-use-device-clause.f95   -O  (test 
>> for excess errors)
>>
>> (This probably means that the clause argument is no longer
>> "evaluated/used".)
>>
>>      PASS: libgomp.c/target-14.c (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.c/target-14.c execution test
>>
>>      libgomp: cuCtxSynchronize error: an illegal memory access was 
>> encountered
>>
>>      PASS: libgomp.c/target-18.c (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.c/target-18.c execution test
>>
>>      libgomp: use_device_ptr pointer wasn't mapped
>>
>>      PASS: libgomp.c++/target-9.C (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.c++/target-9.C execution test
>>
>>      libgomp: use_device_ptr pointer wasn't mapped
>>
>>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O0  (test for excess errors)
>>      [-PASS:-]{+FAIL:+} 
>> libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O0  execution test
>>
>>      libgomp: use_device_ptr pointer wasn't mapped
>>
>>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O2  (test for excess errors)
>>      [-PASS:-]{+FAIL:+} 
>> libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-5.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O2  execution test
>>
>> No error message.
>>
>>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O0  (test for excess errors)
>>      [-PASS:-]{+FAIL:+} 
>> libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O0  execution test
>>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O2  (test for excess errors)
>>      [-PASS:-]{+FAIL:+} 
>> libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c 
>> -DACC_DEVICE_TYPE_nvidia="nvptx-none" -DACC_MEM_SHARED=0 
>> -foffload=nvptx-none  -O2  execution test
>>      PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  
>> (test for excess errors)
>>      [-PASS:-]{+FAIL:+} 
>> libgomp.oacc-c/../libgomp.oacc-c-c++-common/host_data-6.c 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  
>> execution test
>>
>>      host_data-6.exe: 
>> [...]/libgomp.oacc-c-c++-common/host_data-6.c:15: foo: Assertion `p == 
>> (float *) host_p' failed.
>> Same for C++, for "libgomp.oacc-c-c++-common/host_data-5.c", and
>> "libgomp.oacc-c-c++-common/host_data-6.c".
>>
>>      PASS: libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O0  
>> (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O0  
>> execution test
>>      PASS: libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O1  
>> (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O1  
>> execution test
>>      PASS: libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  
>> (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O2  
>> execution test
>>      PASS: libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 
>> -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer 
>> -finline-functions  (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 
>> -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer 
>> -finline-functions  execution test
>>      PASS: libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 
>> -g  (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -O3 
>> -g  execution test
>>      PASS: libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -Os  
>> (test for excess errors)
>>      [-PASS:-]{+FAIL:+} libgomp.oacc-fortran/host_data-1.f90 
>> -DACC_DEVICE_TYPE_host="" -DACC_MEM_SHARED=1 -foffload=disable  -Os  
>> execution test
>>
>>      STOP 1
>>
>>
>> Grüße
>>   Thomas
>>
>>
>>> Optional arguments should be treated as references rather than pointers
>>> in the lowering.  However, for non-present arguments, this would result
>>> in a null dereference, so conditionals need to be added to detect and
>>> handle this.
>>>
>>>     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.
>>>
>>> Reviewed-by: Julian Brown <julian@codesourcery.com>
>>> ---
>>>    gcc/ChangeLog.openacc |  7 +++++
>>>    gcc/omp-low.c         | 77
>>> ++++++++++++++++++++++++++++++++++++++++++++++++---
>>>    2 files changed, 80 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
>>> index 5104eaa..58258fd 100644
>>> --- a/gcc/ChangeLog.openacc
>>> +++ b/gcc/ChangeLog.openacc
>>> @@ -1,5 +1,12 @@
>>>    2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
>>>
>>> +    * 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.
>>> +
>>> +2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
>>> +
>>>        * omp-general.c (omp_is_optional_argument): Add comment.  Add 
>>> extra
>>>        check for Fortran language.
>>>
>>> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
>>> index ef71704..3ae39c3 100644
>>> --- a/gcc/omp-low.c
>>> +++ b/gcc/omp-low.c
>>> @@ -8938,18 +8938,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);
>>>            decl_args = append_decl_arg (ovar, decl_args, ctx);
>>>            purpose = size_int (map_idx++);
>>> @@ -9137,11 +9170,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;
>>> @@ -9158,6 +9223,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;
>>>          }
From 2889b3618ae906bee9af58baf9e38e41c189e632 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung <kcy@codesourcery.com>
Date: Fri, 1 Feb 2019 13:15:32 -0800
Subject: [PATCH] [og8] Fix bug introduced by use_device optional arguments
 patch

	* omp-low.c (lower_omp_target): Move gimplify_assign out from innermost
	if statement.
---
 gcc/ChangeLog.openacc | 5 +++++
 gcc/omp-low.c         | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
index a347263..84f6a38 100644
--- a/gcc/ChangeLog.openacc
+++ b/gcc/ChangeLog.openacc
@@ -1,3 +1,8 @@
+2019-02-01  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
+	* omp-low.c (lower_omp_target): Move gimplify_assign out from innermost
+	if statement.
+
 2019-02-01  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* omp-oacc-kernels.c (struct adjust_nested_loop_clauses_wi_info): New.
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 636a4a0..f8bb1fa 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -9016,9 +9016,9 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 		    if (TREE_CODE (type) != ARRAY_TYPE)
 		      var = build_simple_mem_ref (var);
 		    var = fold_convert (TREE_TYPE (x), var);
-		    gimplify_assign (x, var, &ilist);
 		  }
 
+		gimplify_assign (x, var, &ilist);
 		if (optional_arg_p)
 		  gimple_seq_add_stmt (&ilist,
 				       gimple_build_label (opt_arg_label));
diff mbox series

Patch

diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
index 5104eaa..58258fd 100644
--- a/gcc/ChangeLog.openacc
+++ b/gcc/ChangeLog.openacc
@@ -1,5 +1,12 @@ 
  2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>

+	* 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.
+
+2019-01-31  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
  	* omp-general.c (omp_is_optional_argument): Add comment.  Add extra
  	check for Fortran language.

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index ef71704..3ae39c3 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -8938,18 +8938,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);
  	    decl_args = append_decl_arg (ovar, decl_args, ctx);