Bug 63861 - OpenACC coarray ICE (also with OpenMP?)
Summary: OpenACC coarray ICE (also with OpenMP?)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: openacc, openmp
Depends on:
Blocks:
 
Reported: 2014-11-14 00:51 UTC by Cesar Philippidis
Modified: 2015-12-02 20:06 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-10-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Cesar Philippidis 2014-11-14 00:51:20 UTC
Coarrays in OpenACC accelerated regions causes an ICE in gfortran. The test case gcc/testsuite/gfortran.dg/goacc/coarray.f95 reproduces this failure. It's unclear whether OpenACC 2.0a even supports coarrays. Regardless, this test case shouldn't cause an ICE.
Comment 1 Tobias Burnus 2015-01-22 09:59:59 UTC
(In reply to Cesar Philippidis from comment #0)
> Coarrays in OpenACC accelerated regions causes an ICE in gfortran. The test
> case gcc/testsuite/gfortran.dg/goacc/coarray.f95 reproduces this failure.

It already ICEs for me with a normal variable: PR64726, besides it currently doesn't ICE because CACHE is not yet implemented: PR63865.

Note that the test case should at the end also use -fcoarray=lib besides -fcoarray=single.


> It's unclear whether OpenACC 2.0a even supports coarrays. Regardless, this
> test case shouldn't cause an ICE.

Regarding coarrays: If they are not coindexed ("a[remote_process_idx]"), they act as local variable. Thus, I'd expect that they work. Synchronization is in the realm of the user, who has to ensure that the host has the current value and the processes are sync'ed when remote access is required. [One probably should also reject calls to a procedures which expect a coarray as argument as that won't fly either. That's something to be done on best-effort base.]

Thus, I think they should be permitted.
Comment 2 Tobias Burnus 2015-01-22 12:46:53 UTC
With the patch for PR64726 applied and the ACC CACHE commented, the program gets stuck (endless loop) for the code in gcc/testsuite/gfortran.dg/goacc/coarray.f95.

It works, if one changes the scalar coarray into either an array coarray or into a noncoarray scalar.

I think gfc_has_alloc_comps just needs to be updated to handle coarrays, which use an array descriptor but aren't true arrays. (I think GFC_TYPE_ARRAY_RANK(...) == 0 can be used to detect this, but that might not work for all array types.)


I marked it as "OpenMP" as it is probably also possible to trigger this for some OpenMP code. Even though, I do not have a test case.


Backtrace:

#1  gfc_get_element_type (type=0x7ffff20f25e8) at ../../gcc/fortran/trans-types.c:1180
#2  0x0000000000742f85 in gfc_has_alloc_comps (type=<optimized out>, decl=<optimized out>) at ../../gcc/fortran/trans-openmp.c:193
#3  0x000000000099eb12 in gimplify_scan_omp_clauses (list_p=0x7ffff20f32a0, pre_p=pre_p@entry=0x7fffffffd918, region_type=region_type@entry=ORT_TARGET) at ../../gcc/gimplify.c:6062
#4  0x00000000009a07c8 in gimplify_omp_workshare (expr_p=expr_p@entry=0x7ffff20eb778, pre_p=pre_p@entry=0x7fffffffd918) at ../../gcc/gimplify.c:7307
Comment 3 Tobias Burnus 2015-01-27 19:58:27 UTC
Author: burnus
Date: Tue Jan 27 19:57:55 2015
New Revision: 220189

URL: https://gcc.gnu.org/viewcvs?rev=220189&root=gcc&view=rev
Log:
2015-01-27  Tobias Burnus  <burnus@net-b.de>

        PR fortran/63861
gcc/fortran/
        * trans-openmp.c (gfc_has_alloc_comps, gfc_trans_omp_clauses):
        Fix handling for scalar coarrays.
        * trans-types.c (gfc_get_element_type): Add comment.
gcc/testsuite/
        * gfortran.dg/goacc/coarray_2.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/goacc/coarray_2.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-openmp.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/testsuite/ChangeLog
Comment 4 Tobias Burnus 2015-01-27 20:09:10 UTC
Comment 3 fixed two issues with gcc/testsuite/gfortran.dg/goacc/coarray.f95.

Still to do:

* gcc/testsuite/gfortran.dg/goacc/coarray_2.f90 contains a FIXME for a "reduction(+:a)" variable.

For an allocatable scalar coarray, that will lead to an ICE in an assignment "a = 0". The problem seems to relate to calling alloc_scalar_allocatable_for_assignment from gfc_trans_assignment_1, which shall not be called at all. (The semantics say that for coarrays those shall always be allocated, when used and noreallocation shall be done. Coarray (de)allocations are always colletively done across all images (= processes)).


I wonder whether there are similar other issues; the same no-realloc also applies to allocatable array coarrays.


* One probably should check for coindexed variables (e.g. "a[i]" or "b(1:8)[5]", where the "[...]" refers to a remote image). For everything handled as pointer or within an offloaded region, that won't work and should be invalid. At places where a temporary is fine, it can be used (cf. dumps with -fcoarray=lib).

I have no idea whether there is an issue in either the OpenMP/OpenACC directives or whether it would be testable for target/acc kernels/parallel regions. Probably only for those declaring a procedure ("routine") to be offloadable.
Comment 5 Dominique d'Humieres 2015-10-20 17:07:29 UTC
Is this fixed after r220189?
Comment 6 Thomas Schwinge 2015-10-27 15:36:01 UTC
(In reply to Dominique d'Humieres from comment #5)
> Is this fixed after r220189?

No.
Comment 7 Dominique d'Humieres 2015-11-03 14:51:12 UTC
The ICE is gone and between revisions r229303 and r229482 compiling the test gcc/testsuite/gfortran.dg/goacc/coarray.f95 with '-fcoarray=single -fopenacc' gives

/opt/gcc/_clean/gcc/testsuite/gfortran.dg/goacc/coarray.f95:13:0:

     !$acc declare device_resident (a)
^
sorry, unimplemented: directive not yet implemented
/opt/gcc/_clean/gcc/testsuite/gfortran.dg/goacc/coarray.f95:21:0:

     !$acc host_data use_device (a)
^
sorry, unimplemented: directive not yet implemented

Is what remains  duplicate of pr63859?
Comment 8 Dominique d'Humieres 2015-12-02 14:23:52 UTC
Compiling gfortran.dg/goacc/coarray.f95 with '-fcoarray=single -fopenacc' gives now an ICE

/opt/gcc/_clean/gcc/testsuite/gfortran.dg/goacc/coarray.f95:23:0:

     !$acc parallel loop reduction(+:a)
 

internal compiler error: in lower_oacc_reductions, at omp-low.c:5437

The change occurred between revisions r231086 (no ICE) and r231131 (ICE).
Comment 9 Dominique d'Humieres 2015-12-02 14:45:56 UTC
> The change occurred between revisions r231086 (no ICE) and r231131 (ICE).

Reduced range from https://gcc.gnu.org/ml/gcc-regression/2015-12/

r231109 (no ICE), r231121 (ICE), likely r231112 or r231118.
Comment 10 cesar 2015-12-02 15:31:23 UTC
Thanks for reducing this Dominique. I'm not sure what caused the ICE yet, I don't think this test case is valid anymore. 

Basically, this test is trying to preform a reduction on a coarrray. Unlike openmp, openacc does not support array reductions. In fact, if you convert 'a' to a regular array, it would also cause an ICE.

One of the solutions I was considering was to error if any array variables are used in as reduction variables. But I was thinking that would be too conservative. Consider 'reduction(+:var(1))'. Here var is an array, but the reduction is specifically operating on var element 1.

The problem I came across here is that reductions on array elements in openmp are rejected as errors and I not sure why. I spent some time looking at it, but I kept on getting preempted by other tasks. This may be the case where the behavior of reductions in openmp diverges from openacc. 

With that in mind, I'm not sure why the ICE was exposed all of the sudden. My changes mostly updated the way that gang is parsed and how clauses in combined constructs are split. It looks like a sorry disappeared because the ICE occurs in omp-low.c.
Comment 11 Thomas Schwinge 2015-12-02 16:09:13 UTC
(In reply to cesar from comment #10)
> Thanks for reducing this Dominique. I'm not sure what caused the ICE yet, I
> don't think this test case is valid anymore. 

But still shouldn't result in an ICE.

> Basically, this test is trying to preform a reduction on a coarrray. Unlike
> openmp, openacc does not support array reductions. In fact, if you convert
> 'a' to a regular array, it would also cause an ICE.

Should possibly be rejected with an appropriate error diagnostic, but not cause an ICE.

> One of the solutions I was considering was to error if any array variables
> are used in as reduction variables. But I was thinking that would be too
> conservative. Consider 'reduction(+:var(1))'. Here var is an array, but the
> reduction is specifically operating on var element 1.

Hmm, but is that useful enough to consider this special case?

> The problem I came across here is that reductions on array elements in
> openmp are rejected as errors and I not sure why. I spent some time looking
> at it, but I kept on getting preempted by other tasks. This may be the case
> where the behavior of reductions in openmp diverges from openacc. 
> 
> With that in mind, I'm not sure why the ICE was exposed all of the sudden.
> My changes mostly updated the way that gang is parsed and how clauses in
> combined constructs are split. It looks like a sorry disappeared because the
> ICE occurs in omp-low.c.

Quite right ("sorry disappeared"; see <http://news.gmane.org/find-root.php?message_id=%3C877fkwn8p6.fsf%40kepler.schwinge.homeip.net%3E>.
Comment 12 cesar 2015-12-02 17:08:16 UTC
(In reply to Thomas Schwinge from comment #11)
> (In reply to cesar from comment #10)
> > Thanks for reducing this Dominique. I'm not sure what caused the ICE yet, I
> > don't think this test case is valid anymore. 
> 
> But still shouldn't result in an ICE.

I agree.

> > Basically, this test is trying to preform a reduction on a coarrray. Unlike
> > openmp, openacc does not support array reductions. In fact, if you convert
> > 'a' to a regular array, it would also cause an ICE.
> 
> Should possibly be rejected with an appropriate error diagnostic, but not
> cause an ICE.

Yeah, I know. I was just raising this as a known issue that I'm working on.

> > One of the solutions I was considering was to error if any array variables
> > are used in as reduction variables. But I was thinking that would be too
> > conservative. Consider 'reduction(+:var(1))'. Here var is an array, but the
> > reduction is specifically operating on var element 1.
> 
> Hmm, but is that useful enough to consider this special case?

Probably. Besides it also affects the c and c++ front ends. 

> > The problem I came across here is that reductions on array elements in
> > openmp are rejected as errors and I not sure why. I spent some time looking
> > at it, but I kept on getting preempted by other tasks. This may be the case
> > where the behavior of reductions in openmp diverges from openacc. 
> > 
> > With that in mind, I'm not sure why the ICE was exposed all of the sudden.
> > My changes mostly updated the way that gang is parsed and how clauses in
> > combined constructs are split. It looks like a sorry disappeared because the
> > ICE occurs in omp-low.c.
> 
> Quite right ("sorry disappeared"; see
> <http://news.gmane.org/find-root.php?message_id=%3C877fkwn8p6.fsf%40kepler.
> schwinge.homeip.net%3E>.

Ok. I'm testing a patch which teaches the fortran front end to error on any type of array reductions. It's a little bit conservative because it doesn't permit reductions on array elements yet. But at least it doesn't ICE. If you want, I can take a look at those gimplifier changes once I'm done with this. Otherwise, I still need to work on the c++ routine error messages.
Comment 13 cesar 2015-12-02 19:59:58 UTC
Author: cesar
Date: Wed Dec  2 19:59:27 2015
New Revision: 231204

URL: https://gcc.gnu.org/viewcvs?rev=231204&root=gcc&view=rev
Log:
	gcc/fortran/
	PR fortran/63861
	* openmp.c (gfc_match_omp_clauses): Allow subarrays for acc reductions.
	(resolve_omp_clauses): Error on any acc reductions on arrays.

	gcc/testsuite/
	* gfortran.dg/goacc/array-reduction.f90: New test.
	* gfortran.dg/goacc/assumed.f95: Update expected diagnostics.
	* gfortran.dg/goacc/coarray.f95: Likewise.
	* gfortran.dg/goacc/coarray_2.f90: Likewise.
	* gfortran.dg/goacc/reduction-2.f95: Likewise.
	* gfortran.dg/goacc/reduction.f95: Likewise.


Added:
    trunk/gcc/testsuite/gfortran.dg/goacc/array-reduction.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/openmp.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/goacc/assumed.f95
    trunk/gcc/testsuite/gfortran.dg/goacc/coarray.f95
    trunk/gcc/testsuite/gfortran.dg/goacc/coarray_2.f90
    trunk/gcc/testsuite/gfortran.dg/goacc/reduction-2.f95
    trunk/gcc/testsuite/gfortran.dg/goacc/reduction.f95
Comment 14 cesar 2015-12-02 20:06:29 UTC
Fixed in r231204. Array reductions aren't supported in OpenACC 2.0. So now the fortran front end generates error when it encounters such a reduction.