| Message ID | YmA9iaVvWhwVGFmT@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | fortran: Fix up gfc_trans_oacc_construct [PR104717] | expand |
Hi! On 2022-04-20T19:06:17+0200, Jakub Jelinek <jakub@redhat.com> wrote: > So that move_sese_region_to_fn works properly, OpenMP/OpenACC constructs > for which that function is invoked need an extra artificial BIND_EXPR > around their body so that we move all variables of the bodies. > > The C/C++ FEs do that both for OpenMP constructs like OMP_PARALLEL, OMP_TASK > or OMP_TARGET and for OpenACC constructs that behave similarly to > OMP_TARGET, but the Fortran FE only does that for OpenMP constructs. > > The following patch does that for OpenACC constructs too. > This fixes ICE on the attached testcase. ACK, thanks. I do see that over the past years, there have been a number of similar fixes been applied for other OMP constructs -- maybe that interface should generally be made more obvious and fool-proof? > Unfortunately, it also regresses > FAIL: gfortran.dg/goacc/privatization-1-compute-loop.f90 -O (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -foffload=disable -O0 (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -foffload=disable -O1 (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -foffload=disable -O2 (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -foffload=disable -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -foffload=disable -O3 -g (test for excess errors) > FAIL: libgomp.oacc-fortran/privatized-ref-2.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -foffload=disable -Os (test for excess errors) > Those emits emit tons of various messages and now there are some extra ones, I've fixed these up. > the previous as well as new ones are mostly on artificial variables created > by the compiler, so I wonder if we should emit those at all. Yes, I did wonder about that, too. (This affects a lot of test cases.) > Anyway, here it is the patch, appart from those regressions passed > bootstrap/regtested on powerpc64le-linux. > > 2022-04-20 Jakub Jelinek <jakub@redhat.com> > > PR fortran/104717 > * trans-openmp.cc (gfc_trans_oacc_construct): Wrap construct body > in an extra BIND_EXPR. > > * gfortran.dg/goacc/pr104717.f90: New test. > > --- gcc/fortran/trans-openmp.cc.jj 2022-04-06 09:59:32.729654664 +0200 > +++ gcc/fortran/trans-openmp.cc 2022-04-20 12:48:19.773402677 +0200 > @@ -4444,7 +4444,9 @@ gfc_trans_oacc_construct (gfc_code *code > gfc_start_block (&block); > oacc_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses, > code->loc, false, true); > + pushlevel (); > stmt = gfc_trans_omp_code (code->block->next, true); > + stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0)); > stmt = build2_loc (gfc_get_location (&code->loc), construct_code, > void_type_node, stmt, oacc_clauses); > gfc_add_expr_to_block (&block, stmt); > --- gcc/testsuite/gfortran.dg/goacc/pr104717.f90.jj 2022-04-20 12:53:54.002748265 +0200 > +++ gcc/testsuite/gfortran.dg/goacc/pr104717.f90 2022-04-20 12:53:12.811321862 +0200 > @@ -0,0 +1,22 @@ > +! PR fortran/104717 > +! { dg-do compile } > +! { dg-options "-O1 -fopenacc -fstack-arrays" } As the ICE was "during IPA pass: pta", I've added an explicit '-fipa-pta' here. > + > +program main > + implicit none (type, external) > + integer :: j > + integer, allocatable :: A(:) > + > + A = [(3*j, j=1, 10)] > + call foo (A, size(A)) > + deallocate (A) > +contains > + subroutine foo (array, nn) > + integer :: i, nn > + integer :: array(nn) > + > + !$acc parallel copyout(array) > + array = [(-i, i = 1, nn)] > + !$acc end parallel > + end subroutine foo > +end Pushed to master branch commit b2202431910e30d8505c94d1cb9341cac7080d10 "fortran: Fix up gfc_trans_oacc_construct [PR104717]", see attached. Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
--- gcc/fortran/trans-openmp.cc.jj 2022-04-06 09:59:32.729654664 +0200 +++ gcc/fortran/trans-openmp.cc 2022-04-20 12:48:19.773402677 +0200 @@ -4444,7 +4444,9 @@ gfc_trans_oacc_construct (gfc_code *code gfc_start_block (&block); oacc_clauses = gfc_trans_omp_clauses (&block, code->ext.omp_clauses, code->loc, false, true); + pushlevel (); stmt = gfc_trans_omp_code (code->block->next, true); + stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0)); stmt = build2_loc (gfc_get_location (&code->loc), construct_code, void_type_node, stmt, oacc_clauses); gfc_add_expr_to_block (&block, stmt); --- gcc/testsuite/gfortran.dg/goacc/pr104717.f90.jj 2022-04-20 12:53:54.002748265 +0200 +++ gcc/testsuite/gfortran.dg/goacc/pr104717.f90 2022-04-20 12:53:12.811321862 +0200 @@ -0,0 +1,22 @@ +! PR fortran/104717 +! { dg-do compile } +! { dg-options "-O1 -fopenacc -fstack-arrays" } + +program main + implicit none (type, external) + integer :: j + integer, allocatable :: A(:) + + A = [(3*j, j=1, 10)] + call foo (A, size(A)) + deallocate (A) +contains + subroutine foo (array, nn) + integer :: i, nn + integer :: array(nn) + + !$acc parallel copyout(array) + array = [(-i, i = 1, nn)] + !$acc end parallel + end subroutine foo +end