On Fri, Nov 29, 2019 at 01:03:13PM +0100, Tobias Burnus wrote:
> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -60,7 +60,8 @@ gfc_omp_is_allocatable_or_ptr (const_tree decl)
>
> /* True if the argument is an optional argument; except that false is also
> returned for arguments with the value attribute (nonpointers) and for
> - assumed-shape variables (decl is a local variable containing arg->data).
> */
> + assumed-shape variables (decl is a local variable containing arg->data).
> + Note that pvoid_type_node is for 'type(c_ptr), value. */
>
> static bool
> gfc_omp_is_optional_argument (const_tree decl)
> @@ -68,6 +69,7 @@ gfc_omp_is_optional_argument (const_tree decl)
> return (TREE_CODE (decl) == PARM_DECL
> && DECL_LANG_SPECIFIC (decl)
> && TREE_CODE (TREE_TYPE (decl)) == POINTER_TYPE
> + && TREE_TYPE (decl) != pvoid_type_node
Is it always pvoid_type_node, or could it be say const qualified version
thereof etc. (C void const *) etc.? If the latter, then
&& !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (decl)))
might be better check. If it is always just pvoid_type_node, the above is
fine sure.
> && GFC_DECL_OPTIONAL_ARGUMENT (decl));
> }
>
> @@ -99,9 +101,12 @@ gfc_omp_check_optional_argument (tree decl, bool
> for_present_check)
> || !GFC_DECL_OPTIONAL_ARGUMENT (decl))
> return NULL_TREE;
>
> - /* For VALUE, the scalar variable is passed as is but a hidden argument
> - denotes the value. Cf. trans-expr.c. */
> - if (TREE_CODE (TREE_TYPE (decl)) != POINTER_TYPE)
> + /* Scalars with VALUE attribute which are passed by value use a hidden
> + argument to denote the present status. They are passed as nonpointer
> type
> + with one exception: 'type(c_ptr), value' as '*void'. */
void* or void * instead of *void?
> + /* Cf. trans-expr.c's gfc_conv_expr_present. */
> + if (TREE_CODE (TREE_TYPE (decl)) != POINTER_TYPE
> + || TREE_TYPE (decl) == pvoid_type_node)
And similar question to the above one.
> @@ -12027,14 +12022,12 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
> omp_context *ctx)
> && omp_is_allocatable_or_ptr (ovar))))
> {
> var = build_simple_mem_ref (var);
> - do_optional_check = true;
> }
The above then becomes { single_stmt } and so should be changed to
single_stmt without {}s around it.
> @@ -1,33 +1,47 @@
> ! Check whether absent optional arguments are properly
> ! handled with use_device_{addr,ptr}.
> program main
> + use iso_c_binding, only: c_ptr, c_loc
> implicit none (type, external)
> call foo()
> contains
> - subroutine foo(v, w, x, y, z)
> + subroutine foo(v, w, x, y, z, cptr)
> integer, target, optional, value :: v
> integer, target, optional :: w
> integer, target, optional :: x(:)
> integer, target, optional, allocatable :: y
> integer, target, optional, allocatable :: z(:)
> + type(c_ptr), target, optional, value :: cptr
> integer :: d
>
> - !$omp target data map(d) use_device_addr(v, w, x, y, z)
> - if(present(v)) stop 1
> - if(present(w)) stop 2
> - if(present(x)) stop 3
> - if(present(y)) stop 4
> - if(present(z)) stop 5
> + ! Need to map per-VALUE arguments, if present
> + if (present(v)) then
> + !$omp target enter data map(to:v)
> + stop 1 ! – but it shall not be present in this test case.
> + end if
> + if (present(cptr)) then
> + !$omp target enter data map(to:cptr)
> + stop 2 ! – but it shall not be present in this test case.
> + end if
Shouln't the stop arguments differ from anything else already in the
testcase?
> +
> + !$omp target data map(d) use_device_addr(v, w, x, y, z, cptr)
> + if (present(v)) then; v = 5; stop 1; endif
> + if (present(w)) then; w = 5; stop 2; endif
> + if (present(x)) then; x(1) = 5; stop 3; endif
> + if (present(y)) then; y = 5; stop 4; endif
> + if (present(z)) then; z(1) = 5; stop 5; endif
> + if (present(cptr)) then; cptr = c_loc(v); stop 6; endif
> !$omp end target data
>
> ! Using 'v' in use_device_ptr gives an ICE
> ! TODO: Find out what the OpenMP spec permits for use_device_ptr
>
> - !$omp target data map(d) use_device_ptr(w, x, y, z)
> - if(present(w)) stop 6
> - if(present(x)) stop 7
> - if(present(y)) stop 8
> - if(present(z)) stop 9
> + !$omp target data map(d) use_device_ptr(w, x, y, z, cptr)
> + if(present(w)) then; w = 5; stop 11; endif
> + if(present(x)) then; x(1) = 5; stop 12; endif
> + if(present(y)) then; y = 5; stop 13; endif
> + if(present(z)) then; z(1) = 5; stop 14; endif
> + if(present(cptr)) then; cptr = c_loc(x); stop 15; endif
> !$omp end target data
> end subroutine foo
> end program main
Otherwise LGTM.
Jakub
