Re: [Patch] Fortran: %re/%im fixes for OpenMP/OpenACC + gfc_is_simplify_contiguous

[Thomas Schwinge <thomas@codesourcery.com>]

Hi Tobias!

On 2021-02-08T18:50:25+0100, Tobias Burnus <tobias@codesourcery.com> wrote:
> Found when looking at Julian's 3/4 OpenACC patch, which has not
> yet been committed (and needs still to be revised a bit.)
>
> The fix (a) avoids an ICE when Julian's patch has been applied.
> The patch (b) just makes one error message less confusing.
>
> The testcase shows that only %re/%im run reach the new code as
> %kind/%len are != EXPR_VARIABLE. (The error message is slightly
> misleading if the 'list item'/'var' is not a variable.)
>
>
> (a) We need to handle REF_INQUIRY gfc_is_simplify_contiguous.
>
> That function is used for 'is_contiguous(a(:)%re)', but it works
> without this patch and simplifies already to .false.
> And it is used in openmp.c, which can ICE without this patch.
>
> (b) Just makes the error message nicer - as only EXPR_VARIABLE
> reaches that code, only %re and %im are mentioned in the
> error message.

As so often, I can't really comment on the Fortran language/GCC Fortran
front end details.  ;-|

> (Actually, it is not completely clear whether %re/%im are invalid
> or not; I think they should be – but one can argue otherwise.
> For OpenMP I just saw that it is tacked internally in Issue 2661,
> for OpenACC it is tracked as Issue 346 – but neither has been
> discussed, yet.)

Thanks for filing/locating these discussion items for OpenACC/OpenMP
upstream.  May also put these references into the testcases, so that once
these get addressed, we have something to 'grep' for in GCC?

> PS: '%re'/'%im' permit accessing the real/imaginary part of a
> complex variable as lvalue (in the C++ sense) and also permit
> "var(:)%re = 1.0", which real(z) or imag(z) does not permit.
>
> var%kind == kind(var), but derived types also permit
> parameterized derived types (PDT), which can use '%foo' for respective
> 'len' and 'kind' components.

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/goacc-gomp/ref_inquiry.f90

Originally, the 'goacc-gomp' testsuites have been meant to be used for
testcases related to OpenACC/OpenMP (non-)interoperability, such as
improper nesting etc. (should be the only/major use -- given that
currently there isn't really any OpenACC/OpenMP interoperability).

This one, I'd have put into separate 'gfortran.dg/goacc/ref_inquiry.f90'
and 'gfortran.dg/gomp/ref_inquiry.f90' (everyone cross-referencing the
other, unless using the same filename makes that obvious enough).

> @@ -0,0 +1,42 @@
> +implicit none
> +type t
> +  integer :: i
> +  character :: c
> +  complex :: z
> +end type t
> +
> +integer :: i
> +character(kind=4, len=5) :: c
> +complex :: z, zz(5)
> +type(t) :: x
> +
> +print *, is_contiguous(zz(:)%re)
> +
> +! EXPR_VARIABLE / Cf. also OpenMP spec issue 2661
> +!$omp target enter data map(to: z%re)    ! { dg-error "Unexpected complex-parts designator" }
> +!$omp target enter data map(to: z%im)    ! { dg-error "Unexpected complex-parts designator" }
> +!$omp target enter data map(to: x%z%re)  ! { dg-error "Unexpected complex-parts designator" }
> +!$omp target enter data map(to: x%z%im)  ! { dg-error "Unexpected complex-parts designator" }
> +
> +! Fails differently as it is not a variable (EXPR_VARIABLE)
> +!$omp target enter data map(to: i%kind, c%len)     ! { dg-error "not a proper array section" }
> +!$omp target enter data map(to: x%i%kind, x%c%len) ! { dg-error "not a proper array section" }

I note that 'zz' variants (see below) are not being checked for OpenMP.

> +! Likewise for OpenACC
> +!$acc enter data copyin(i%kind, c%len)     ! { dg-error "not a proper array section" }
> +!$acc enter data copyin(x%i%kind)          ! { dg-error "not a proper array section" }
> +!$acc enter data copyin(x%c%len)           ! { dg-error "not a proper array section" }
> +!$acc update self(i%kind, c%len)           ! { dg-error "not a proper array section" }
> +!$acc update self(x%i%kind)                ! { dg-error "not a proper array section" }
> +!$acc update self(x%c%len)                 ! { dg-error "not a proper array section" }
> +
> +! EXPR_VARIABLE / cf. OpenACC spec issue 346
> +!$acc enter data copyin(z%re)   ! { dg-error "Unexpected complex-parts designator" }
> +!$acc enter data copyin(z%im)   ! { dg-error "Unexpected complex-parts designator" }

Need to check that for 'zz', too?  (See below.)

> +!$acc enter data copyin(x%z%re) ! { dg-error "Unexpected complex-parts designator" }
> +!$acc enter data copyin(x%z%im) ! { dg-error "Unexpected complex-parts designator" }
> +!$acc update self(z%re)         ! { dg-error "Unexpected complex-parts designator" }
> +!$acc update self(z%im)         ! { dg-error "Unexpected complex-parts designator" }
> +!$acc update self(zz%re)
> +!$acc update self(zz%im)
> +end

And for OpenACC, the 'zz' variants do not emit this error message here.
(That's not immediately obvious to me.)

I can see how data mapping of '[...]%re' etc. are problematic (we're
constructing an "incomplete object"?), but 'update' etc. I'd have
expected to work: would just copy the respective "part".

That, again, is however just my gut feeling, without having done any
actual research.


Grüße
 Thomas
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf