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 <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 testlibgomp: cuCtxSynchronize error: an illegal memory access was encounteredPASS: 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 mappedPASS: 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 testlibgomp: use_device_ptr pointer wasn't mappedPASS: 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 testNo 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 testhost_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 testSTOP 1 Grüße ThomasOptional 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 extracheck 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 <k...@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 <k...@codesourcery.com>
+
+ * omp-low.c (lower_omp_target): Move gimplify_assign out from innermost
+ if statement.
+
2019-02-01 Thomas Schwinge <tho...@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));
--
2.8.1
