[OpenACC,Fortran] Fix PR77371, ICE on allocatable

Hi Jakub,
This patch fixes the two ICEs listed on PR77371.
One is due to the Fortran omp_privatize_by_reference hook returning true
for types like 'character(kind=1)[1:XX] *', causing them to be processed
by the path intended for C++ reference types.

The other one is simply not setting 'remove = true' while error_at() was already called.

Tested without regressions, committed on gomp-4_0-branch,
is this okay for trunk as well?

Thanks,
Chung-Lin

	PR fortran/77371
	* omp-low.c (lower_omp_target): Avoid reference-type processing
	on pointers for firstprivate clause.
	* gimplify.c (gimplify_adjust_omp_clauses): Add 'remove = true'
	when emitting error on private/firstprivate reductions.

	testsuite/
	* gfortran.dg/goacc/pr77371-1.f90: New test.
	* gfortran.dg/goacc/pr77371-2.f90: New test.

Index: testsuite/gfortran.dg/goacc/pr77371-1.f90
===================================================================
--- testsuite/gfortran.dg/goacc/pr77371-1.f90	(revision 0)
+++ testsuite/gfortran.dg/goacc/pr77371-1.f90	(revision 0)
@@ -0,0 +1,9 @@
+! PR fortran/77371
+! { dg-do compile }
+program p
+  character(:), allocatable :: z
+  !$acc parallel
+  z = 'abc' 
+  !$acc end parallel
+  print *, z
+end
Index: testsuite/gfortran.dg/goacc/pr77371-2.f90
===================================================================
--- testsuite/gfortran.dg/goacc/pr77371-2.f90	(revision 0)
+++ testsuite/gfortran.dg/goacc/pr77371-2.f90	(revision 0)
@@ -0,0 +1,7 @@
+! PR fortran/77371
+! { dg-do compile }
+program p
+   integer, allocatable :: n
+!$acc parallel reduction (+:n) private(n) ! { dg-error "invalid private reduction" }
+!$acc end parallel
+end

On Sun, Oct 02, 2016 at 06:15:18PM +0800, Chung-Lin Tang wrote:
> This patch fixes the two ICEs listed on PR77371.
> One is due to the Fortran omp_privatize_by_reference hook returning true
> for types like 'character(kind=1)[1:XX] *', causing them to be processed
> by the path intended for C++ reference types.

The path isn't something intended for C++ reference types, but for all the
vars where whatever they point to should be privatized rather than just
their value.  Consider

program p
   integer, allocatable :: n
   integer :: m
   allocate (n)
   n = 6
!$acc parallel firstprivate(n) private(m)
   m = n
!$acc end parallel
end

testcase which with -fopenacc ICEs the same way, and then look carefully
what is done on

program p
   integer, allocatable :: n
   integer :: m
   allocate (n)
   n = 6
!$omp parallel firstprivate(n) private(m)
   m = n
!$omp end parallel
end

with -fopenmp.  The var is actually properly allocatable in the latter case,
while it is not with your patch on the first testcase, you just copy over the host pointer, that
is definitely not going to work on non-shared memory offloading.
There is nothing special about references that use POINTER_TYPE as opposed
to REFERENCE_TYPE.  So, please first get this working with firstprivate on
allocatables and only then start to play with reductions.

> The other one is simply not setting 'remove = true' while error_at() was already called.

The gimplify.c change is ok for trunk.

> Tested without regressions, committed on gomp-4_0-branch,
> is this okay for trunk as well?
> 
> Thanks,
> Chung-Lin
> 
> 	PR fortran/77371
> 	* omp-low.c (lower_omp_target): Avoid reference-type processing
> 	on pointers for firstprivate clause.
> 	* gimplify.c (gimplify_adjust_omp_clauses): Add 'remove = true'
> 	when emitting error on private/firstprivate reductions.
> 
> 	testsuite/
> 	* gfortran.dg/goacc/pr77371-1.f90: New test.
> 	* gfortran.dg/goacc/pr77371-2.f90: New test.

	Jakub

On 10/03/2016 07:59 AM, Jakub Jelinek wrote:

> with -fopenmp.  The var is actually properly allocatable in the latter case,
> while it is not with your patch on the first testcase, you just copy over the host pointer, that
> is definitely not going to work on non-shared memory offloading.

I think that's the expected behavior in OpenACC. Basically, unless
pointers have explicit data clauses with subarray arguments, the
compiler is supposed to treat those pointers as "scalars" and not remap
the contents of the pointer.

Chung-Lin, maybe this issue with allocatable data along with a different
void will persuade the OpenACC technical committee update the implicit
data mapping behavior of pointers. Can you raise this issue with the
OpenACC technical committee?

> There is nothing special about references that use POINTER_TYPE as opposed
> to REFERENCE_TYPE.  So, please first get this working with firstprivate on
> allocatables and only then start to play with reductions.

I agree something like that would be better. Is OpenMP supposed to
implicitly map the allocated data on the accelerator too?

Cesar