[Patch, Fortran, OpenMP] Support OpenMP 5.0's use_device_addr

[Jakub Jelinek]

On Wed, Oct 02, 2019 at 11:59:46AM +0200, Tobias Burnus wrote:
> This patch adds the first OpenMP 5 feature to gfortran – which is trivial as
> it justs adds some parsing, the heavy lifting is already done in the middle
> end. (Minus bugs.)
> 
> Additionally, I add a check for "is_device_ptr" – which is explicitly is
> specified in the OpenMP 4.5/5.0 spec. (i.e. dummy argument w/o
> pointer/allocatable/value attribute; C/C++ have the requirement pointer or
> array - or (C++) reference to those.) – I am not sure whether the Fortran
> restrictions make sense, but that's a OpenMP spec issue (for OpenMP 5.x?).

Indeed.  I guess the reason for dummy argument that is passed by reference
is that it under the hood works most closely to a C/C++ pointer (well, more
precisely a reference, but for C++ references there are special rules that
might not be appropriate for this purpose, in many cases it then behaves
more like fortran scalar pointer, it is a pointer + what it points to.

> I am a bit unsure about checks for use_device_…. The spec has no explicit
> rules, only some with can be deduced from the semantic. For C/C++, gcc/g++
> has for OpenMP 4.5 (_ptr) and for OpenMP 5 (_addr): only accept pointer or
> array type and C++ additionally for references to those. OpenMP 5 for _ptr
> accepts only POINTER_TYPE (C++: plus refs to it).

Indeed, you've seen the discussions.

> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -4563,7 +4571,25 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
>  		}
>  	    break;
>  	  case OMP_LIST_IS_DEVICE_PTR:
	    if (!n->sym->attr.dummy)
		  gfc_error ("Non-dummy object %qs in %s clause at %L",
			     n->sym->name, name, &n->where);

Formatting, gfc_error should be only 2 columns to the left of it, several
times below too.

> +	    if (n->sym->attr.allocatable
> +		|| (n->sym->ts.type == BT_CLASS
> +		    && CLASS_DATA (n->sym)->attr.allocatable))
> +		  gfc_error ("ALLOCATABLE object %qs in %s clause at %L",
> +			     n->sym->name, name, &n->where);
> +	    if (n->sym->attr.pointer
> +		|| (n->sym->ts.type == BT_CLASS
> +		    && CLASS_DATA (n->sym)->attr.pointer))
> +		  gfc_error ("POINTER object %qs in %s clause at %L",
> +			     n->sym->name, name, &n->where);
> +	    if (n->sym->attr.value)
> +		  gfc_error ("VALUE object %qs in %s clause at %L",
> +			     n->sym->name, name, &n->where);
> +	    break;
>  	  case OMP_LIST_USE_DEVICE_PTR:
> +	  case OMP_LIST_USE_DEVICE_ADDR:
>  	    /* FIXME: Handle these.  */

I think for use_device_addr there is no reason for a FIXME, there shouldn't
be restrictions on what variable is allowed in it, the restrictions are how
one can actually use the variable inside of the region (though, the
restriction is vague).  Though perhaps not worth having a separate break;
for it.

> diff --git a/gcc/testsuite/gfortran.dg/gomp/is_device_ptr-1.f90 b/gcc/testsuite/gfortran.dg/gomp/is_device_ptr-1.f90
> new file mode 100644
> index 00000000000..18211df0ea4
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/gomp/is_device_ptr-1.f90
> @@ -0,0 +1,24 @@
> +! { dg-do compile }
> +subroutine test(b,c,d)
> +  implicit none
> +  integer, value, target :: b
> +  integer, pointer :: c
> +  integer, allocatable, target :: d
> +
> +  integer, target :: a(5)
> +
> +  !$omp target is_device_ptr(a) ! { dg-error "Non-dummy object .a. in IS_DEVICE_PTR clause" }
> +  !$omp target is_device_ptr(b) ! { dg-error "VALUE object .b. in IS_DEVICE_PTR clause" }
> +  !$omp target is_device_ptr(c) ! { dg-error "POINTER object .c. in IS_DEVICE_PTR clause" }
> +  !$omp target is_device_ptr(d) ! { dg-error "ALLOCATABLE object .d. in IS_DEVICE_PTR clause" }
> +  !$omp end target
> +  !$omp end target
> +  !$omp end target
> +  !$omp end target

Can you please move the !$opm end target directives after each !$omp target?
Encountering a target inside of target is unspecified behavior...

> +
> +  !$omp target data map(a) use_device_addr(a)  ! Should be okay
> +  !$omp end target data
> +
> +  !$omp target data map(c) use_device_ptr(c)  ! Should be okay
> +  !$omp end target data
> +end subroutine test

Ok with those nits fixed.

	Jakub