Hi Kwok!

On Thu, 31 Jan 2019 18:30:35 +0000, Kwok Cheung Yeung <k...@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 <jul...@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  <k...@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  <k...@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;
>         }

Reply via email to