Richard Biener wrote:
On Mon, Jul 29, 2024 at 9:26 PM Tobias Burnus<tbur...@baylibre.com> wrote:
Inside pass_omp_target_link::execute, there is a call to
gimple_regimplify_operands but the value expression is not
expanded.[...]
Where is_gimple_mem_ref_addr is defined as:
/* Return true if T is a valid address operand of a MEM_REF. */
bool
is_gimple_mem_ref_addr (tree t)
{
return (is_gimple_reg (t)
|| TREE_CODE (t) == INTEGER_CST
|| (TREE_CODE (t) == ADDR_EXPR
&& (CONSTANT_CLASS_P (TREE_OPERAND (t, 0))
|| decl_address_invariant_p (TREE_OPERAND (t, 0)))));
}
I think iff then decl_address_invariant_p should be amended.
This does not work - at least not for my use case if OpenMP
link variables - due to ordering issues.
For the device compilers, the VALUE_EXPR is added in lto_main
or in do_whole_program_analysis (same file: lto/lto.cc) by
callingoffload_handle_link_vars. The value expression is then later expanded
via pass_omp_target_link::execute, but in between the following happens:
lto_main callssymbol_table::compile, which then calls
cgraph_node::expand and that executes
res |= verify_types_in_gimple_reference (lhs, true); for lhs being:
MEM <uint128_t> [(c_char * {ref-all})&arr2]
But when adding the has-value-expr check either directly to is_gimple_mem_ref_addr or to the decl_address_invariant_pit calls, the following condition becomes true the called function in
tree-cfg.cc:
3302 if (!is_gimple_mem_ref_addr (TREE_OPERAND (expr, 0))
3303 || (TREE_CODE (TREE_OPERAND (expr, 0)) == ADDR_EXPR
3304 && verify_address (TREE_OPERAND (expr, 0), false)))
3305 {
3306 error ("invalid address operand in %qs", code_name);
* * * Thus, I am now back to the previous change, except for:
Why is the gimplify_addr_expr hunk needed? It should get
to gimplifying the VAR_DECL/PARM_DECL by recursion?
Indeed. I wonder why I had (thought to) need it before; possibly
because it was needed or thought to be needed when trying to trace
this down.
Previous patch - except for that bit removed - attached.
Thoughts, better ideas?
Tobias
gimplify.cc: Handle VALUE_EXPR of MEM_REF's ADDR_EXPR argument [PR115637]
As the PR and included testcase shows, replacing 'arr2' by its value expression
'*arr2$13$linkptr' failed for
MEM <uint128_t> [(c_char * {ref-all})&arr2]
which left 'arr2' in the code as unknown symbol.
PR middle-end/115637
gcc/ChangeLog:
* gimplify.cc (gimplify_expr): For MEM_REF and an ADDR_EXPR, also
check for value-expr arguments.
(gimplify_body): Fix macro name in the comment.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/declare-target-link.f90: Uncomment
now working code.
gcc/gimplify.cc | 9 +++++++--
libgomp/testsuite/libgomp.fortran/declare-target-link.f90 | 15 ++++++---------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index ab323d764e8..4fa88c9b21c 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -18251,8 +18251,13 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
in suitable form. Re-gimplifying would mark the address
operand addressable. Always gimplify when not in SSA form
as we still may have to gimplify decls with value-exprs. */
+ tmp = TREE_OPERAND (*expr_p, 0);
if (!gimplify_ctxp || !gimple_in_ssa_p (cfun)
- || !is_gimple_mem_ref_addr (TREE_OPERAND (*expr_p, 0)))
+ || (!is_gimple_mem_ref_addr (tmp)
+ || (TREE_CODE (tmp) == ADDR_EXPR
+ && (VAR_P (TREE_OPERAND (tmp, 0))
+ || TREE_CODE (TREE_OPERAND (tmp, 0)) == PARM_DECL)
+ && DECL_HAS_VALUE_EXPR_P (TREE_OPERAND (tmp, 0)))))
{
ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
is_gimple_mem_ref_addr, fb_rvalue);
@@ -19422,7 +19427,7 @@ gimplify_body (tree fndecl, bool do_parms)
DECL_SAVED_TREE (fndecl) = NULL_TREE;
/* If we had callee-copies statements, insert them at the beginning
- of the function and clear DECL_VALUE_EXPR_P on the parameters. */
+ of the function and clear DECL_HAS_VALUE_EXPR_P on the parameters. */
if (!gimple_seq_empty_p (parm_stmts))
{
tree parm;
diff --git a/libgomp/testsuite/libgomp.fortran/declare-target-link.f90 b/libgomp/testsuite/libgomp.fortran/declare-target-link.f90
index 2ce212d114f..44c67f925bd 100644
--- a/libgomp/testsuite/libgomp.fortran/declare-target-link.f90
+++ b/libgomp/testsuite/libgomp.fortran/declare-target-link.f90
@@ -1,5 +1,7 @@
! { dg-additional-options "-Wall" }
+
! PR fortran/115559
+! PR middle-end/115637
module m
integer :: A
@@ -73,24 +75,19 @@ contains
!$omp target map(from:res)
res = run_device1()
!$omp end target
- print *, res
- ! FIXME: arr2 not link mapped -> PR115637
- ! if (res /= -11436) stop 5
- if (res /= -11546) stop 5 ! FIXME
+ ! print *, res
+ if (res /= -11436) stop 5
end
integer function run_device1()
!$omp declare target
integer :: i
run_device1 = -99
- ! FIXME: arr2 not link mapped -> PR115637
- ! arr2 = [11,22,33,44]
+ arr2 = [11,22,33,44]
if (any (arr(10:50) /= [(i, i=10,50)])) then
run_device1 = arr(11)
return
end if
- ! FIXME: -> PR115637
- ! run_device1 = sum(arr(10:13) + arr2)
- run_device1 = sum(arr(10:13) ) ! FIXME
+ run_device1 = sum(arr(10:13) + arr2)
do i = 10, 50
arr(i) = 3 - 10 * arr(i)
end do
