Re: [nvptx, committed] Unify C/Fortran routine handling in nvptx_goacc_validate_dims
Hi!
On Mon, 17 Dec 2018 22:46:50 +0100, Tom de Vries wrote:
> [ was: Re: [nvptx] vector length patch series ]
>
> On 14-12-18 20:58, Tom de Vries wrote:
> >> 0004-openacc-Make-GFC-default-to-1-for-OpenACC-routine-di.patch
> > If I remove this, I run into ICEs in the compiler, but I think that
> > means we need to understand and fix that ICE, instead of concluding that
> > we need this patch. It looks completely unrelated.
>
> Indeed this
> (0004-openacc-Make-GFC-default-to-1-for-OpenACC-routine-di.patch) patch
> is unrelated to the vector length functionality.
>
> However, it fixes a problem in the Fortran front-end which has as
> consequence that C and Fortran routines look the same in
> nvptx_goacc_validate_dims, which is a good thing to have.
>
> However, the upstreaming of the patch seems to be stuck, so I've
> committed an nvptx workaround patch that has the same effect, allowing
> us to drop it
> (0004-openacc-Make-GFC-default-to-1-for-OpenACC-routine-di.patch) from
> the patch series.
ACK, thanks.
> [nvptx] Unify C/Fortran routine handling in nvptx_goacc_validate_dims
>
> The Fortran front-end has a bug (PR72741) that means what when
> nvptx_goacc_validate_dims is called for a Fortran routine, the dims parameter
> is not the same as it would have been if the function would have been called
> for
> an equivalent C routine.
>
> Work around this bug by overriding the dims parameter for routines, allowing
> the
> function to handle routines in Fortran and C the same.
I have now finally identified the relevant changes (scattered over
several commits on the OpenACC development branch, each of these trying
to do too many things at once, but also incompletely...), and then have
rewritten most of it anyway, into a more pleasant form, and now committed
to trunk in r269105 "[PR72741] Use 'oacc_build_routine_dims' for Fortran
OpenACC 'routine' directives, too", as attached.
Grüße
Thomas
From 1d740b07b3ba5b15b7ece7fdb25236e32251131a Mon Sep 17 00:00:00 2001
From: tschwinge
Date: Fri, 22 Feb 2019 10:50:35 +
Subject: [PATCH 4/9] [PR72741] Use 'oacc_build_routine_dims' for Fortran
OpenACC 'routine' directives, too
... instead of having an incomplete local implementation.
With these changes in place, we can then also revert the work-around r267213
"[nvptx] Unify C/Fortran routine handling in nvptx_goacc_validate_dims".
gcc/fortran/
PR fortran/72741
* gfortran.h (oacc_routine_lop): New enum.
(symbol_attribute): Use it.
* openmp.c (gfc_oacc_routine_dims): Replace with...
(gfc_oacc_routine_lop): ... this new function.
(gfc_match_oacc_routine): Adjust.
* trans-decl.c (add_attributes_to_decl): Likewise.
gcc/
PR fortran/72741
* omp-general.c (oacc_replace_fn_attrib): Mostly split out into...
(oacc_replace_fn_attrib_attr): ... this new function.
* omp-general.h (oacc_replace_fn_attrib_attr): New prototype.
* config/nvptx/nvptx.c (nvptx_goacc_validate_dims_1): Revert workaround.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/classify-routine.f95: Adjust.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269105 138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/ChangeLog | 8
gcc/config/nvptx/nvptx.c | 35
gcc/fortran/ChangeLog | 11 +
gcc/fortran/gfortran.h| 13 +-
gcc/fortran/openmp.c | 41 +++
gcc/fortran/trans-decl.c | 34 ++-
gcc/omp-general.c | 18 ++--
gcc/omp-general.h | 1 +
gcc/testsuite/ChangeLog | 3 ++
.../gfortran.dg/goacc/classify-routine.f95| 4 +-
10 files changed, 99 insertions(+), 69 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4745a1999d9..f14cbbce477 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2019-02-22 Thomas Schwinge
+
+ PR fortran/72741
+ * omp-general.c (oacc_replace_fn_attrib): Mostly split out into...
+ (oacc_replace_fn_attrib_attr): ... this new function.
+ * omp-general.h (oacc_replace_fn_attrib_attr): New prototype.
+ * config/nvptx/nvptx.c (nvptx_goacc_validate_dims_1): Revert workaround.
+
2019-02-22 Kyrylo Tkachov
* config/arm/arm-cpus.in (ares): Rename to...
diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 23459e1c6f4..424b43ac8b4 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -5577,41 +5577,6 @@ nvptx_goacc_validate_dims_1 (tree decl, int dims[], int fn_level, unsigned used)
else
gcc_unreachable ();
- if (routine_p)
-{
- /* OpenACC routines in C arrive here with the following attributes
- (omitting the 'omp declare target'):
- seq : __attribute__((oacc function (0 1, 0 1, 0 1)))
- vector: __attribute__((oacc function (0 1, 0 1, 1 0)))
- worker: __attribute__((oacc function (0 1, 1 0, 1 0)))
- gang : __attribute__((oacc function (1 0,
[nvptx, committed] Unify C/Fortran routine handling in nvptx_goacc_validate_dims
[ was: Re: [nvptx] vector length patch series ]
On 14-12-18 20:58, Tom de Vries wrote:
>> 0004-openacc-Make-GFC-default-to-1-for-OpenACC-routine-di.patch
> If I remove this, I run into ICEs in the compiler, but I think that
> means we need to understand and fix that ICE, instead of concluding that
> we need this patch. It looks completely unrelated.
Indeed this
(0004-openacc-Make-GFC-default-to-1-for-OpenACC-routine-di.patch) patch
is unrelated to the vector length functionality.
However, it fixes a problem in the Fortran front-end which has as
consequence that C and Fortran routines look the same in
nvptx_goacc_validate_dims, which is a good thing to have.
However, the upstreaming of the patch seems to be stuck, so I've
committed an nvptx workaround patch that has the same effect, allowing
us to drop it
(0004-openacc-Make-GFC-default-to-1-for-OpenACC-routine-di.patch) from
the patch series.
Thanks,
- Tom
[nvptx] Unify C/Fortran routine handling in nvptx_goacc_validate_dims
The Fortran front-end has a bug (PR72741) that means what when
nvptx_goacc_validate_dims is called for a Fortran routine, the dims parameter
is not the same as it would have been if the function would have been called for
an equivalent C routine.
Work around this bug by overriding the dims parameter for routines, allowing the
function to handle routines in Fortran and C the same.
Build and reg-tested on x86_64 with nvptx accelerator.
2018-12-17 Tom de Vries
* config/nvptx/nvptx.c (nvptx_goacc_validate_dims): Work around Fortran
bug PR72741 by overriding dims parameter for routines.
---
gcc/config/nvptx/nvptx.c | 36
1 file changed, 36 insertions(+)
diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 746d8bfb100..24727ad5920 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -5212,6 +5212,42 @@ nvptx_goacc_validate_dims (tree decl, int dims[], int fn_level)
else
gcc_unreachable ();
+ if (routine_p)
+{
+ /* OpenACC routines in C arrive here with the following attributes
+ (omitting the 'omp declare target'):
+ seq : __attribute__((oacc function (0 1, 0 1, 0 1)))
+ vector: __attribute__((oacc function (0 1, 0 1, 1 0)))
+ worker: __attribute__((oacc function (0 1, 1 0, 1 0)))
+ gang : __attribute__((oacc function (1 0, 1 0, 1 0)))
+
+ If we take f.i. the oacc function attribute of the worker routine
+ (0 1, 1 0, 1 0), then:
+ - the slice (0, 1, 1) is interpreted by oacc_fn_attrib_level as
+ meaning: worker routine, that is:
+ - can't contain gang loop (0),
+ - can contain worker loop (1),
+ - can contain vector loop (1).
+ - the slice (1, 0, 0) is interpreted by oacc_validate_dims as the
+ dimensions: gang: 1, worker: 0, vector: 0.
+
+ OTOH, routines in Fortran arrive here with these attributes:
+ seq : __attribute__((oacc function (0 0, 0 0, 0 0)))
+ vector: __attribute__((oacc function (0 0, 0 0, 1 0)))
+ worker: __attribute__((oacc function (0 0, 1 0, 1 0)))
+ gang : __attribute__((oacc function (1 0, 1 0, 1 0)))
+ that is, the same as for C but with the dimensions set to 0.
+
+ This is due to a bug in the Fortran front-end: PR72741. Work around
+ this bug by forcing the dimensions to be the same in Fortran as for C,
+ to be able to handle C and Fortran routines uniformly in this
+ function. */
+ dims[GOMP_DIM_VECTOR] = fn_level > GOMP_DIM_VECTOR ? 1 : 0;
+ dims[GOMP_DIM_WORKER] = fn_level > GOMP_DIM_WORKER ? 1 : 0;
+ dims[GOMP_DIM_GANG] = fn_level > GOMP_DIM_GANG ? 1 : 0;
+ changed = true;
+}
+
/* The vector size must be 32, unless this is a SEQ routine. */
if ((offload_region_p || oacc_default_dims_p
|| (routine_p && !routine_seq_p))
