* [PATCH] OpenACC routines in fortran modules
@ 2016-07-01 20:41 ` Cesar Philippidis
2016-07-28 9:55 ` Tobias Burnus
2016-08-11 15:19 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules) Thomas Schwinge
0 siblings, 2 replies; 15+ messages in thread
From: Cesar Philippidis @ 2016-07-01 20:41 UTC (permalink / raw)
To: gcc-patches, Fortran List, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 987 bytes --]
It turns out that the acc routine parallelism isn't being recorded in
fortran .mod files. This is a problem because then the ME can't validate
if a routine has compatible parallelism with the call site. This patch
does two things:
1. Encode gang, worker, vector and seq level parallelism in module
files. This introduces a new oacc_function enum, which I ended
up using to record the parallelism of standalone acc routines too.
2. Extends gfc_match_oacc_routine to add acc routine directive support
for intrinsic procedures such as abort.
Is this patch OK for trunk? I included support for intrinsic procedures
because it was necessary with my previous patch which treated all calls
to non-acc routines from within an OpenACC offloaded region as errors.
Now that it has been determined that those patches should be link time
errors, we technically don't need to add acc routine support for
intrinsic procedures. So I can drop that part of the patch if necessary.
Cesar
[-- Attachment #2: fortran-module-routines.diff --]
[-- Type: text/x-patch, Size: 13869 bytes --]
2016-07-01 Cesar Philippidis <cesar@codesourcery.com>
gcc/fortran/
* gfortran.h (enum oacc_function): Define.
(oacc_function_type): Declare.
(symbol_attribute): Change the type of oacc_function from unsigned
to an ENUM_BITFIELD.
* module.c (oacc_function): New DECL_MIO_NAME.
(mio_symbol_attribute): Set the oacc_function attribute.
* openmp.c (gfc_oacc_routine_dims): Change the return type from
int to oacc_function.
(gfc_match_oacc_routine): Handle intrinsic procedures.
* symbol.c (oacc_function_types): Define.
* trans-decl.c (add_attributes_to_decl): Update to handle the
retyped oacc_function attribute.
gcc/testsuite/
* gfortran.dg/goacc/fixed-1.f: Add test coverage.
* gfortran.dg/goacc/routine-7.f90: New test.
libgomp/
* testsuite/libgomp.oacc-fortran/abort-1.f90: Test acc routine
on intrinsic abort.
* testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f: Likewise.
* testsuite/libgomp.oacc-fortran/routine-7.f90: Likewise.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 0bb71cb..fac94ca 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -303,6 +303,15 @@ enum save_state
{ SAVE_NONE = 0, SAVE_EXPLICIT, SAVE_IMPLICIT
};
+/* Flags to keep track of ACC routine states. */
+enum oacc_function
+{ OACC_FUNCTION_NONE = 0,
+ OACC_FUNCTION_SEQ,
+ OACC_FUNCTION_GANG,
+ OACC_FUNCTION_WORKER,
+ OACC_FUNCTION_VECTOR
+};
+
/* Strings for all symbol attributes. We use these for dumping the
parse tree, in error messages, and also when reading and writing
modules. In symbol.c. */
@@ -312,6 +321,7 @@ extern const mstring intents[];
extern const mstring access_types[];
extern const mstring ifsrc_types[];
extern const mstring save_status[];
+extern const mstring oacc_function_types[];
/* Enumeration of all the generic intrinsic functions. Used by the
backend for identification of a function. */
@@ -862,7 +872,7 @@ typedef struct
unsigned oacc_declare_link:1;
/* This is an OpenACC acclerator function at level N - 1 */
- unsigned oacc_function:3;
+ ENUM_BITFIELD (oacc_function) oacc_function:3;
/* Attributes set by compiler extensions (!GCC$ ATTRIBUTES). */
unsigned ext_attr:EXT_ATTR_NUM;
diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index 4d664f0..267858f 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -2095,6 +2095,7 @@ DECL_MIO_NAME (procedure_type)
DECL_MIO_NAME (ref_type)
DECL_MIO_NAME (sym_flavor)
DECL_MIO_NAME (sym_intent)
+DECL_MIO_NAME (oacc_function)
#undef DECL_MIO_NAME
/* Symbol attributes are stored in list with the first three elements
@@ -2116,6 +2117,8 @@ mio_symbol_attribute (symbol_attribute *attr)
attr->proc = MIO_NAME (procedure_type) (attr->proc, procedures);
attr->if_source = MIO_NAME (ifsrc) (attr->if_source, ifsrc_types);
attr->save = MIO_NAME (save_state) (attr->save, save_status);
+ attr->oacc_function = MIO_NAME (oacc_function) (attr->oacc_function,
+ oacc_function_types);
ext_attr = attr->ext_attr;
mio_integer ((int *) &ext_attr);
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 865e0d9..10b880c 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -1714,21 +1714,31 @@ gfc_match_oacc_cache (void)
/* Determine the loop level for a routine. */
-static int
+static oacc_function
gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
{
int level = -1;
+ oacc_function ret = OACC_FUNCTION_SEQ;
if (clauses)
{
unsigned mask = 0;
if (clauses->gang)
- level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
+ {
+ level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
+ ret = OACC_FUNCTION_GANG;
+ }
if (clauses->worker)
- level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
+ {
+ level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
+ ret = OACC_FUNCTION_WORKER;
+ }
if (clauses->vector)
- level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
+ {
+ level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
+ ret = OACC_FUNCTION_VECTOR;
+ }
if (clauses->seq)
level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
@@ -1736,10 +1746,7 @@ gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
gfc_error ("Multiple loop axes specified for routine");
}
- if (level < 0)
- level = GOMP_DIM_MAX;
-
- return level;
+ return ret;
}
match
@@ -1750,6 +1757,7 @@ gfc_match_oacc_routine (void)
match m;
gfc_omp_clauses *c = NULL;
gfc_oacc_routine_name *n = NULL;
+ gfc_intrinsic_sym *isym = NULL;
old_loc = gfc_current_locus;
@@ -1767,12 +1775,14 @@ gfc_match_oacc_routine (void)
if (m == MATCH_YES)
{
char buffer[GFC_MAX_SYMBOL_LEN + 1];
- gfc_symtree *st;
+ gfc_symtree *st = NULL;
m = gfc_match_name (buffer);
if (m == MATCH_YES)
{
- st = gfc_find_symtree (gfc_current_ns->sym_root, buffer);
+ if ((isym = gfc_find_function (buffer)) == NULL
+ && (isym = gfc_find_subroutine (buffer)) == NULL)
+ st = gfc_find_symtree (gfc_current_ns->sym_root, buffer);
if (st)
{
sym = st->n.sym;
@@ -1780,7 +1790,7 @@ gfc_match_oacc_routine (void)
sym = NULL;
}
- if (st == NULL
+ if ((st == NULL && isym == NULL)
|| (sym
&& !sym->attr.external
&& !sym->attr.function
@@ -1814,7 +1824,10 @@ gfc_match_oacc_routine (void)
!= MATCH_YES))
return MATCH_ERROR;
- if (sym != NULL)
+ if (isym != NULL)
+ /* There is nothing to do for intrinsic procedures. */
+ ;
+ else if (sym != NULL)
{
n = gfc_get_oacc_routine_name ();
n->sym = sym;
@@ -1832,7 +1845,7 @@ gfc_match_oacc_routine (void)
&old_loc))
goto cleanup;
gfc_current_ns->proc_name->attr.oacc_function
- = gfc_oacc_routine_dims (c) + 1;
+ = gfc_oacc_routine_dims (c);
}
if (n)
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 0ee7dec..b1dd32b 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -87,6 +87,15 @@ const mstring save_status[] =
minit ("IMPLICIT-SAVE", SAVE_IMPLICIT),
};
+const mstring oacc_function_types[] =
+{
+ minit ("NONE", OACC_FUNCTION_NONE),
+ minit ("OACC_FUNCTION_SEQ", OACC_FUNCTION_SEQ),
+ minit ("OACC_FUNCTION_GANG", OACC_FUNCTION_GANG),
+ minit ("OACC_FUNCTION_WORKER", OACC_FUNCTION_WORKER),
+ minit ("OACC_FUNCTION_VECTOR", OACC_FUNCTION_VECTOR)
+};
+
/* This is to make sure the backend generates setup code in the correct
order. */
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 2f5e434..04f9860 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -1327,11 +1327,26 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list)
list = tree_cons (get_identifier ("omp declare target"),
NULL_TREE, list);
- if (sym_attr.oacc_function)
+ if (sym_attr.oacc_function != OACC_FUNCTION_NONE)
{
tree dims = NULL_TREE;
int ix;
- int level = sym_attr.oacc_function - 1;
+ int level = GOMP_DIM_MAX;
+
+ switch (sym_attr.oacc_function)
+ {
+ case OACC_FUNCTION_GANG:
+ level = GOMP_DIM_GANG;
+ break;
+ case OACC_FUNCTION_WORKER:
+ level = GOMP_DIM_WORKER;
+ break;
+ case OACC_FUNCTION_VECTOR:
+ level = GOMP_DIM_VECTOR;
+ break;
+ case OACC_FUNCTION_SEQ:
+ default:;
+ }
for (ix = GOMP_DIM_MAX; ix--;)
dims = tree_cons (build_int_cst (boolean_type_node, ix >= level),
diff --git a/gcc/testsuite/gfortran.dg/goacc/fixed-1.f b/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
index 6a454190..0c0fb98 100644
--- a/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
+++ b/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
@@ -1,3 +1,5 @@
+!$ACC ROUTINE(ABORT) SEQ
+
INTEGER :: ARGC
ARGC = COMMAND_ARGUMENT_COUNT ()
diff --git a/gcc/testsuite/gfortran.dg/goacc/routine-7.f90 b/gcc/testsuite/gfortran.dg/goacc/routine-7.f90
new file mode 100644
index 0000000..e1e0ab7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/routine-7.f90
@@ -0,0 +1,69 @@
+! Test acc routines inside modules.
+
+! { dg-additional-options "-O0" }
+
+module routines
+contains
+ subroutine vector
+ implicit none
+ !$acc routine vector
+ end subroutine vector
+
+ subroutine worker
+ implicit none
+ !$acc routine worker
+ end subroutine worker
+
+ subroutine gang
+ implicit none
+ !$acc routine gang
+ end subroutine gang
+
+ subroutine seq
+ implicit none
+ !$acc routine seq
+ end subroutine seq
+end module routines
+
+program main
+ use routines
+ implicit none
+
+ integer :: i
+
+ !$acc parallel loop gang
+ do i = 1, 10
+ call gang ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call worker
+ call vector
+ call seq
+ end do
+ !$acc end parallel loop
+
+ !$acc parallel loop worker
+ do i = 1, 10
+ call gang ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call worker ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call vector
+ call seq
+ end do
+ !$acc end parallel loop
+
+ !$acc parallel loop vector
+ do i = 1, 10
+ call gang ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call worker ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call vector ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call seq
+ end do
+ !$acc end parallel loop
+
+ !$acc parallel loop seq
+ do i = 1, 10
+ call gang
+ call worker
+ call vector
+ call seq
+ end do
+ !$acc end parallel loop
+end program main
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90
index b38303d..48ebc38 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90
@@ -1,5 +1,6 @@
program main
implicit none
+ !$acc routine(abort) seq
print *, "CheCKpOInT"
!$acc parallel
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f
index a19045b..cbd1dd9 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f
+++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f
@@ -6,6 +6,7 @@
USE OPENACC
IMPLICIT NONE
+!$ACC ROUTINE(ABORT) SEQ
!Host.
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90 b/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
index 200188e..07cd6d9 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
@@ -1,121 +1,95 @@
+! Test acc routines inside modules.
! { dg-do run }
-! { dg-additional-options "-cpp" }
-#define M 8
-#define N 32
+module routines
+ integer, parameter :: N = 32
-program main
- integer :: i
- integer :: a(N)
- integer :: b(M * N)
-
- do i = 1, N
- a(i) = 0
- end do
+contains
+ subroutine vector (a)
+ implicit none
+ !$acc routine vector
+ integer, intent (inout) :: a(N)
+ integer :: i
- !$acc parallel copy (a)
- !$acc loop seq
+ !$acc loop vector
do i = 1, N
- call seq (a)
+ a(i) = 1
end do
- !$acc end parallel
+ end subroutine vector
- do i = 1, N
- if (a(i) .ne.N) call abort
- end do
+ subroutine worker (a)
+ implicit none
+ !$acc routine worker
+ integer, intent (inout) :: a(N)
+ integer :: i
- !$acc parallel copy (a)
- !$acc loop seq
- do i = 1, N
- call gang (a)
+ !$acc loop worker
+ do i = 1, N
+ a(i) = 2
end do
- !$acc end parallel
-
- do i = 1, N
- if (a(i) .ne. (N + (N * (-1 * i)))) call abort
- end do
+ end subroutine worker
- do i = 1, N
- b(i) = i
- end do
+ subroutine gang (a)
+ implicit none
+ !$acc routine gang
+ integer, intent (inout) :: a(N)
+ integer :: i
- !$acc parallel copy (b)
- !$acc loop seq
+ !$acc loop gang
do i = 1, N
- call worker (b)
+ a(i) = 3
end do
- !$acc end parallel
+ end subroutine gang
- do i = 1, N
- if (b(i) .ne. N + i) call abort
- end do
+ subroutine seq (a)
+ implicit none
+ !$acc routine seq
+ integer, intent (inout) :: a(N)
+ integer :: i
- do i = 1, N
- a(i) = i
- end do
-
- !$acc parallel copy (a)
- !$acc loop seq
do i = 1, N
- call vector (a)
+ a(i) = 4
end do
- !$acc end parallel
-
- do i = 1, N
- if (a(i) .ne. 0) call abort
- end do
+ end subroutine seq
+end module routines
-contains
+program main
+ use routines
+ implicit none
-subroutine vector (a)
- !$acc routine vector
- integer, intent (inout) :: a(N)
integer :: i
+ integer :: a(N)
+
+ !$acc parallel
+ call seq (a)
+ !$acc end parallel
- !$acc loop vector
do i = 1, N
- a(i) = a(i) - a(i)
+ if (a(i) .ne. 4) call abort
end do
-end subroutine vector
-
-subroutine worker (b)
- !$acc routine worker
- integer, intent (inout) :: b(M*N)
- integer :: i, j
+ !$acc parallel
+ call gang (a)
+ !$acc end parallel
- !$acc loop worker
do i = 1, N
- !$acc loop vector
- do j = 1, M
- b(j + ((i - 1) * M)) = b(j + ((i - 1) * M)) + 1
- end do
+ if (a(i) .ne. 3) call abort
end do
-end subroutine worker
-
-subroutine gang (a)
- !$acc routine gang
- integer, intent (inout) :: a(N)
- integer :: i
+ !$acc parallel
+ call worker (a)
+ !$acc end parallel
- !$acc loop gang
do i = 1, N
- a(i) = a(i) - i
+ if (a(i) .ne. 2) call abort
end do
-end subroutine gang
-
-subroutine seq (a)
- !$acc routine seq
- integer, intent (inout) :: a(M)
- integer :: i
+ !$acc parallel
+ call vector (a)
+ !$acc end parallel
do i = 1, N
- a(i) = a(i) + 1
+ if (a(i) .ne. 1) call abort
end do
-
-end subroutine seq
-
end program main
^ permalink raw reply [flat|nested] 15+ messages in thread
* [gomp4] encode acc routine clauses inside fortran module files
@ 2016-07-28 2:54 Cesar Philippidis
2016-07-29 4:21 ` [gomp4] Fix PR72741 Cesar Philippidis
0 siblings, 1 reply; 15+ messages in thread
From: Cesar Philippidis @ 2016-07-28 2:54 UTC (permalink / raw)
To: gcc-patches, Fortran List
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
This patch contains the following changes:
* Enhance support for OpenACC routine clauses inside fortran module
files. Also, allow the routine directive to be applied to intrinsic
procedures. The trunk patch can be found here:
https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00063.html
* Change an LTO wrapper assert failure to an error when it detects
missing symbols. This situation can arise in offloading code, e.g.
when the user forgets to declare a global variable as offloadable.
The trunk patch can be found here. Part of this patch was already
present in gomp-4_0-branch.
https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00043.html
I've applied this patch gomp-4_0-branch.
Cesar
[-- Attachment #2: gomp4-fortran-routines.diff --]
[-- Type: text/x-patch, Size: 11282 bytes --]
2016-07-27 Cesar Philippidis <cesar@codesourcery.com>
gcc/fortran/
* gfortran.h (enum oacc_function): Define.
(oacc_function_type): Declare.
(symbol_attribute): Change the type of oacc_function from unsigned
to an ENUM_BITFIELD.
* module.c (oacc_function): New DECL_MIO_NAME.
(mio_symbol_attribute): Set the oacc_function attribute.
* openmp.c (gfc_oacc_routine_dims): Change the return type from
int to oacc_function.
(gfc_match_oacc_routine): Handle intrinsic procedures.
* symbol.c (oacc_function_types): Define.
* trans-decl.c (add_attributes_to_decl): Update to handle the
retyped oacc_function attribute.
gcc/
* lto-cgraph.c (input_overwrite_node): Change the assertion to an
error for missing symbols.
gcc/testsuite/
* gfortran.dg/goacc/fixed-1.f: Add test coverage.
* gfortran.dg/goacc/routine-7.f90: New test.
libgomp/
* testsuite/libgomp.oacc-fortran/abort-1.f90:
* testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f: Add test
coverage.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 2327b13..7784e93 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -292,6 +292,15 @@ enum save_state
{ SAVE_NONE = 0, SAVE_EXPLICIT, SAVE_IMPLICIT
};
+/* Flags to keep track of ACC routine states. */
+enum oacc_function
+{ OACC_FUNCTION_NONE = 0,
+ OACC_FUNCTION_SEQ,
+ OACC_FUNCTION_GANG,
+ OACC_FUNCTION_WORKER,
+ OACC_FUNCTION_VECTOR
+};
+
/* Strings for all symbol attributes. We use these for dumping the
parse tree, in error messages, and also when reading and writing
modules. In symbol.c. */
@@ -301,6 +310,7 @@ extern const mstring intents[];
extern const mstring access_types[];
extern const mstring ifsrc_types[];
extern const mstring save_status[];
+extern const mstring oacc_function_types[];
/* Enumeration of all the generic intrinsic functions. Used by the
backend for identification of a function. */
@@ -851,7 +861,7 @@ typedef struct
unsigned oacc_declare_link:1;
/* This is an OpenACC acclerator function at level N - 1 */
- unsigned oacc_function:3;
+ ENUM_BITFIELD (oacc_function) oacc_function:3;
unsigned oacc_function_nohost:1;
/* Attributes set by compiler extensions (!GCC$ ATTRIBUTES). */
diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index 32ee526..6ee81c3 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -2095,6 +2095,7 @@ DECL_MIO_NAME (procedure_type)
DECL_MIO_NAME (ref_type)
DECL_MIO_NAME (sym_flavor)
DECL_MIO_NAME (sym_intent)
+DECL_MIO_NAME (oacc_function)
#undef DECL_MIO_NAME
/* Symbol attributes are stored in list with the first three elements
@@ -2116,6 +2117,8 @@ mio_symbol_attribute (symbol_attribute *attr)
attr->proc = MIO_NAME (procedure_type) (attr->proc, procedures);
attr->if_source = MIO_NAME (ifsrc) (attr->if_source, ifsrc_types);
attr->save = MIO_NAME (save_state) (attr->save, save_status);
+ attr->oacc_function = MIO_NAME (oacc_function) (attr->oacc_function,
+ oacc_function_types);
ext_attr = attr->ext_attr;
mio_integer ((int *) &ext_attr);
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 52c0309..c20a0a3 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -1879,21 +1879,31 @@ gfc_match_oacc_cache (void)
/* Determine the loop level for a routine. */
-static int
+static oacc_function
gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
{
int level = -1;
+ oacc_function ret = OACC_FUNCTION_SEQ;
if (clauses)
{
unsigned mask = 0;
if (clauses->gang)
- level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
+ {
+ level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
+ ret = OACC_FUNCTION_GANG;
+ }
if (clauses->worker)
- level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
+ {
+ level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
+ ret = OACC_FUNCTION_WORKER;
+ }
if (clauses->vector)
- level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
+ {
+ level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
+ ret = OACC_FUNCTION_VECTOR;
+ }
if (clauses->seq)
level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
@@ -1901,10 +1911,7 @@ gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
gfc_error ("Multiple loop axes specified for routine");
}
- if (level < 0)
- level = GOMP_DIM_MAX;
-
- return level;
+ return ret;
}
match
@@ -1915,6 +1922,7 @@ gfc_match_oacc_routine (void)
match m;
gfc_omp_clauses *c = NULL;
gfc_oacc_routine_name *n = NULL;
+ gfc_intrinsic_sym *isym = NULL;
old_loc = gfc_current_locus;
@@ -1932,12 +1940,14 @@ gfc_match_oacc_routine (void)
if (m == MATCH_YES)
{
char buffer[GFC_MAX_SYMBOL_LEN + 1];
- gfc_symtree *st;
+ gfc_symtree *st = NULL;
m = gfc_match_name (buffer);
if (m == MATCH_YES)
{
- st = gfc_find_symtree (gfc_current_ns->sym_root, buffer);
+ if ((isym = gfc_find_function (buffer)) == NULL
+ && (isym = gfc_find_subroutine (buffer)) == NULL)
+ st = gfc_find_symtree (gfc_current_ns->sym_root, buffer);
if (st)
{
sym = st->n.sym;
@@ -1945,7 +1955,7 @@ gfc_match_oacc_routine (void)
sym = NULL;
}
- if (st == NULL
+ if ((st == NULL && isym == NULL)
|| (sym
&& !sym->attr.external
&& !sym->attr.function
@@ -1981,7 +1991,10 @@ gfc_match_oacc_routine (void)
!= MATCH_YES))
return MATCH_ERROR;
- if (sym != NULL)
+ if (isym != NULL)
+ /* There is nothing to do for intrinsic procedures. */
+ ;
+ else if (sym != NULL)
{
n = gfc_get_oacc_routine_name ();
n->sym = sym;
@@ -1999,7 +2012,7 @@ gfc_match_oacc_routine (void)
&old_loc))
goto cleanup;
gfc_current_ns->proc_name->attr.oacc_function
- = gfc_oacc_routine_dims (c) + 1;
+ = gfc_oacc_routine_dims (c);
gfc_current_ns->proc_name->attr.oacc_function_nohost
= c ? c->nohost : false;
}
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 8efd12c..3ef3276 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -86,6 +86,15 @@ const mstring save_status[] =
minit ("IMPLICIT-SAVE", SAVE_IMPLICIT),
};
+const mstring oacc_function_types[] =
+{
+ minit ("NONE", OACC_FUNCTION_NONE),
+ minit ("OACC_FUNCTION_SEQ", OACC_FUNCTION_SEQ),
+ minit ("OACC_FUNCTION_GANG", OACC_FUNCTION_GANG),
+ minit ("OACC_FUNCTION_WORKER", OACC_FUNCTION_WORKER),
+ minit ("OACC_FUNCTION_VECTOR", OACC_FUNCTION_VECTOR)
+};
+
/* This is to make sure the backend generates setup code in the correct
order. */
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 1feee82..5271268 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -1327,11 +1327,26 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list)
NULL_TREE, list);
#endif
- if (sym_attr.oacc_function)
+ if (sym_attr.oacc_function != OACC_FUNCTION_NONE)
{
tree dims = NULL_TREE;
int ix;
- int level = sym_attr.oacc_function - 1;
+ int level = GOMP_DIM_MAX;
+
+ switch (sym_attr.oacc_function)
+ {
+ case OACC_FUNCTION_GANG:
+ level = GOMP_DIM_GANG;
+ break;
+ case OACC_FUNCTION_WORKER:
+ level = GOMP_DIM_WORKER;
+ break;
+ case OACC_FUNCTION_VECTOR:
+ level = GOMP_DIM_VECTOR;
+ break;
+ case OACC_FUNCTION_SEQ:
+ default:;
+ }
for (ix = GOMP_DIM_MAX; ix--;)
dims = tree_cons (build_int_cst (boolean_type_node, ix >= level),
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index 60e73d6..857ce4d 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -1201,15 +1201,8 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
int success = flag_ltrans || (!node->in_other_partition
&& !node->used_from_other_partition);
-
if (!success)
- {
- gcc_assert (flag_openacc);
- if (TREE_CODE (node->decl) == FUNCTION_DECL)
- error ("missing OpenACC %<routine%> function %qD", node->decl);
- else
- error ("missing OpenACC %<declare%> variable %qD", node->decl);
- }
+ error ("Missing %<%s%>", node->name ());
}
/* Return string alias is alias of. */
diff --git a/gcc/testsuite/gfortran.dg/goacc/fixed-1.f b/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
index 6a454190..0c0fb98 100644
--- a/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
+++ b/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
@@ -1,3 +1,5 @@
+!$ACC ROUTINE(ABORT) SEQ
+
INTEGER :: ARGC
ARGC = COMMAND_ARGUMENT_COUNT ()
diff --git a/gcc/testsuite/gfortran.dg/goacc/routine-7.f90 b/gcc/testsuite/gfortran.dg/goacc/routine-7.f90
new file mode 100644
index 0000000..c48269b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/routine-7.f90
@@ -0,0 +1,70 @@
+! Test acc routines inside modules.
+
+! { dg-additional-options "-O0" }
+
+module routines
+ integer a
+contains
+ subroutine vector ! { dg-warning "partitioned but does not contain" }
+ implicit none
+ !$acc routine vector
+ end subroutine vector
+
+ subroutine worker ! { dg-warning "partitioned but does not contain" }
+ implicit none
+ !$acc routine worker
+ end subroutine worker
+
+ subroutine gang ! { dg-warning "partitioned but does not contain" }
+ implicit none
+ !$acc routine gang
+ end subroutine gang
+
+ subroutine seq
+ implicit none
+ !$acc routine seq
+ end subroutine seq
+end module routines
+
+program main
+ use routines
+ implicit none
+
+ integer :: i
+
+ !$acc parallel loop gang
+ do i = 1, 10
+ call gang ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call worker
+ call vector
+ call seq
+ end do
+ !$acc end parallel loop
+
+ !$acc parallel loop worker
+ do i = 1, 10
+ call gang ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call worker ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call vector
+ call seq
+ end do
+ !$acc end parallel loop
+
+ !$acc parallel loop vector
+ do i = 1, 10
+ call gang ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call worker ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call vector ! { dg-error "routine call uses same OpenACC parallelism as containing loop" }
+ call seq
+ end do
+ !$acc end parallel loop
+
+ !$acc parallel loop seq
+ do i = 1, 10
+ call gang
+ call worker
+ call vector
+ call seq
+ end do
+ !$acc end parallel loop
+end program main
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90
index b38303d..48ebc38 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/abort-1.f90
@@ -1,5 +1,6 @@
program main
implicit none
+ !$acc routine(abort) seq
print *, "CheCKpOInT"
!$acc parallel
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f
index a19045b..cbd1dd9 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f
+++ b/libgomp/testsuite/libgomp.oacc-fortran/acc_on_device-1-2.f
@@ -6,6 +6,7 @@
USE OPENACC
IMPLICIT NONE
+!$ACC ROUTINE(ABORT) SEQ
!Host.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] OpenACC routines in fortran modules
2016-07-01 20:41 ` [PATCH] OpenACC routines in fortran modules Cesar Philippidis
@ 2016-07-28 9:55 ` Tobias Burnus
2016-07-28 21:33 ` Cesar Philippidis
2016-08-11 15:19 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules) Thomas Schwinge
1 sibling, 1 reply; 15+ messages in thread
From: Tobias Burnus @ 2016-07-28 9:55 UTC (permalink / raw)
To: Cesar Philippidis, gcc-patches, fortran, Jakub Jelinek
Cesar Philippidis wrote:
> It turns out that the acc routine parallelism isn't being recorded in
> fortran .mod files. This is a problem because then the ME can't validate
> if a routine has compatible parallelism with the call site.
Nothing against saving such information in .mod files. However, I wonder
whether it can happen that one places such an 'acc routine' outside of a
module in one file - and still accesses it from another file. In the simple
non-ACC case, one can have:
!----- one.f90 ----
subroutine foo()
print *, "abc"
end subroutine foo
!---- two.f90 ---
program example
call foo()
end program example
where "foo()" is torn in without any information about it (except that it
is a subroutine, does not require an explicit interface, and takes no
arguments).
I don't know whether the ACC spec requires an explicit interface in that
case (i.e. for acc routines); I bet it does - or at least should. In that
case, something like the following would be valid - and should be supported
as well. (I don't know whether it currently is.)
!----- one.f90 ----
subroutine foo()
!$acc routine gang
.... ! something
end subroutine foo
!---- two.f90 ---
program example
INTERFACE
subroutine foo()
!$acc routine gang
! Nothing here
end subroutine foo
END INTERFACE
call foo()
end program example
Namely, a replication of the declaration of the procedure, including
the "acc routine", in the 'interface'.
(If one concats the two files, I would also expect an error with -fopenacc,
if the "acc routine" doesn't match between "foo" and the "foo" in the
"interface" block.)
Otherwise: Have you checked whether an unmodified gfortran still accepts the
.mod file written by the patched gfortran - and vice versa? Especially if
-fopenacc is not used, backward compatibility of .mod files is a goal.
(Even though we often have to bump the .mod version for major releases.)
Cheers,
Tobias
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] OpenACC routines in fortran modules
2016-07-28 9:55 ` Tobias Burnus
@ 2016-07-28 21:33 ` Cesar Philippidis
0 siblings, 0 replies; 15+ messages in thread
From: Cesar Philippidis @ 2016-07-28 21:33 UTC (permalink / raw)
To: Tobias Burnus, gcc-patches, fortran, Jakub Jelinek
On 07/28/2016 02:55 AM, Tobias Burnus wrote:
> Cesar Philippidis wrote:
>> It turns out that the acc routine parallelism isn't being recorded in
>> fortran .mod files. This is a problem because then the ME can't validate
>> if a routine has compatible parallelism with the call site.
>
> Nothing against saving such information in .mod files. However, I wonder
> whether it can happen that one places such an 'acc routine' outside of a
> module in one file - and still accesses it from another file. In the simple
> non-ACC case, one can have:
>
> !----- one.f90 ----
> subroutine foo()
> print *, "abc"
> end subroutine foo
>
> !---- two.f90 ---
> program example
> call foo()
> end program example
>
> where "foo()" is torn in without any information about it (except that it
> is a subroutine, does not require an explicit interface, and takes no
> arguments).
>
> I don't know whether the ACC spec requires an explicit interface in that
> case (i.e. for acc routines); I bet it does - or at least should. In that
Jakub and I discussed this issue a while ago. There were two major
problems with treating calls to non-acc routines as errors. 1) What do
we do about intrinsic procedures, and 2) how should builtin and
libc/libm functions get handled? Jakub and I came to the conclusion that
the linker should resolve those issues, hence this patch
<https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00043.html> which teaches
the lto wrapper to error when it encounters missing symbols. From a
compiler standpoint, if the user does something like this
!$acc parallel
...
call foo()
...
!$acc end parallel
and if foo isn't marked as an acc routine, then the compiler will treat
that function as having an implicit 'acc routine seq'.
Note that trunk currently generates an error if the user tries apply an
acc routine directive on an intrinsic routine. This patch teaches
gfortran to accept acc routine directives on those procedures. However,
note that those routines aren't automatically parallelized though, i.e.
they are effectively implemented as 'acc routine seq'.
> case, something like the following would be valid - and should be supported
> as well. (I don't know whether it currently is.)
>
> !----- one.f90 ----
> subroutine foo()
> !$acc routine gang
> .... ! something
> end subroutine foo
>
> !---- two.f90 ---
> program example
> INTERFACE
> subroutine foo()
> !$acc routine gang
> ! Nothing here
> end subroutine foo
> END INTERFACE
>
> call foo()
> end program example
>
> Namely, a replication of the declaration of the procedure, including
> the "acc routine", in the 'interface'.
> (If one concats the two files, I would also expect an error with -fopenacc,
> if the "acc routine" doesn't match between "foo" and the "foo" in the
> "interface" block.)
I tested this case and it works. There is, however, a problem with
mismatched routine clauses. See PR72741 that Thomas filed recently.
> Otherwise: Have you checked whether an unmodified gfortran still accepts the
> .mod file written by the patched gfortran - and vice versa? Especially if
> -fopenacc is not used, backward compatibility of .mod files is a goal.
> (Even though we often have to bump the .mod version for major releases.)
I just tested this situation, and neither backward or forward compatible
isn't preserved. Basically, this patch introduces a mandatory
OACC_FUNCTION_ field inside the module file. Perhaps I should make that
field optional. At least that way we'd maintain backwards compatibility.
Is there something I can do to maintain forward compatibility?
Cesar
^ permalink raw reply [flat|nested] 15+ messages in thread
* [gomp4] Fix PR72741
@ 2016-07-29 4:21 ` Cesar Philippidis
2016-07-01 20:41 ` [PATCH] OpenACC routines in fortran modules Cesar Philippidis
2019-02-28 20:35 ` [PR72741] For all Fortran OpenACC 'routine' directive variants check for multiple clauses specifying the level of parallelism Thomas Schwinge
0 siblings, 2 replies; 15+ messages in thread
From: Cesar Philippidis @ 2016-07-29 4:21 UTC (permalink / raw)
To: gcc-patches, Fortran List
[-- Attachment #1: Type: text/plain, Size: 708 bytes --]
Thomas found a bug in the fortran routine parser where errors involving
invalid combinations of gang, worker, vector and seq clauses were
getting suppressed. This patch does two things:
1) It moves the error handling into gfc_match_oacc_routine. So now
gfc_oacc_routine_dims returns OACC_FUNCTION_NONE when it detects
an error. That's fine because it's ok for routines to default to
OACC_FUNCTION_SEQ.
2) It makes gfc_match_oacc_routine return a MATCH_ERROR when an error
has been detected in gfc_oacc_routine_dims.
This bug is also present in trunk, but I'd like to see my other fortran
module patch go in first. In the meantime, I'll commit this patch to
gomp-4_0-branch.
Cesar
[-- Attachment #2: gomp4-pr72741.diff --]
[-- Type: text/x-patch, Size: 3023 bytes --]
2016-07-28 Cesar Philippidis <cesar@codesourcery.com>
PR fortran/72741
gcc/fortran/
* openmp.c (gfc_oacc_routine_dims): Move gfc_error to
gfc_match_oacc_routine. Return OACC_FUNCTION_NONE on error.
(gfc_match_oacc_routine): Call gfc_oacc_routine_dims for all
routines directives. Propagate error as appropriate.
gcc/testsuite/
* gfortran.dg/goacc/pr72741.f90: New test.
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index c20a0a3..3c39836 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -1877,7 +1877,8 @@ gfc_match_oacc_cache (void)
return MATCH_YES;
}
-/* Determine the loop level for a routine. */
+/* Determine the loop level for a routine. Returns OACC_FUNCTION_NONE if
+ any error is detected. */
static oacc_function
gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
@@ -1908,7 +1909,7 @@ gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
if (mask != (mask & -mask))
- gfc_error ("Multiple loop axes specified for routine");
+ ret = OACC_FUNCTION_NONE;
}
return ret;
@@ -1923,6 +1924,7 @@ gfc_match_oacc_routine (void)
gfc_omp_clauses *c = NULL;
gfc_oacc_routine_name *n = NULL;
gfc_intrinsic_sym *isym = NULL;
+ oacc_function dims = OACC_FUNCTION_NONE;
old_loc = gfc_current_locus;
@@ -1991,6 +1993,14 @@ gfc_match_oacc_routine (void)
!= MATCH_YES))
return MATCH_ERROR;
+ dims = gfc_oacc_routine_dims (c);
+ if (dims == OACC_FUNCTION_NONE)
+ {
+ gfc_error ("Multiple loop axes specified for routine %C");
+ gfc_current_locus = old_loc;
+ return MATCH_ERROR;
+ }
+
if (isym != NULL)
/* There is nothing to do for intrinsic procedures. */
;
@@ -2011,8 +2021,7 @@ gfc_match_oacc_routine (void)
gfc_current_ns->proc_name->name,
&old_loc))
goto cleanup;
- gfc_current_ns->proc_name->attr.oacc_function
- = gfc_oacc_routine_dims (c);
+ gfc_current_ns->proc_name->attr.oacc_function = dims;
gfc_current_ns->proc_name->attr.oacc_function_nohost
= c ? c->nohost : false;
}
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr72741.f90 b/gcc/testsuite/gfortran.dg/goacc/pr72741.f90
new file mode 100644
index 0000000..cf89727
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/pr72741.f90
@@ -0,0 +1,28 @@
+SUBROUTINE v_1
+ !$ACC ROUTINE VECTOR WORKER ! { dg-error "Multiple loop axes" }
+END SUBROUTINE v_1
+
+SUBROUTINE sub_1
+ IMPLICIT NONE
+ EXTERNAL :: g_1
+ !$ACC ROUTINE (g_1) GANG WORKER ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Multiple loop axes" }
+
+ CALL v_1
+ CALL g_1
+ CALL ABORT
+END SUBROUTINE sub_1
+
+MODULE m_w_1
+ IMPLICIT NONE
+ EXTERNAL :: w_1
+ !$ACC ROUTINE (w_1) WORKER SEQ ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Multiple loop axes" }
+
+CONTAINS
+ SUBROUTINE sub_2
+ CALL v_1
+ CALL w_1
+ CALL ABORT
+ END SUBROUTINE sub_2
+END MODULE m_w_1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules)
2016-07-01 20:41 ` [PATCH] OpenACC routines in fortran modules Cesar Philippidis
2016-07-28 9:55 ` Tobias Burnus
@ 2016-08-11 15:19 ` Thomas Schwinge
2016-08-11 15:40 ` Jakub Jelinek
2016-08-11 16:44 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
1 sibling, 2 replies; 15+ messages in thread
From: Thomas Schwinge @ 2016-08-11 15:19 UTC (permalink / raw)
To: Cesar Philippidis, gcc-patches, Fortran List; +Cc: Tobias Burnus, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 43473 bytes --]
Hi!
This is still hacky and WIP; posting for Cesar and Tobias to have a look.
I'm still not too much of a Fortran person. ;-)
On Fri, 1 Jul 2016 13:40:58 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> It turns out that the acc routine parallelism isn't being recorded in
> fortran .mod files. This is a problem because then the ME can't validate
> if a routine has compatible parallelism with the call site. This patch
> does two things:
>
> 1. Encode gang, worker, vector and seq level parallelism in module
> files. This introduces a new oacc_function enum, which I ended
> up using to record the parallelism of standalone acc routines too.
Building on top of this patch, and on top of
<https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01910.html> "[gomp4] Fix
PR72741", I reworked these patches (effectively reverting a lot of
Cesar's earlier changes, which nevertheless gave good guidance to me,
about which code I needed to touch). With this patch, we now handle more
Fortran OpenACC routine directive use/misuse (see the test case changes),
much in spirit of what I discussed in <http://gcc.gnu.org/PR72741>
"Fortran OpenACC routine directive doesn't properly handle clauses
specifying the level of parallelism", minus items that Cesar already
clarified for me, where Fortran is different from what I expected,
different from the C/C++ environment I'm more used to. This now also
paves the way for adding Fortran support to my recent patch
<https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00069.html> "Use
verify_oacc_routine_clauses", and then ultimately
<https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00071.html> "Repeated use
of the OpenACC routine directive".
However, my changes are still hacky and WIP, still contains a bunch of
TODOs. Can you, Cesar and/or Tobias, please advise on these?
> 2. Extends gfc_match_oacc_routine to add acc routine directive support
> for intrinsic procedures such as abort.
>
> Is this patch OK for trunk? I included support for intrinsic procedures
> because it was necessary with my previous patch which treated all calls
> to non-acc routines from within an OpenACC offloaded region as errors.
> Now that it has been determined that those patches should be link time
> errors, we technically don't need to add acc routine support for
> intrinsic procedures. So I can drop that part of the patch if necessary.
That could've been a patch separate from the others, as it's doing a
separate thing. We will want to handle intrinsics used with the OpenACC
routine directive with a name (but it certainly isn't a priority). I
left in these changes, and also extended them a bit.
First some comments on Cesar's patch:
> --- a/gcc/fortran/module.c
> +++ b/gcc/fortran/module.c
> [...]
> +DECL_MIO_NAME (oacc_function)
> [...]
As discussed between Cesar and Tobias, these module.c/symbol.c changes
introduce an incompatibility in the Fortran module file format, which
we'll want to avoid. Reverting that to use individual bit flags instead
of the "enum oacc_function", I think that we're safe (but I have not
verified that). On the other hand, given that I'm not at all handling in
module.c/symbol.c the new "locus omp_clauses_locus" and "struct
symbol_attribute *next" members that I'm adding to "symbol_attribute",
I'm not sure whether I'm actually testing this properly. ;-) I guess I'm
not.
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -1814,7 +1824,10 @@ gfc_match_oacc_routine (void)
> != MATCH_YES))
> return MATCH_ERROR;
>
> - if (sym != NULL)
> + if (isym != NULL)
> + /* There is nothing to do for intrinsic procedures. */
> + ;
We will want to check that no incompatible clauses are being specified,
for example (but, low priority). I'm adding a hacky implementation of
that.
> + else if (sym != NULL)
> {
> n = gfc_get_oacc_routine_name ();
> n->sym = sym;
> --- a/gcc/fortran/trans-decl.c
> +++ b/gcc/fortran/trans-decl.c
> @@ -1327,11 +1327,26 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list)
> list = tree_cons (get_identifier ("omp declare target"),
> NULL_TREE, list);
>
> - if (sym_attr.oacc_function)
> + if (sym_attr.oacc_function != OACC_FUNCTION_NONE)
> {
> tree dims = NULL_TREE;
> int ix;
> - int level = sym_attr.oacc_function - 1;
> + int level = GOMP_DIM_MAX;
> +
> + switch (sym_attr.oacc_function)
> + {
> + case OACC_FUNCTION_GANG:
> + level = GOMP_DIM_GANG;
> + break;
> + case OACC_FUNCTION_WORKER:
> + level = GOMP_DIM_WORKER;
> + break;
> + case OACC_FUNCTION_VECTOR:
> + level = GOMP_DIM_VECTOR;
> + break;
> + case OACC_FUNCTION_SEQ:
> + default:;
> + }
>
> for (ix = GOMP_DIM_MAX; ix--;)
> dims = tree_cons (build_int_cst (boolean_type_node, ix >= level),
As discussed before, this should use the generic omp-low.c functions,
which I've implemented.
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/goacc/routine-7.f90
> @@ -0,0 +1,69 @@
> +! Test acc routines inside modules.
> +
> +! { dg-additional-options "-O0" }
-O0 to prevent inlining of functions tagged with OpenACC routine
directives, or another reason?
> --- a/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
> +++ b/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
> @@ -1,121 +1,95 @@
> +! Test acc routines inside modules.
>
> ! { dg-do run }
> -! { dg-additional-options "-cpp" }
>
> -#define M 8
> -#define N 32
> +module routines
> + integer, parameter :: N = 32
>
> -program main
> - integer :: i
> - integer :: a(N)
> - integer :: b(M * N)
> -
> - do i = 1, N
> - a(i) = 0
> - end do
> +contains
> + subroutine vector (a)
> + implicit none
> + !$acc routine vector
> + integer, intent (inout) :: a(N)
> + integer :: i
> [...]
This seems to completely rewrite the test case. Is that intentional, or
should the original test case be preserved, and the changed/new/rewritten
one be added as a new test case?
Now, my hacky WIP patch.
One big chunk of the gcc/fortran/gfortran.h changes is just to move some
stuff around, without any changes, so that I can use "locus" in
"symbol_attribute".
I very much "cargo cult"ed all that "oacc_routine*" bit flag stuff in
module.c/symbol.c, replicating what's being done for "omp target
declare", without really knowing what I'm doing there. I will appreciate
test cases actually exercising this code -- which doesn't currently at
all handle the new "locus omp_clauses_locus" and "struct symbol_attribute
*next" members that I'm adding to "symbol_attribute", as I've mentioned
before. (But I suppose it should?)
We're not implementing the OpenACC device_type clause at the moment, so
the "TODO: handle device_type clauses" comment in
gcc/fortran/openmp.c:gfc_match_oacc_routine is not a concern right now.
With these changes, we're now actually also paying attention the clauses
specified with the OpenACC routine directive with a name -- one of the
things mentioned as missing in <http://gcc.gnu.org/PR72741> "Fortran
OpenACC routine directive doesn't properly handle clauses specifying the
level of parallelism".
To handle several "pending" OpenACC routine directives, I had to add the
"struct symbol_attribute *next" member to "symbol_attribute" -- I hope
that doesn't disqualify the proposed changes as too ugly. (Several other
structs already contain such "next" pointers, and the use is very much
confined to only the OpenACC routine directive.) I will of course be
happy to learn about a better/different way to do this.
commit ca4a098dab72f27c6e1121aa7e5e49764921974e
Author: Thomas Schwinge <thomas@codesourcery.com>
Date: Thu Aug 11 16:34:22 2016 +0200
[WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling
---
gcc/fortran/gfortran.h | 275 +++++++++++----------
gcc/fortran/module.c | 34 ++-
gcc/fortran/openmp.c | 106 ++++----
gcc/fortran/symbol.c | 135 +++++++++-
gcc/fortran/trans-decl.c | 106 ++++++--
.../gfortran.dg/goacc/oaccdevlow-routine.f95 | 2 +-
gcc/testsuite/gfortran.dg/goacc/pr72741-2.f | 39 +++
.../gfortran.dg/goacc/pr72741-intrinsic-1.f | 16 ++
gcc/testsuite/gfortran.dg/goacc/pr72741.f90 | 14 +-
9 files changed, 501 insertions(+), 226 deletions(-)
diff --git gcc/fortran/gfortran.h gcc/fortran/gfortran.h
index c70f51f..5f19421 100644
--- gcc/fortran/gfortran.h
+++ gcc/fortran/gfortran.h
@@ -303,15 +303,6 @@ enum save_state
{ SAVE_NONE = 0, SAVE_EXPLICIT, SAVE_IMPLICIT
};
-/* Flags to keep track of ACC routine states. */
-enum oacc_function
-{ OACC_FUNCTION_NONE = 0,
- OACC_FUNCTION_SEQ,
- OACC_FUNCTION_GANG,
- OACC_FUNCTION_WORKER,
- OACC_FUNCTION_VECTOR
-};
-
/* Strings for all symbol attributes. We use these for dumping the
parse tree, in error messages, and also when reading and writing
modules. In symbol.c. */
@@ -321,7 +312,6 @@ extern const mstring intents[];
extern const mstring access_types[];
extern const mstring ifsrc_types[];
extern const mstring save_status[];
-extern const mstring oacc_function_types[];
/* Enumeration of all the generic intrinsic functions. Used by the
backend for identification of a function. */
@@ -705,6 +695,126 @@ CInteropKind_t;
extern CInteropKind_t c_interop_kinds_table[];
+/* We need to store source lines as sequences of multibyte source
+ characters. We define here a type wide enough to hold any multibyte
+ source character, just like libcpp does. A 32-bit type is enough. */
+
+#if HOST_BITS_PER_INT >= 32
+typedef unsigned int gfc_char_t;
+#elif HOST_BITS_PER_LONG >= 32
+typedef unsigned long gfc_char_t;
+#elif defined(HAVE_LONG_LONG) && (HOST_BITS_PER_LONGLONG >= 32)
+typedef unsigned long long gfc_char_t;
+#else
+# error "Cannot find an integer type with at least 32 bits"
+#endif
+
+
+/* The following three structures are used to identify a location in
+ the sources.
+
+ gfc_file is used to maintain a tree of the source files and how
+ they include each other
+
+ gfc_linebuf holds a single line of source code and information
+ which file it resides in
+
+ locus point to the sourceline and the character in the source
+ line.
+*/
+
+typedef struct gfc_file
+{
+ struct gfc_file *next, *up;
+ int inclusion_line, line;
+ char *filename;
+} gfc_file;
+
+typedef struct gfc_linebuf
+{
+ source_location location;
+ struct gfc_file *file;
+ struct gfc_linebuf *next;
+
+ int truncated;
+ bool dbg_emitted;
+
+ gfc_char_t line[1];
+} gfc_linebuf;
+
+#define gfc_linebuf_header_size (offsetof (gfc_linebuf, line))
+
+#define gfc_linebuf_linenum(LBUF) (LOCATION_LINE ((LBUF)->location))
+
+typedef struct
+{
+ gfc_char_t *nextc;
+ gfc_linebuf *lb;
+} locus;
+
+/* In order for the "gfc" format checking to work correctly, you must
+ have declared a typedef locus first. */
+#if GCC_VERSION >= 4001
+#define ATTRIBUTE_GCC_GFC(m, n) __attribute__ ((__format__ (__gcc_gfc__, m, n))) ATTRIBUTE_NONNULL(m)
+#else
+#define ATTRIBUTE_GCC_GFC(m, n) ATTRIBUTE_NONNULL(m)
+#endif
+
+
+/* Suppress error messages or re-enable them. */
+
+void gfc_push_suppress_errors (void);
+void gfc_pop_suppress_errors (void);
+
+
+/* Character length structures hold the expression that gives the
+ length of a character variable. We avoid putting these into
+ gfc_typespec because doing so prevents us from doing structure
+ copies and forces us to deallocate any typespecs we create, as well
+ as structures that contain typespecs. They also can have multiple
+ character typespecs pointing to them.
+
+ These structures form a singly linked list within the current
+ namespace and are deallocated with the namespace. It is possible to
+ end up with gfc_charlen structures that have nothing pointing to them. */
+
+typedef struct gfc_charlen
+{
+ struct gfc_expr *length;
+ struct gfc_charlen *next;
+ bool length_from_typespec; /* Length from explicit array ctor typespec? */
+ tree backend_decl;
+ tree passed_length; /* Length argument explicitly passed. */
+
+ int resolved;
+}
+gfc_charlen;
+
+#define gfc_get_charlen() XCNEW (gfc_charlen)
+
+/* Type specification structure. */
+typedef struct
+{
+ bt type;
+ int kind;
+
+ union
+ {
+ struct gfc_symbol *derived; /* For derived types only. */
+ gfc_charlen *cl; /* For character types only. */
+ int pad; /* For hollerith types only. */
+ }
+ u;
+
+ struct gfc_symbol *interface; /* For PROCEDURE declarations. */
+ int is_c_interop;
+ int is_iso_c;
+ bt f90_type;
+ bool deferred;
+}
+gfc_typespec;
+
+
/* Structure and list of supported extension attributes. */
typedef enum
{
@@ -729,7 +839,7 @@ ext_attr_t;
extern const ext_attr_t ext_attr_list[];
/* Symbol attribute structure. */
-typedef struct
+typedef struct symbol_attribute
{
/* Variable attributes. */
unsigned allocatable:1, dimension:1, codimension:1, external:1, intrinsic:1,
@@ -864,6 +974,13 @@ typedef struct
/* Mentioned in OMP DECLARE TARGET. */
unsigned omp_declare_target:1;
+ /* OpenACC routine. */
+ unsigned oacc_routine:1;
+ unsigned oacc_routine_gang:1;
+ unsigned oacc_routine_worker:1;
+ unsigned oacc_routine_vector:1;
+ unsigned oacc_routine_seq:1;
+
/* Mentioned in OACC DECLARE. */
unsigned oacc_declare_create:1;
unsigned oacc_declare_copyin:1;
@@ -871,137 +988,24 @@ typedef struct
unsigned oacc_declare_device_resident:1;
unsigned oacc_declare_link:1;
- /* This is an OpenACC acclerator function at level N - 1 */
- ENUM_BITFIELD (oacc_function) oacc_function:3;
-
/* Attributes set by compiler extensions (!GCC$ ATTRIBUTES). */
unsigned ext_attr:EXT_ATTR_NUM;
+ /* Location information for OMP clauses. */
+ //TODO: how to handle in module.c/symbol.c?
+ locus omp_clauses_locus;
+
/* The namespace where the attribute has been set. */
struct gfc_namespace *volatile_ns, *asynchronous_ns;
+
+ /* Chain to another set of symbol attributes. Currently only used for
+ OpenACC routine. */
+ //TODO: how to handle in module.c/symbol.c?
+ struct symbol_attribute *next;
}
symbol_attribute;
-/* We need to store source lines as sequences of multibyte source
- characters. We define here a type wide enough to hold any multibyte
- source character, just like libcpp does. A 32-bit type is enough. */
-
-#if HOST_BITS_PER_INT >= 32
-typedef unsigned int gfc_char_t;
-#elif HOST_BITS_PER_LONG >= 32
-typedef unsigned long gfc_char_t;
-#elif defined(HAVE_LONG_LONG) && (HOST_BITS_PER_LONGLONG >= 32)
-typedef unsigned long long gfc_char_t;
-#else
-# error "Cannot find an integer type with at least 32 bits"
-#endif
-
-
-/* The following three structures are used to identify a location in
- the sources.
-
- gfc_file is used to maintain a tree of the source files and how
- they include each other
-
- gfc_linebuf holds a single line of source code and information
- which file it resides in
-
- locus point to the sourceline and the character in the source
- line.
-*/
-
-typedef struct gfc_file
-{
- struct gfc_file *next, *up;
- int inclusion_line, line;
- char *filename;
-} gfc_file;
-
-typedef struct gfc_linebuf
-{
- source_location location;
- struct gfc_file *file;
- struct gfc_linebuf *next;
-
- int truncated;
- bool dbg_emitted;
-
- gfc_char_t line[1];
-} gfc_linebuf;
-
-#define gfc_linebuf_header_size (offsetof (gfc_linebuf, line))
-
-#define gfc_linebuf_linenum(LBUF) (LOCATION_LINE ((LBUF)->location))
-
-typedef struct
-{
- gfc_char_t *nextc;
- gfc_linebuf *lb;
-} locus;
-
-/* In order for the "gfc" format checking to work correctly, you must
- have declared a typedef locus first. */
-#if GCC_VERSION >= 4001
-#define ATTRIBUTE_GCC_GFC(m, n) __attribute__ ((__format__ (__gcc_gfc__, m, n))) ATTRIBUTE_NONNULL(m)
-#else
-#define ATTRIBUTE_GCC_GFC(m, n) ATTRIBUTE_NONNULL(m)
-#endif
-
-
-/* Suppress error messages or re-enable them. */
-
-void gfc_push_suppress_errors (void);
-void gfc_pop_suppress_errors (void);
-
-
-/* Character length structures hold the expression that gives the
- length of a character variable. We avoid putting these into
- gfc_typespec because doing so prevents us from doing structure
- copies and forces us to deallocate any typespecs we create, as well
- as structures that contain typespecs. They also can have multiple
- character typespecs pointing to them.
-
- These structures form a singly linked list within the current
- namespace and are deallocated with the namespace. It is possible to
- end up with gfc_charlen structures that have nothing pointing to them. */
-
-typedef struct gfc_charlen
-{
- struct gfc_expr *length;
- struct gfc_charlen *next;
- bool length_from_typespec; /* Length from explicit array ctor typespec? */
- tree backend_decl;
- tree passed_length; /* Length argument explicitly passed. */
-
- int resolved;
-}
-gfc_charlen;
-
-#define gfc_get_charlen() XCNEW (gfc_charlen)
-
-/* Type specification structure. */
-typedef struct
-{
- bt type;
- int kind;
-
- union
- {
- struct gfc_symbol *derived; /* For derived types only. */
- gfc_charlen *cl; /* For character types only. */
- int pad; /* For hollerith types only. */
- }
- u;
-
- struct gfc_symbol *interface; /* For PROCEDURE declarations. */
- int is_c_interop;
- int is_iso_c;
- bt f90_type;
- bool deferred;
-}
-gfc_typespec;
-
/* Array specification. */
typedef struct
{
@@ -2816,6 +2820,11 @@ bool gfc_add_result (symbol_attribute *, const char *, locus *);
bool gfc_add_save (symbol_attribute *, save_state, const char *, locus *);
bool gfc_add_threadprivate (symbol_attribute *, const char *, locus *);
bool gfc_add_omp_declare_target (symbol_attribute *, const char *, locus *);
+bool gfc_add_oacc_routine (symbol_attribute *, const char *, locus *);
+bool gfc_add_oacc_routine_gang (symbol_attribute *, const char *, locus *);
+bool gfc_add_oacc_routine_worker (symbol_attribute *, const char *, locus *);
+bool gfc_add_oacc_routine_vector (symbol_attribute *, const char *, locus *);
+bool gfc_add_oacc_routine_seq (symbol_attribute *, const char *, locus *);
bool gfc_add_saved_common (symbol_attribute *, locus *);
bool gfc_add_target (symbol_attribute *, locus *);
bool gfc_add_dummy (symbol_attribute *, const char *, locus *);
diff --git gcc/fortran/module.c gcc/fortran/module.c
index 267858f..4b590c6 100644
--- gcc/fortran/module.c
+++ gcc/fortran/module.c
@@ -1986,6 +1986,7 @@ enum ab_attribute
AB_IS_CLASS, AB_PROCEDURE, AB_PROC_POINTER, AB_ASYNCHRONOUS, AB_CODIMENSION,
AB_COARRAY_COMP, AB_VTYPE, AB_VTAB, AB_CONTIGUOUS, AB_CLASS_POINTER,
AB_IMPLICIT_PURE, AB_ARTIFICIAL, AB_UNLIMITED_POLY, AB_OMP_DECLARE_TARGET,
+ AB_OACC_ROUTINE, AB_OACC_ROUTINE_GANG, AB_OACC_ROUTINE_WORKER, AB_OACC_ROUTINE_VECTOR, AB_OACC_ROUTINE_SEQ,
AB_ARRAY_OUTER_DEPENDENCY, AB_MODULE_PROCEDURE, AB_OACC_DECLARE_CREATE,
AB_OACC_DECLARE_COPYIN, AB_OACC_DECLARE_DEVICEPTR,
AB_OACC_DECLARE_DEVICE_RESIDENT, AB_OACC_DECLARE_LINK
@@ -2044,6 +2045,11 @@ static const mstring attr_bits[] =
minit ("IMPLICIT_PURE", AB_IMPLICIT_PURE),
minit ("UNLIMITED_POLY", AB_UNLIMITED_POLY),
minit ("OMP_DECLARE_TARGET", AB_OMP_DECLARE_TARGET),
+ minit ("OACC_ROUTINE", AB_OACC_ROUTINE),
+ minit ("OACC_ROUTINE_GANG", AB_OACC_ROUTINE_GANG),
+ minit ("OACC_ROUTINE_WORKER", AB_OACC_ROUTINE_WORKER),
+ minit ("OACC_ROUTINE_VECTOR", AB_OACC_ROUTINE_VECTOR),
+ minit ("OACC_ROUTINE_SEQ", AB_OACC_ROUTINE_SEQ),
minit ("ARRAY_OUTER_DEPENDENCY", AB_ARRAY_OUTER_DEPENDENCY),
minit ("MODULE_PROCEDURE", AB_MODULE_PROCEDURE),
minit ("OACC_DECLARE_CREATE", AB_OACC_DECLARE_CREATE),
@@ -2095,7 +2101,6 @@ DECL_MIO_NAME (procedure_type)
DECL_MIO_NAME (ref_type)
DECL_MIO_NAME (sym_flavor)
DECL_MIO_NAME (sym_intent)
-DECL_MIO_NAME (oacc_function)
#undef DECL_MIO_NAME
/* Symbol attributes are stored in list with the first three elements
@@ -2117,8 +2122,6 @@ mio_symbol_attribute (symbol_attribute *attr)
attr->proc = MIO_NAME (procedure_type) (attr->proc, procedures);
attr->if_source = MIO_NAME (ifsrc) (attr->if_source, ifsrc_types);
attr->save = MIO_NAME (save_state) (attr->save, save_status);
- attr->oacc_function = MIO_NAME (oacc_function) (attr->oacc_function,
- oacc_function_types);
ext_attr = attr->ext_attr;
mio_integer ((int *) &ext_attr);
@@ -2236,6 +2239,16 @@ mio_symbol_attribute (symbol_attribute *attr)
MIO_NAME (ab_attribute) (AB_VTAB, attr_bits);
if (attr->omp_declare_target)
MIO_NAME (ab_attribute) (AB_OMP_DECLARE_TARGET, attr_bits);
+ if (attr->oacc_routine)
+ MIO_NAME (ab_attribute) (AB_OACC_ROUTINE, attr_bits);
+ if (attr->oacc_routine_gang)
+ MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_GANG, attr_bits);
+ if (attr->oacc_routine_worker)
+ MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_WORKER, attr_bits);
+ if (attr->oacc_routine_vector)
+ MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_VECTOR, attr_bits);
+ if (attr->oacc_routine_seq)
+ MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_SEQ, attr_bits);
if (attr->array_outer_dependency)
MIO_NAME (ab_attribute) (AB_ARRAY_OUTER_DEPENDENCY, attr_bits);
if (attr->module_procedure)
@@ -2422,6 +2435,21 @@ mio_symbol_attribute (symbol_attribute *attr)
case AB_OMP_DECLARE_TARGET:
attr->omp_declare_target = 1;
break;
+ case AB_OACC_ROUTINE:
+ attr->oacc_routine = 1;
+ break;
+ case AB_OACC_ROUTINE_GANG:
+ attr->oacc_routine_gang = 1;
+ break;
+ case AB_OACC_ROUTINE_WORKER:
+ attr->oacc_routine_worker = 1;
+ break;
+ case AB_OACC_ROUTINE_VECTOR:
+ attr->oacc_routine_vector = 1;
+ break;
+ case AB_OACC_ROUTINE_SEQ:
+ attr->oacc_routine_seq = 1;
+ break;
case AB_ARRAY_OUTER_DEPENDENCY:
attr->array_outer_dependency =1;
break;
diff --git gcc/fortran/openmp.c gcc/fortran/openmp.c
index 05e4661..5a69e38 100644
--- gcc/fortran/openmp.c
+++ gcc/fortran/openmp.c
@@ -1714,44 +1714,6 @@ gfc_match_oacc_cache (void)
return MATCH_YES;
}
-/* Determine the loop level for a routine. Returns OACC_FUNCTION_NONE if
- any error is detected. */
-
-static oacc_function
-gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
-{
- int level = -1;
- oacc_function ret = OACC_FUNCTION_SEQ;
-
- if (clauses)
- {
- unsigned mask = 0;
-
- if (clauses->gang)
- {
- level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
- ret = OACC_FUNCTION_GANG;
- }
- if (clauses->worker)
- {
- level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
- ret = OACC_FUNCTION_WORKER;
- }
- if (clauses->vector)
- {
- level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
- ret = OACC_FUNCTION_VECTOR;
- }
- if (clauses->seq)
- level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
-
- if (mask != (mask & -mask))
- ret = OACC_FUNCTION_NONE;
- }
-
- return ret;
-}
-
match
gfc_match_oacc_routine (void)
{
@@ -1761,7 +1723,8 @@ gfc_match_oacc_routine (void)
gfc_omp_clauses *c = NULL;
gfc_oacc_routine_name *n = NULL;
gfc_intrinsic_sym *isym = NULL;
- oacc_function dims = OACC_FUNCTION_NONE;
+ symbol_attribute *add_attr = NULL;
+ const char *add_attr_name = NULL;
old_loc = gfc_current_locus;
@@ -1828,19 +1791,26 @@ gfc_match_oacc_routine (void)
!= MATCH_YES))
return MATCH_ERROR;
- dims = gfc_oacc_routine_dims (c);
- if (dims == OACC_FUNCTION_NONE)
- {
- gfc_error ("Multiple loop axes specified for routine %C");
- gfc_current_locus = old_loc;
- return MATCH_ERROR;
- }
-
if (isym != NULL)
- /* There is nothing to do for intrinsic procedures. */
- ;
+ {
+ //TODO gfc_intrinsic_sym doesn't have symbol_attribute?
+ //add_attr = &isym->attr;
+ //add_attr_name = NULL; //TODO
+ /* Fake it. TODO: handle device_type clauses... */
+ if (c->gang || c->worker || c->vector)
+ {
+ gfc_error ("Intrinsic symbol specified in !$ACC ROUTINE ( NAME )"
+ " at %C, with incompatible clauses specifying the level"
+ " of parallelism");
+ gfc_current_locus = old_loc;
+ return MATCH_ERROR;
+ }
+ }
else if (sym != NULL)
{
+ add_attr = &sym->attr;
+ add_attr_name = NULL; //TODO
+
n = gfc_get_oacc_routine_name ();
n->sym = sym;
n->clauses = NULL;
@@ -1852,11 +1822,41 @@ gfc_match_oacc_routine (void)
}
else if (gfc_current_ns->proc_name)
{
- if (!gfc_add_omp_declare_target (&gfc_current_ns->proc_name->attr,
- gfc_current_ns->proc_name->name,
- &old_loc))
+ add_attr = &gfc_current_ns->proc_name->attr;
+ add_attr_name = gfc_current_ns->proc_name->name;
+ }
+ else
+ gcc_unreachable ();
+
+ if (add_attr != NULL)
+ {
+ if (!gfc_add_omp_declare_target (add_attr, add_attr_name, &old_loc))
goto cleanup;
- gfc_current_ns->proc_name->attr.oacc_function = dims;
+ /* Skip over any existing symbol attributes capturing OpenACC routine
+ directives. */
+ while (add_attr->next != NULL)
+ add_attr = add_attr->next;
+ if (add_attr->oacc_routine)
+ {
+ add_attr->next = XCNEW (symbol_attribute);
+ gfc_clear_attr (add_attr->next);
+ add_attr = add_attr->next;
+ }
+ if (!gfc_add_oacc_routine (add_attr, add_attr_name, &old_loc))
+ goto cleanup;
+ if (c && c->gang
+ && !gfc_add_oacc_routine_gang (add_attr, add_attr_name, &old_loc))
+ goto cleanup;
+ if (c && c->worker
+ && !gfc_add_oacc_routine_worker (add_attr, add_attr_name, &old_loc))
+ goto cleanup;
+ if (c && c->vector
+ && !gfc_add_oacc_routine_vector (add_attr, add_attr_name, &old_loc))
+ goto cleanup;
+ if (c && c->seq
+ && !gfc_add_oacc_routine_seq (add_attr, add_attr_name, &old_loc))
+ goto cleanup;
+ add_attr->omp_clauses_locus = old_loc; //TODO OK to just assign that?
}
if (n)
diff --git gcc/fortran/symbol.c gcc/fortran/symbol.c
index 84fa2bd..36852da 100644
--- gcc/fortran/symbol.c
+++ gcc/fortran/symbol.c
@@ -87,15 +87,6 @@ const mstring save_status[] =
minit ("IMPLICIT-SAVE", SAVE_IMPLICIT),
};
-const mstring oacc_function_types[] =
-{
- minit ("NONE", OACC_FUNCTION_NONE),
- minit ("OACC_FUNCTION_SEQ", OACC_FUNCTION_SEQ),
- minit ("OACC_FUNCTION_GANG", OACC_FUNCTION_GANG),
- minit ("OACC_FUNCTION_WORKER", OACC_FUNCTION_WORKER),
- minit ("OACC_FUNCTION_VECTOR", OACC_FUNCTION_VECTOR)
-};
-
/* This is to make sure the backend generates setup code in the correct
order. */
@@ -385,6 +376,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
*contiguous = "CONTIGUOUS", *generic = "GENERIC";
static const char *threadprivate = "THREADPRIVATE";
static const char *omp_declare_target = "OMP DECLARE TARGET";
+ static const char *oacc_routine = "OACC ROUTINE";
+ static const char *oacc_routine_gang = "OACC ROUTINE GANG";
+ static const char *oacc_routine_worker = "OACC ROUTINE WORKER";
+ static const char *oacc_routine_vector = "OACC ROUTINE VECTOR";
+ static const char *oacc_routine_seq = "OACC ROUTINE SEQ";
static const char *oacc_declare_copyin = "OACC DECLARE COPYIN";
static const char *oacc_declare_create = "OACC DECLARE CREATE";
static const char *oacc_declare_deviceptr = "OACC DECLARE DEVICEPTR";
@@ -482,6 +478,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
conf (dummy, intrinsic);
conf (dummy, threadprivate);
conf (dummy, omp_declare_target);
+ conf (dummy, oacc_routine);
+ conf (dummy, oacc_routine_gang);
+ conf (dummy, oacc_routine_worker);
+ conf (dummy, oacc_routine_vector);
+ conf (dummy, oacc_routine_seq);
conf (pointer, target);
conf (pointer, intrinsic);
conf (pointer, elemental);
@@ -526,6 +527,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
conf (in_equivalence, allocatable);
conf (in_equivalence, threadprivate);
conf (in_equivalence, omp_declare_target);
+ conf (in_equivalence, oacc_routine);
+ conf (in_equivalence, oacc_routine_gang);
+ conf (in_equivalence, oacc_routine_worker);
+ conf (in_equivalence, oacc_routine_vector);
+ conf (in_equivalence, oacc_routine_seq);
conf (in_equivalence, oacc_declare_create);
conf (in_equivalence, oacc_declare_copyin);
conf (in_equivalence, oacc_declare_deviceptr);
@@ -579,6 +585,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
conf (cray_pointee, in_equivalence);
conf (cray_pointee, threadprivate);
conf (cray_pointee, omp_declare_target);
+ conf (cray_pointee, oacc_routine);
+ conf (cray_pointee, oacc_routine_gang);
+ conf (cray_pointee, oacc_routine_worker);
+ conf (cray_pointee, oacc_routine_vector);
+ conf (cray_pointee, oacc_routine_seq);
conf (cray_pointee, oacc_declare_create);
conf (cray_pointee, oacc_declare_copyin);
conf (cray_pointee, oacc_declare_deviceptr);
@@ -637,6 +648,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
conf (proc_pointer, abstract)
conf (entry, omp_declare_target)
+ conf (entry, oacc_routine)
+ conf (entry, oacc_routine_gang)
+ conf (entry, oacc_routine_worker)
+ conf (entry, oacc_routine_vector)
+ conf (entry, oacc_routine_seq)
conf (entry, oacc_declare_create)
conf (entry, oacc_declare_copyin)
conf (entry, oacc_declare_deviceptr)
@@ -678,6 +694,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
conf2 (subroutine);
conf2 (threadprivate);
conf2 (omp_declare_target);
+ conf2 (oacc_routine);
+ conf2 (oacc_routine_gang);
+ conf2 (oacc_routine_worker);
+ conf2 (oacc_routine_vector);
+ conf2 (oacc_routine_seq);
conf2 (oacc_declare_create);
conf2 (oacc_declare_copyin);
conf2 (oacc_declare_deviceptr);
@@ -764,6 +785,11 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
conf2 (threadprivate);
conf2 (result);
conf2 (omp_declare_target);
+ conf2 (oacc_routine);
+ conf2 (oacc_routine_gang);
+ conf2 (oacc_routine_worker);
+ conf2 (oacc_routine_vector);
+ conf2 (oacc_routine_seq);
conf2 (oacc_declare_create);
conf2 (oacc_declare_copyin);
conf2 (oacc_declare_deviceptr);
@@ -1266,7 +1292,6 @@ bool
gfc_add_omp_declare_target (symbol_attribute *attr, const char *name,
locus *where)
{
-
if (check_used (attr, name, where))
return false;
@@ -1279,6 +1304,81 @@ gfc_add_omp_declare_target (symbol_attribute *attr, const char *name,
bool
+gfc_add_oacc_routine (symbol_attribute *attr, const char *name,
+ locus *where)
+{
+ if (check_used (attr, name, where))
+ return false;
+
+ if (attr->oacc_routine)
+ return true;
+
+ attr->oacc_routine = 1;
+ return check_conflict (attr, name, where);
+}
+
+
+bool
+gfc_add_oacc_routine_gang (symbol_attribute *attr, const char *name,
+ locus *where)
+{
+ if (check_used (attr, name, where))
+ return false;
+
+ if (attr->oacc_routine_gang)
+ return true;
+
+ attr->oacc_routine_gang = 1;
+ return check_conflict (attr, name, where);
+}
+
+
+bool
+gfc_add_oacc_routine_worker (symbol_attribute *attr, const char *name,
+ locus *where)
+{
+ if (check_used (attr, name, where))
+ return false;
+
+ if (attr->oacc_routine_worker)
+ return true;
+
+ attr->oacc_routine_worker = 1;
+ return check_conflict (attr, name, where);
+}
+
+
+bool
+gfc_add_oacc_routine_vector (symbol_attribute *attr, const char *name,
+ locus *where)
+{
+ if (check_used (attr, name, where))
+ return false;
+
+ if (attr->oacc_routine_vector)
+ return true;
+
+ attr->oacc_routine_vector = 1;
+ return check_conflict (attr, name, where);
+}
+
+
+bool
+gfc_add_oacc_routine_seq (symbol_attribute *attr, const char *name,
+ locus *where)
+{
+ if (check_used (attr, name, where))
+ return false;
+
+ if (attr->oacc_routine_seq)
+ return true;
+
+ attr->oacc_routine_seq = 1;
+ return check_conflict (attr, name, where);
+}
+
+
+bool
gfc_add_oacc_declare_create (symbol_attribute *attr, const char *name,
locus *where)
{
@@ -1915,6 +2015,21 @@ gfc_copy_attr (symbol_attribute *dest, symbol_attribute *src, locus *where)
if (src->omp_declare_target
&& !gfc_add_omp_declare_target (dest, NULL, where))
goto fail;
+ if (src->oacc_routine
+ && !gfc_add_oacc_routine (dest, NULL, where))
+ goto fail;
+ if (src->oacc_routine_gang
+ && !gfc_add_oacc_routine_gang (dest, NULL, where))
+ goto fail;
+ if (src->oacc_routine_worker
+ && !gfc_add_oacc_routine_worker (dest, NULL, where))
+ goto fail;
+ if (src->oacc_routine_vector
+ && !gfc_add_oacc_routine_vector (dest, NULL, where))
+ goto fail;
+ if (src->oacc_routine_seq
+ && !gfc_add_oacc_routine_seq (dest, NULL, where))
+ goto fail;
if (src->oacc_declare_create
&& !gfc_add_oacc_declare_create (dest, NULL, where))
goto fail;
diff --git gcc/fortran/trans-decl.c gcc/fortran/trans-decl.c
index 1934453..d1b956c 100644
--- gcc/fortran/trans-decl.c
+++ gcc/fortran/trans-decl.c
@@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see
#include "trans-stmt.h"
#include "gomp-constants.h"
#include "gimplify.h"
+#include "omp-low.h"
#define MAX_LABEL_VALUE 99999
@@ -1360,37 +1361,94 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list)
}
if (sym_attr.omp_declare_target)
- list = tree_cons (get_identifier ("omp declare target"),
- NULL_TREE, list);
-
- if (sym_attr.oacc_function != OACC_FUNCTION_NONE)
{
- tree dims = NULL_TREE;
- int ix;
- int level = GOMP_DIM_MAX;
+ tree clauses = NULL_TREE;
+ symbol_attribute *oacc_routine_attr = &sym_attr;
+ while (oacc_routine_attr != NULL
+ && oacc_routine_attr->oacc_routine)
+ {
+ location_t loc = oacc_routine_attr->omp_clauses_locus.lb->location;
+ //TODO use gfc_trans_omp_clauses?
+ tree clauses_ = NULL_TREE;
+ if (oacc_routine_attr->oacc_routine_gang)
+ {
+ tree c = build_omp_clause (loc, OMP_CLAUSE_GANG);
+ OMP_CLAUSE_CHAIN (c) = clauses_;
+ clauses_ = c;
+ }
+ if (oacc_routine_attr->oacc_routine_worker)
+ {
+ tree c = build_omp_clause (loc, OMP_CLAUSE_WORKER);
+ OMP_CLAUSE_CHAIN (c) = clauses_;
+ clauses_ = c;
+ }
+ if (oacc_routine_attr->oacc_routine_vector)
+ {
+ tree c = build_omp_clause (loc, OMP_CLAUSE_VECTOR);
+ OMP_CLAUSE_CHAIN (c) = clauses_;
+ clauses_ = c;
+ }
+ /* Default to seq if nothing else has been specified. */
+ if (oacc_routine_attr->oacc_routine_seq
+ || clauses_ == NULL_TREE)
+ {
+ tree c = build_omp_clause (loc, OMP_CLAUSE_SEQ);
+ OMP_CLAUSE_CHAIN (c) = clauses_;
+ clauses_ = c;
+ }
+
+ /* If we saw more than one clause specifying the level of
+ parallelism... */
+ if (OMP_CLAUSE_CHAIN (clauses_) != NULL_TREE)
+ {
+ gfc_error ("Multiple loop axes specified for routine at %L",
+ &oacc_routine_attr->omp_clauses_locus);
+
+ /* ..., only one clause survives. */
+ OMP_CLAUSE_CHAIN (clauses_) = NULL_TREE;
+ }
+
+ OMP_CLAUSE_CHAIN (clauses_) = clauses;
+ clauses = clauses_;
+
+ oacc_routine_attr = oacc_routine_attr->next;
+ }
- switch (sym_attr.oacc_function)
+ /* For any chained symbol attributes for OpenACC routine, handle, and
+ clean these up. */
+ while (sym_attr.next != NULL)
{
- case OACC_FUNCTION_GANG:
- level = GOMP_DIM_GANG;
- break;
- case OACC_FUNCTION_WORKER:
- level = GOMP_DIM_WORKER;
- break;
- case OACC_FUNCTION_VECTOR:
- level = GOMP_DIM_VECTOR;
- break;
- case OACC_FUNCTION_SEQ:
- default:;
+ symbol_attribute *sym_attr_next = sym_attr.next->next;
+
+ gfc_error ("!$ACC ROUTINE already applied at %L",
+ &sym_attr.next->omp_clauses_locus);
+
+ free (sym_attr.next);
+
+ sym_attr.next = sym_attr_next;
}
- for (ix = GOMP_DIM_MAX; ix--;)
- dims = tree_cons (build_int_cst (boolean_type_node, ix >= level),
- integer_zero_node, dims);
+ if (sym_attr.oacc_routine)
+ {
+ gcc_checking_assert (clauses != NULL_TREE);
+ /* If we saw more than one set of symbol attributes for OpenACC
+ routine, only one clause survives. */
+ OMP_CLAUSE_CHAIN (clauses) = NULL_TREE;
- list = tree_cons (get_identifier ("oacc function"),
- dims, list);
+ /* Set the routine's level of parallelism. */
+ tree dims = build_oacc_routine_dims (clauses);
+#if 0
+ // TODO Can we call this before decl_attributes has been called, which happens only after returning from add_attributes_to_decl?
+ replace_oacc_fn_attrib (fndecl, dims);
+#else
+ list = tree_cons (get_identifier ("oacc function"),
+ dims, list);
+#endif
+ }
+ list = tree_cons (get_identifier ("omp declare target"),
+ NULL_TREE, list);
}
+ gcc_checking_assert (sym_attr.next == NULL);
return list;
}
diff --git gcc/testsuite/gfortran.dg/goacc/oaccdevlow-routine.f95 gcc/testsuite/gfortran.dg/goacc/oaccdevlow-routine.f95
index 2161fe2..6af19d5 100644
--- gcc/testsuite/gfortran.dg/goacc/oaccdevlow-routine.f95
+++ gcc/testsuite/gfortran.dg/goacc/oaccdevlow-routine.f95
@@ -20,7 +20,7 @@ subroutine ROUTINE
end subroutine ROUTINE
! Check the offloaded function's attributes.
-! { dg-final { scan-tree-dump-times "(?n)__attribute__\\(\\(omp declare target, oacc function \\(0 0, 1 0, 1 0\\)\\)\\)" 1 "ompexp" } }
+! { dg-final { scan-tree-dump-times "(?n)__attribute__\\(\\(oacc function \\(0 1, 1 0, 1 0\\), omp declare target\\)\\)" 1 "ompexp" } }
! Check the offloaded function's classification and compute dimensions (will
! always be [1, 1, 1] for target compilation).
diff --git gcc/testsuite/gfortran.dg/goacc/pr72741-2.f gcc/testsuite/gfortran.dg/goacc/pr72741-2.f
new file mode 100644
index 0000000..e0c35d6
--- /dev/null
+++ gcc/testsuite/gfortran.dg/goacc/pr72741-2.f
@@ -0,0 +1,39 @@
+ SUBROUTINE v_1
+!$ACC ROUTINE
+!$ACC ROUTINE ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE GANG ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE SEQ ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE WORKER ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ END SUBROUTINE v_1
+
+ SUBROUTINE sub_1
+ IMPLICIT NONE
+ EXTERNAL :: g_1
+!$ACC ROUTINE (g_1) GANG
+!$ACC ROUTINE (g_1) VECTOR ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) SEQ ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+
+ CALL v_1
+ CALL g_1
+ CALL ABORT
+ END SUBROUTINE sub_1
+
+ MODULE m_w_1
+ IMPLICIT NONE
+ EXTERNAL :: w_1
+!$ACC ROUTINE (w_1) WORKER
+!$ACC ROUTINE (w_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) SEQ ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) VECTOR ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+
+ CONTAINS
+ SUBROUTINE sub_2
+ CALL v_1
+ CALL w_1
+ CALL ABORT
+ END SUBROUTINE sub_2
+ END MODULE m_w_1
diff --git gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
new file mode 100644
index 0000000..d84cdf9
--- /dev/null
+++ gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
@@ -0,0 +1,16 @@
+ SUBROUTINE sub_1
+ IMPLICIT NONE
+!$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
+
+ CALL ABORT
+ END SUBROUTINE sub_1
+
+ MODULE m_w_1
+ IMPLICIT NONE
+!$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
+
+ CONTAINS
+ SUBROUTINE sub_2
+ CALL ABORT
+ END SUBROUTINE sub_2
+ END MODULE m_w_1
diff --git gcc/testsuite/gfortran.dg/goacc/pr72741.f90 gcc/testsuite/gfortran.dg/goacc/pr72741.f90
index cf89727..bf47fc2 100644
--- gcc/testsuite/gfortran.dg/goacc/pr72741.f90
+++ gcc/testsuite/gfortran.dg/goacc/pr72741.f90
@@ -1,12 +1,19 @@
SUBROUTINE v_1
!$ACC ROUTINE VECTOR WORKER ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE VECTOR ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ !$ACC ROUTINE ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ !$ACC ROUTINE GANG VECTOR ! { dg-error "Multiple loop axes" }
+ ! { dg-error "\\!\\\$ACC ROUTINE already applied" "already" { target *-*-* } 5 }
END SUBROUTINE v_1
SUBROUTINE sub_1
IMPLICIT NONE
EXTERNAL :: g_1
!$ACC ROUTINE (g_1) GANG WORKER ! { dg-error "Multiple loop axes" }
- !$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE (g_1) GANG ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ !$ACC ROUTINE (g_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ !$ACC ROUTINE (g_1) VECTOR GANG ! { dg-error "Multiple loop axes" }
+ ! { dg-error "\\!\\\$ACC ROUTINE already applied" "already" { target *-*-* } 15 }
CALL v_1
CALL g_1
@@ -17,7 +24,10 @@ MODULE m_w_1
IMPLICIT NONE
EXTERNAL :: w_1
!$ACC ROUTINE (w_1) WORKER SEQ ! { dg-error "Multiple loop axes" }
- !$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE (w_1) WORKER ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ !$ACC ROUTINE (w_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+ !$ACC ROUTINE (w_1) VECTOR WORKER ! { dg-error "Multiple loop axes" }
+ ! { dg-error "\\!\\\$ACC ROUTINE already applied" "already" { target *-*-* } 29 }
CONTAINS
SUBROUTINE sub_2
Grüße
Thomas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules)
2016-08-11 15:19 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules) Thomas Schwinge
@ 2016-08-11 15:40 ` Jakub Jelinek
2016-08-11 16:27 ` Thomas Schwinge
2016-08-11 16:44 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
1 sibling, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2016-08-11 15:40 UTC (permalink / raw)
To: Thomas Schwinge
Cc: Cesar Philippidis, gcc-patches, Fortran List, Tobias Burnus
On Thu, Aug 11, 2016 at 05:18:43PM +0200, Thomas Schwinge wrote:
> --- gcc/fortran/gfortran.h
> +++ gcc/fortran/gfortran.h
> @@ -729,7 +839,7 @@ ext_attr_t;
> extern const ext_attr_t ext_attr_list[];
>
> /* Symbol attribute structure. */
> -typedef struct
> +typedef struct symbol_attribute
> {
> /* Variable attributes. */
> unsigned allocatable:1, dimension:1, codimension:1, external:1, intrinsic:1,
> @@ -864,6 +974,13 @@ typedef struct
> /* Mentioned in OMP DECLARE TARGET. */
> unsigned omp_declare_target:1;
>
> + /* OpenACC routine. */
> + unsigned oacc_routine:1;
> + unsigned oacc_routine_gang:1;
> + unsigned oacc_routine_worker:1;
> + unsigned oacc_routine_vector:1;
> + unsigned oacc_routine_seq:1;
> +
> /* Mentioned in OACC DECLARE. */
> unsigned oacc_declare_create:1;
> unsigned oacc_declare_copyin:1;
> @@ -871,137 +988,24 @@ typedef struct
> unsigned oacc_declare_device_resident:1;
> unsigned oacc_declare_link:1;
>
> - /* This is an OpenACC acclerator function at level N - 1 */
> - ENUM_BITFIELD (oacc_function) oacc_function:3;
> -
> /* Attributes set by compiler extensions (!GCC$ ATTRIBUTES). */
> unsigned ext_attr:EXT_ATTR_NUM;
>
> + /* Location information for OMP clauses. */
> + //TODO: how to handle in module.c/symbol.c?
> + locus omp_clauses_locus;
> +
> /* The namespace where the attribute has been set. */
> struct gfc_namespace *volatile_ns, *asynchronous_ns;
> +
> + /* Chain to another set of symbol attributes. Currently only used for
> + OpenACC routine. */
> + //TODO: how to handle in module.c/symbol.c?
> + struct symbol_attribute *next;
While symbol_attribute is already bloated, I don't like bloating it this
much further. Do you really need it for all symbols, or just all subroutines?
In the latter case, it is much better to add some openacc specific pointer
into the namespace structure and stick everything you need into some custom
structure it will refer to. E.g. look at gfc_omp_declare_simd struct
in ns->omp_declare_simd.
omp_clauses_locus makes no sense, symbol_attribute contains parsed info from
many different clauses, which one it is?
Jakub
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules)
2016-08-11 15:40 ` Jakub Jelinek
@ 2016-08-11 16:27 ` Thomas Schwinge
2016-08-11 16:42 ` Jakub Jelinek
2016-08-16 1:55 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
0 siblings, 2 replies; 15+ messages in thread
From: Thomas Schwinge @ 2016-08-11 16:27 UTC (permalink / raw)
To: Cesar Philippidis, Jakub Jelinek; +Cc: gcc-patches, Fortran List, Tobias Burnus
Hi!
As Cesar asked for it, there is now a Git branch
tschwinge/omp/pr72741-wip containing these changes (plus some other
pending changes that I didn't single out at this time), at
<https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/tschwinge/omp/pr72741-wip>.
(I expect it does, but I didn't verify that this actually builds; I have
further changes on top of that.) Cesar, please tell me if you'd like me
to push this to GitHub, in case you want to use their review/commentary
functions, or the like.
On Thu, 11 Aug 2016 17:40:26 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Aug 11, 2016 at 05:18:43PM +0200, Thomas Schwinge wrote:
> > --- gcc/fortran/gfortran.h
> > +++ gcc/fortran/gfortran.h
> > /* Symbol attribute structure. */
> > -typedef struct
> > +typedef struct symbol_attribute
> > {
> While symbol_attribute is already bloated, I don't like bloating it this
> much further. Do you really need it for all symbols, or just all subroutines?
Certainly not for all symbole; just for what is valid to be used with the
OpenACC routine directive, which per OpenACC 2.0a, 2.13.1 Routine
Directive is:
In Fortran the syntax of the routine directive is:
!$acc routine clause-list
!$acc routine( name ) clause-list
In Fortran, the routine directive without a name may appear within the specification part of a subroutine or function definition, or within an interface body for a subroutine or function in an interface block, and applies to the containing subroutine or function. The routine directive with a name may appear in the specification part of a subroutine, function or module, and applies to the named subroutine or function.
(Pasting that in full just in case that contains some additional Fortran
lingo, meaning more than "subroutines".)
> In the latter case, it is much better to add some openacc specific pointer
> into the namespace structure and stick everything you need into some custom
> structure it will refer to. E.g. look at gfc_omp_declare_simd struct
> in ns->omp_declare_simd.
Thanks for the suggestion, I'll look into that.
> omp_clauses_locus makes no sense, symbol_attribute contains parsed info from
> many different clauses, which one it is?
Well, it makes some sense -- it works no worse than the existing code ;-)
-- but I agree that it's not exactly pretty. To the best of my
knowledge, in Fortran OpenACC/OpenMP clauses parsing, we're currently not
tracking (saving) specific location information for individual clauses
(at least, that's what a casual scan through the code, and
gfc_match_oacc_routine or gfc_match_omp_declare_target in particular make
me think: gfc_omp_clauses collects all clause data, but only contains a
single "locus loc" member (which maybe I should have used instead of
"old_loc", the location information for the directive itself?). Maybe I
misunderstood, and we do have more precise location information available
for individual clauses? In that case, I'll happily use that, of course.
Grüße
Thomas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules)
2016-08-11 16:27 ` Thomas Schwinge
@ 2016-08-11 16:42 ` Jakub Jelinek
2016-08-16 1:55 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
1 sibling, 0 replies; 15+ messages in thread
From: Jakub Jelinek @ 2016-08-11 16:42 UTC (permalink / raw)
To: Thomas Schwinge
Cc: Cesar Philippidis, gcc-patches, Fortran List, Tobias Burnus
On Thu, Aug 11, 2016 at 06:26:50PM +0200, Thomas Schwinge wrote:
> > > --- gcc/fortran/gfortran.h
> > > +++ gcc/fortran/gfortran.h
>
> > > /* Symbol attribute structure. */
> > > -typedef struct
> > > +typedef struct symbol_attribute
> > > {
>
> > While symbol_attribute is already bloated, I don't like bloating it this
> > much further. Do you really need it for all symbols, or just all subroutines?
>
> Certainly not for all symbole; just for what is valid to be used with the
> OpenACC routine directive, which per OpenACC 2.0a, 2.13.1 Routine
> Directive is:
>
> In Fortran the syntax of the routine directive is:
> !$acc routine clause-list
> !$acc routine( name ) clause-list
> In Fortran, the routine directive without a name may appear within the specification part of a subroutine or function definition, or within an interface body for a subroutine or function in an interface block, and applies to the containing subroutine or function. The routine directive with a name may appear in the specification part of a subroutine, function or module, and applies to the named subroutine or function.
>
> (Pasting that in full just in case that contains some additional Fortran
> lingo, meaning more than "subroutines".)
By "subroutines" I've meant of course also functions, those have their own
namespace structure too.
> > omp_clauses_locus makes no sense, symbol_attribute contains parsed info from
> > many different clauses, which one it is?
>
> Well, it makes some sense -- it works no worse than the existing code ;-)
> -- but I agree that it's not exactly pretty. To the best of my
> knowledge, in Fortran OpenACC/OpenMP clauses parsing, we're currently not
> tracking (saving) specific location information for individual clauses
> (at least, that's what a casual scan through the code, and
> gfc_match_oacc_routine or gfc_match_omp_declare_target in particular make
> me think: gfc_omp_clauses collects all clause data, but only contains a
> single "locus loc" member (which maybe I should have used instead of
> "old_loc", the location information for the directive itself?). Maybe I
> misunderstood, and we do have more precise location information available
> for individual clauses? In that case, I'll happily use that, of course.
The Fortran FE generally doesn't track locations of any of the attributes
symbols have, attributes as well as OpenMP clauses are represented just as
bits (for boolean stuff), etc., only if you have some expression you have
location for the expression.
I don't see what is so special on these clauses that they need to have
location tracked compared to say CONTIGUOUS or whatever other attribute, just
use the location of the function. Unless of course you want to rewrite all
the Fortran FE data structures and track detailed locations for everything.
But just treating selected OpenACC clauses specially, ignoring how the FE is
structured, is at least inconsistent with the rest.
Jakub
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling
2016-08-11 15:19 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules) Thomas Schwinge
2016-08-11 15:40 ` Jakub Jelinek
@ 2016-08-11 16:44 ` Cesar Philippidis
1 sibling, 0 replies; 15+ messages in thread
From: Cesar Philippidis @ 2016-08-11 16:44 UTC (permalink / raw)
To: Thomas Schwinge, gcc-patches, Fortran List; +Cc: Tobias Burnus, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 15956 bytes --]
On 08/11/2016 08:18 AM, Thomas Schwinge wrote:
>> --- a/gcc/fortran/module.c
>> +++ b/gcc/fortran/module.c
>
>> [...]
>> +DECL_MIO_NAME (oacc_function)
>> [...]
>
> As discussed between Cesar and Tobias, these module.c/symbol.c changes
> introduce an incompatibility in the Fortran module file format, which
> we'll want to avoid. Reverting that to use individual bit flags instead
> of the "enum oacc_function", I think that we're safe (but I have not
> verified that). On the other hand, given that I'm not at all handling in
> module.c/symbol.c the new "locus omp_clauses_locus" and "struct
> symbol_attribute *next" members that I'm adding to "symbol_attribute",
> I'm not sure whether I'm actually testing this properly. ;-) I guess I'm
> not.
How are you testing it? Basically, what you need to do is create two
source files, one containing a module and another with the program unit.
Then compile one of those files with the old, say gcc6 fortran, and the
other with trunk gfortran and try to link the .o files together.
I've attached some test cases so that you can experiment with. Each
driver file corresponds to a test file, with the exception of
test-driver which uses both test-interface.f90 and test-module.f90.
>> --- a/gcc/fortran/openmp.c
>> +++ b/gcc/fortran/openmp.c
>
>> @@ -1814,7 +1824,10 @@ gfc_match_oacc_routine (void)
>> != MATCH_YES))
>> return MATCH_ERROR;
>>
>> - if (sym != NULL)
>> + if (isym != NULL)
>> + /* There is nothing to do for intrinsic procedures. */
>> + ;
>
> We will want to check that no incompatible clauses are being specified,
> for example (but, low priority). I'm adding a hacky implementation of
> that.
So this is what I was overlooking in PR72741. For some reason I was only
considering invalid clauses of the form
!$acc routine gang worker
and not actually checking for compatible parallelism at the call sites.
The title "Fortran OpenACC routine directive doesn't properly handle
clauses specifying the level of parallelism" was kind of misleading.
Shouldn't the oaccdevlow pass already catch these types of errors already?
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/goacc/routine-7.f90
>> @@ -0,0 +1,69 @@
>> +! Test acc routines inside modules.
>> +
>> +! { dg-additional-options "-O0" }
>
> -O0 to prevent inlining of functions tagged with OpenACC routine
> directives, or another reason?
I'm not sure why, but that's probably it.
>> --- a/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
>> +++ b/libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
>> @@ -1,121 +1,95 @@
>> +! Test acc routines inside modules.
>>
>> ! { dg-do run }
>> -! { dg-additional-options "-cpp" }
>>
>> -#define M 8
>> -#define N 32
>> +module routines
>> + integer, parameter :: N = 32
>>
>> -program main
>> - integer :: i
>> - integer :: a(N)
>> - integer :: b(M * N)
>> -
>> - do i = 1, N
>> - a(i) = 0
>> - end do
>> +contains
>> + subroutine vector (a)
>> + implicit none
>> + !$acc routine vector
>> + integer, intent (inout) :: a(N)
>> + integer :: i
>> [...]
>
> This seems to completely rewrite the test case. Is that intentional, or
> should the original test case be preserved, and the changed/new/rewritten
> one be added as a new test case?
The original test was completely bogus because it had a lot of race
conditions when writing to variables. I could restore it, but then you'd
need to remove all of the gang, worker, vector clauses and force it to
run in seq. But that would defeat the intent behind the patch.
> Now, my hacky WIP patch.
>
> One big chunk of the gcc/fortran/gfortran.h changes is just to move some
> stuff around, without any changes, so that I can use "locus" in
> "symbol_attribute".
I agree with Jakub about creating a new gfc_acc_routine struct to
contain the locus and clauses for acc routines. That way, you can also
link them together for device_type.
But at the same time, since device_type isn't a priority for in the near
term, we might be better off using the existing oacc_function and nohost
attribute bits instead of introducing a new struct.
> I very much "cargo cult"ed all that "oacc_routine*" bit flag stuff in
> module.c/symbol.c, replicating what's being done for "omp target
> declare", without really knowing what I'm doing there. I will appreciate
> test cases actually exercising this code -- which doesn't currently at
> all handle the new "locus omp_clauses_locus" and "struct symbol_attribute
> *next" members that I'm adding to "symbol_attribute", as I've mentioned
> before. (But I suppose it should?)
>
> We're not implementing the OpenACC device_type clause at the moment, so
> the "TODO: handle device_type clauses" comment in
> gcc/fortran/openmp.c:gfc_match_oacc_routine is not a concern right now.
>
> With these changes, we're now actually also paying attention the clauses
> specified with the OpenACC routine directive with a name -- one of the
> things mentioned as missing in <http://gcc.gnu.org/PR72741> "Fortran
> OpenACC routine directive doesn't properly handle clauses specifying the
> level of parallelism".
>
> To handle several "pending" OpenACC routine directives, I had to add the
> "struct symbol_attribute *next" member to "symbol_attribute" -- I hope
> that doesn't disqualify the proposed changes as too ugly. (Several other
> structs already contain such "next" pointers, and the use is very much
> confined to only the OpenACC routine directive.) I will of course be
> happy to learn about a better/different way to do this.
>
> commit ca4a098dab72f27c6e1121aa7e5e49764921974e
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date: Thu Aug 11 16:34:22 2016 +0200
>
> [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling
> /* Structure and list of supported extension attributes. */
> typedef enum
> {
> @@ -729,7 +839,7 @@ ext_attr_t;
> extern const ext_attr_t ext_attr_list[];
>
> /* Symbol attribute structure. */
> -typedef struct
> +typedef struct symbol_attribute
> {
> /* Variable attributes. */
> unsigned allocatable:1, dimension:1, codimension:1, external:1, intrinsic:1,
> @@ -864,6 +974,13 @@ typedef struct
> /* Mentioned in OMP DECLARE TARGET. */
> unsigned omp_declare_target:1;
>
> + /* OpenACC routine. */
> + unsigned oacc_routine:1;
> + unsigned oacc_routine_gang:1;
> + unsigned oacc_routine_worker:1;
> + unsigned oacc_routine_vector:1;
> + unsigned oacc_routine_seq:1;
> +
> /* Mentioned in OACC DECLARE. */
> unsigned oacc_declare_create:1;
> unsigned oacc_declare_copyin:1;
> @@ -871,137 +988,24 @@ typedef struct
> unsigned oacc_declare_device_resident:1;
> unsigned oacc_declare_link:1;
>
> - /* This is an OpenACC acclerator function at level N - 1 */
> - ENUM_BITFIELD (oacc_function) oacc_function:3;
> -
I'm not sure what's better from a stylistic standpoint. Personally, I'd
prefer if all of these extra bits were coalesced into an oacc_routine
and oacc_declare enums. At least for acc routines, gang, worker, vector
and seq are all mutually exclusive.
> +++ gcc/fortran/module.c
> @@ -1986,6 +1986,7 @@ enum ab_attribute
> AB_IS_CLASS, AB_PROCEDURE, AB_PROC_POINTER, AB_ASYNCHRONOUS, AB_CODIMENSION,
> AB_COARRAY_COMP, AB_VTYPE, AB_VTAB, AB_CONTIGUOUS, AB_CLASS_POINTER,
> AB_IMPLICIT_PURE, AB_ARTIFICIAL, AB_UNLIMITED_POLY, AB_OMP_DECLARE_TARGET,
> + AB_OACC_ROUTINE, AB_OACC_ROUTINE_GANG, AB_OACC_ROUTINE_WORKER, AB_OACC_ROUTINE_VECTOR, AB_OACC_ROUTINE_SEQ,
> AB_ARRAY_OUTER_DEPENDENCY, AB_MODULE_PROCEDURE, AB_OACC_DECLARE_CREATE,
> AB_OACC_DECLARE_COPYIN, AB_OACC_DECLARE_DEVICEPTR,
> AB_OACC_DECLARE_DEVICE_RESIDENT, AB_OACC_DECLARE_LINK
> @@ -2044,6 +2045,11 @@ static const mstring attr_bits[] =
> minit ("IMPLICIT_PURE", AB_IMPLICIT_PURE),
> minit ("UNLIMITED_POLY", AB_UNLIMITED_POLY),
> minit ("OMP_DECLARE_TARGET", AB_OMP_DECLARE_TARGET),
> + minit ("OACC_ROUTINE", AB_OACC_ROUTINE),
> + minit ("OACC_ROUTINE_GANG", AB_OACC_ROUTINE_GANG),
> + minit ("OACC_ROUTINE_WORKER", AB_OACC_ROUTINE_WORKER),
> + minit ("OACC_ROUTINE_VECTOR", AB_OACC_ROUTINE_VECTOR),
> + minit ("OACC_ROUTINE_SEQ", AB_OACC_ROUTINE_SEQ),
> minit ("ARRAY_OUTER_DEPENDENCY", AB_ARRAY_OUTER_DEPENDENCY),
> minit ("MODULE_PROCEDURE", AB_MODULE_PROCEDURE),
> minit ("OACC_DECLARE_CREATE", AB_OACC_DECLARE_CREATE),
> @@ -2095,7 +2101,6 @@ DECL_MIO_NAME (procedure_type)
> DECL_MIO_NAME (ref_type)
> DECL_MIO_NAME (sym_flavor)
> DECL_MIO_NAME (sym_intent)
> -DECL_MIO_NAME (oacc_function)
> #undef DECL_MIO_NAME
>
> /* Symbol attributes are stored in list with the first three elements
> @@ -2117,8 +2122,6 @@ mio_symbol_attribute (symbol_attribute *attr)
> attr->proc = MIO_NAME (procedure_type) (attr->proc, procedures);
> attr->if_source = MIO_NAME (ifsrc) (attr->if_source, ifsrc_types);
> attr->save = MIO_NAME (save_state) (attr->save, save_status);
> - attr->oacc_function = MIO_NAME (oacc_function) (attr->oacc_function,
> - oacc_function_types);
>
> ext_attr = attr->ext_attr;
> mio_integer ((int *) &ext_attr);
> @@ -2236,6 +2239,16 @@ mio_symbol_attribute (symbol_attribute *attr)
> MIO_NAME (ab_attribute) (AB_VTAB, attr_bits);
> if (attr->omp_declare_target)
> MIO_NAME (ab_attribute) (AB_OMP_DECLARE_TARGET, attr_bits);
> + if (attr->oacc_routine)
> + MIO_NAME (ab_attribute) (AB_OACC_ROUTINE, attr_bits);
> + if (attr->oacc_routine_gang)
> + MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_GANG, attr_bits);
> + if (attr->oacc_routine_worker)
> + MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_WORKER, attr_bits);
> + if (attr->oacc_routine_vector)
> + MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_VECTOR, attr_bits);
> + if (attr->oacc_routine_seq)
> + MIO_NAME (ab_attribute) (AB_OACC_ROUTINE_SEQ, attr_bits);
> if (attr->array_outer_dependency)
> MIO_NAME (ab_attribute) (AB_ARRAY_OUTER_DEPENDENCY, attr_bits);
> if (attr->module_procedure)
> @@ -2422,6 +2435,21 @@ mio_symbol_attribute (symbol_attribute *attr)
> case AB_OMP_DECLARE_TARGET:
> attr->omp_declare_target = 1;
> break;
> + case AB_OACC_ROUTINE:
> + attr->oacc_routine = 1;
> + break;
> + case AB_OACC_ROUTINE_GANG:
> + attr->oacc_routine_gang = 1;
> + break;
> + case AB_OACC_ROUTINE_WORKER:
> + attr->oacc_routine_worker = 1;
> + break;
> + case AB_OACC_ROUTINE_VECTOR:
> + attr->oacc_routine_vector = 1;
> + break;
> + case AB_OACC_ROUTINE_SEQ:
> + attr->oacc_routine_seq = 1;
> + break;
> case AB_ARRAY_OUTER_DEPENDENCY:
> attr->array_outer_dependency =1;
> break;
That seems similar to what my patch is did, albeit with some checking
deferred. I don't think this would maintain backwards compatibility with
object files generated by older versions of gcc.
Regarding backwards compatibility, maybe we should teach gfortran to
default to seq parallelism if an oacc_function attribute is missing in
an older version of the .mod file? I'm not sure if there's anything we
can do about forwards compatibility, i.e., linking a module generated by
gcc7 with gcc6.
> diff --git gcc/fortran/openmp.c gcc/fortran/openmp.c
> index 05e4661..5a69e38 100644
> --- gcc/fortran/openmp.c
> +++ gcc/fortran/openmp.c
> @@ -1714,44 +1714,6 @@ gfc_match_oacc_cache (void)
> return MATCH_YES;
> }
>
> -/* Determine the loop level for a routine. Returns OACC_FUNCTION_NONE if
> - any error is detected. */
> -
> -static oacc_function
> -gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
> -{
> - int level = -1;
> - oacc_function ret = OACC_FUNCTION_SEQ;
> -
> - if (clauses)
> - {
> - unsigned mask = 0;
> -
> - if (clauses->gang)
> - {
> - level = GOMP_DIM_GANG, mask |= GOMP_DIM_MASK (level);
> - ret = OACC_FUNCTION_GANG;
> - }
> - if (clauses->worker)
> - {
> - level = GOMP_DIM_WORKER, mask |= GOMP_DIM_MASK (level);
> - ret = OACC_FUNCTION_WORKER;
> - }
> - if (clauses->vector)
> - {
> - level = GOMP_DIM_VECTOR, mask |= GOMP_DIM_MASK (level);
> - ret = OACC_FUNCTION_VECTOR;
> - }
> - if (clauses->seq)
> - level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level);
> -
> - if (mask != (mask & -mask))
> - ret = OACC_FUNCTION_NONE;
> - }
> -
> - return ret;
> -}
> -
> match
> gfc_match_oacc_routine (void)
> {
> @@ -1761,7 +1723,8 @@ gfc_match_oacc_routine (void)
> gfc_omp_clauses *c = NULL;
> gfc_oacc_routine_name *n = NULL;
> gfc_intrinsic_sym *isym = NULL;
> - oacc_function dims = OACC_FUNCTION_NONE;
> + symbol_attribute *add_attr = NULL;
> + const char *add_attr_name = NULL;
>
> old_loc = gfc_current_locus;
>
> @@ -1828,19 +1791,26 @@ gfc_match_oacc_routine (void)
> != MATCH_YES))
> return MATCH_ERROR;
>
> - dims = gfc_oacc_routine_dims (c);
> - if (dims == OACC_FUNCTION_NONE)
> - {
> - gfc_error ("Multiple loop axes specified for routine %C");
> - gfc_current_locus = old_loc;
> - return MATCH_ERROR;
> - }
> -
> if (isym != NULL)
> - /* There is nothing to do for intrinsic procedures. */
> - ;
> + {
> + //TODO gfc_intrinsic_sym doesn't have symbol_attribute?
> + //add_attr = &isym->attr;
> + //add_attr_name = NULL; //TODO
> + /* Fake it. TODO: handle device_type clauses... */
> + if (c->gang || c->worker || c->vector)
> + {
> + gfc_error ("Intrinsic symbol specified in !$ACC ROUTINE ( NAME )"
> + " at %C, with incompatible clauses specifying the level"
> + " of parallelism");
> + gfc_current_locus = old_loc;
> + return MATCH_ERROR;
> + }
> + }
> else if (sym != NULL)
> {
> + add_attr = &sym->attr;
> + add_attr_name = NULL; //TODO
> +
> n = gfc_get_oacc_routine_name ();
> n->sym = sym;
> n->clauses = NULL;
> @@ -1852,11 +1822,41 @@ gfc_match_oacc_routine (void)
> }
> else if (gfc_current_ns->proc_name)
> {
> - if (!gfc_add_omp_declare_target (&gfc_current_ns->proc_name->attr,
> - gfc_current_ns->proc_name->name,
> - &old_loc))
> + add_attr = &gfc_current_ns->proc_name->attr;
> + add_attr_name = gfc_current_ns->proc_name->name;
> + }
> + else
> + gcc_unreachable ();
> +
> + if (add_attr != NULL)
> + {
> + if (!gfc_add_omp_declare_target (add_attr, add_attr_name, &old_loc))
> goto cleanup;
> - gfc_current_ns->proc_name->attr.oacc_function = dims;
> + /* Skip over any existing symbol attributes capturing OpenACC routine
> + directives. */
> + while (add_attr->next != NULL)
> + add_attr = add_attr->next;
> + if (add_attr->oacc_routine)
> + {
> + add_attr->next = XCNEW (symbol_attribute);
> + gfc_clear_attr (add_attr->next);
> + add_attr = add_attr->next;
> + }
> + if (!gfc_add_oacc_routine (add_attr, add_attr_name, &old_loc))
> + goto cleanup;
> + if (c && c->gang
> + && !gfc_add_oacc_routine_gang (add_attr, add_attr_name, &old_loc))
> + goto cleanup;
> + if (c && c->worker
> + && !gfc_add_oacc_routine_worker (add_attr, add_attr_name, &old_loc))
> + goto cleanup;
> + if (c && c->vector
> + && !gfc_add_oacc_routine_vector (add_attr, add_attr_name, &old_loc))
> + goto cleanup;
> + if (c && c->seq
> + && !gfc_add_oacc_routine_seq (add_attr, add_attr_name, &old_loc))
> + goto cleanup;
> + add_attr->omp_clauses_locus = old_loc; //TODO OK to just assign that?
> }
This is another stylistic thing I don't like. Instead of having a single
function for mutually exclusive attributes, you need five. And each of
those functions are extremely similar, and I copy and paste issues when
dealing with such functions in the past.
With that in mind, I do see some value in preserving the routine clauses
and location information for device_type. But I thought that device_type
was more of a future project.
Cesar
[-- Attachment #2: module-test.tar --]
[-- Type: application/x-tar, Size: 10240 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling
2016-08-11 16:27 ` Thomas Schwinge
2016-08-11 16:42 ` Jakub Jelinek
@ 2016-08-16 1:55 ` Cesar Philippidis
2016-08-16 22:17 ` Thomas Schwinge
2019-02-28 20:37 ` [PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive Thomas Schwinge
1 sibling, 2 replies; 15+ messages in thread
From: Cesar Philippidis @ 2016-08-16 1:55 UTC (permalink / raw)
To: Thomas Schwinge, Jakub Jelinek; +Cc: gcc-patches, Fortran List, Tobias Burnus
[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]
On 08/11/2016 09:26 AM, Thomas Schwinge wrote:
> As Cesar asked for it, there is now a Git branch
> tschwinge/omp/pr72741-wip containing these changes (plus some other
> pending changes that I didn't single out at this time), at
> <https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/tschwinge/omp/pr72741-wip>.
> (I expect it does, but I didn't verify that this actually builds; I have
> further changes on top of that.) Cesar, please tell me if you'd like me
> to push this to GitHub, in case you want to use their review/commentary
> functions, or the like.
No, that git repository is fine.
> On Thu, 11 Aug 2016 17:40:26 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, Aug 11, 2016 at 05:18:43PM +0200, Thomas Schwinge wrote:
>>> --- gcc/fortran/gfortran.h
>>> +++ gcc/fortran/gfortran.h
>
>>> /* Symbol attribute structure. */
>>> -typedef struct
>>> +typedef struct symbol_attribute
>>> {
>
>> While symbol_attribute is already bloated, I don't like bloating it this
>> much further. Do you really need it for all symbols, or just all subroutines?
>
> Certainly not for all symbole; just for what is valid to be used with the
> OpenACC routine directive, which per OpenACC 2.0a, 2.13.1 Routine
> Directive is:
>
> In Fortran the syntax of the routine directive is:
> !$acc routine clause-list
> !$acc routine( name ) clause-list
> In Fortran, the routine directive without a name may appear within the specification part of a subroutine or function definition, or within an interface body for a subroutine or function in an interface block, and applies to the containing subroutine or function. The routine directive with a name may appear in the specification part of a subroutine, function or module, and applies to the named subroutine or function.
>
> (Pasting that in full just in case that contains some additional Fortran
> lingo, meaning more than "subroutines".)
I'm avoided that problem in this patch. For the moment, I'm ignoring the
device_type problem and handling all of the matching errors in
gfc_match_oacc_routine. You're patch was handling those errors in
add_attributes_to_decls, which I think is too late.
device_type will require extra handling down the road. But instead of
introducing new attributes, we can just use the existing
gfc_oacc_routine_name struct to capture and chain all of the clauses for
all of the different device_types. Then we can teach
add_attributes_to_decls to call gfc_oacc_routine_dims to generate the
appropriate OACC_FUNCTION attribute for a given set of device_type clauses.
Note that besides for checking for multiple acc routine directives, this
patch also handles the case where the optional name argument in 'acc
routine (NAME)' is the name of the current procedure. This was a TODO
item in gomp4.
Thomas, does this patch ok to you for gomp4?
Cesar
[-- Attachment #2: pr72741-gomp4.diff --]
[-- Type: text/x-patch, Size: 13085 bytes --]
2016-08-15 Cesar Philippidis <cesar@codesourcery.com>
gcc/fortran/
* openmp.c (gfc_match_oacc_routine): Error on repeated ACC ROUTINE
directives. Consider the optional NAME argument being the current
procedure name.
* trans-decl.c (add_attributes_to_decl): Use build_oacc_routine_dims
to construct the oacc_function attribute arguments.
gcc/testsuite/
* gfortran.dg/goacc/pr72741-2.f: New test.
* gfortran.dg/goacc/pr72741-intrinsic-1.f: Add test coverage.
* gfortran.dg/goacc/pr72741-intrinsic-2.f: Likewise.
* gfortran.dg/goacc/pr72741.f90: Likewise.
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 80f46c0..cb8efb8 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -1877,8 +1877,9 @@ gfc_match_oacc_cache (void)
return MATCH_YES;
}
-/* Determine the loop level for a routine. Returns OACC_FUNCTION_NONE if
- any error is detected. */
+/* Determine the loop level for a routine. Returns OACC_FUNCTION_NONE
+ if any error is detected. Note that this function needs to be
+ called repeatedly for each DEVICE_TYPE. */
static oacc_function
gfc_oacc_routine_dims (gfc_omp_clauses *clauses)
@@ -1925,6 +1926,7 @@ gfc_match_oacc_routine (void)
gfc_omp_clauses *c = NULL;
gfc_oacc_routine_name *n = NULL;
oacc_function dims = OACC_FUNCTION_NONE;
+ bool seen_error = false;
old_loc = gfc_current_locus;
@@ -1969,6 +1971,13 @@ gfc_match_oacc_routine (void)
gfc_current_locus = old_loc;
return MATCH_ERROR;
}
+
+ /* Set sym to NULL if it matches the current procedure's
+ name. This will simplify the check for duplicate ACC
+ ROUTINE attributes. */
+ if (gfc_current_ns->proc_name
+ && !strcmp (buffer, gfc_current_ns->proc_name->name))
+ sym = NULL;
}
else
{
@@ -1993,19 +2002,24 @@ gfc_match_oacc_routine (void)
!= MATCH_YES))
return MATCH_ERROR;
+ /* Scan for invalid routine geometry. */
dims = gfc_oacc_routine_dims (c);
if (dims == OACC_FUNCTION_NONE)
{
gfc_error ("Multiple loop axes specified in !$ACC ROUTINE at %C");
- goto cleanup;
+
+ /* Don't abort early, because it's important to let the user
+ know of any potential duplicate routine directives. */
+ seen_error = true;
}
if (isym != NULL)
{
if (c && (c->gang || c->worker || c->vector))
{
- gfc_error ("Intrinsic function specified in !$ACC ROUTINE ( NAME )"
- " at %C, with incompatible GANG, WORKER, or VECTOR clause");
+ gfc_error ("Intrinsic symbol specified in !$ACC ROUTINE ( NAME ) "
+ "at %C, with incompatible clauses specifying the level "
+ "of parallelism");
goto cleanup;
}
/* The intrinsic symbol has been marked with a SEQ, or with no clause at
@@ -2013,24 +2027,59 @@ gfc_match_oacc_routine (void)
}
else if (sym != NULL)
{
- n = gfc_get_oacc_routine_name ();
- n->sym = sym;
- n->clauses = NULL;
- n->next = NULL;
- if (gfc_current_ns->oacc_routine_names != NULL)
- n->next = gfc_current_ns->oacc_routine_names;
-
- gfc_current_ns->oacc_routine_names = n;
+ bool needs_entry = true;
+
+ /* Scan for any repeated routine directives on 'sym' and report
+ an error if necessary. TODO: Extend this function to scan
+ for compatible DEVICE_TYPE dims. */
+ for (n = gfc_current_ns->oacc_routine_names; n; n = n->next)
+ if (n->sym == sym)
+ {
+ needs_entry = false;
+ if (dims != gfc_oacc_routine_dims (n->clauses))
+ {
+ gfc_error ("$!ACC ROUTINE already applied at %C");
+ goto cleanup;
+ }
+ }
+
+ if (needs_entry)
+ {
+ n = gfc_get_oacc_routine_name ();
+ n->sym = sym;
+ n->clauses = c;
+ n->next = NULL;
+
+ if (gfc_current_ns->oacc_routine_names != NULL)
+ n->next = gfc_current_ns->oacc_routine_names;
+
+ gfc_current_ns->oacc_routine_names = n;
+ }
+
+ if (seen_error)
+ goto cleanup;
}
else if (gfc_current_ns->proc_name)
{
+ if (gfc_current_ns->proc_name->attr.oacc_function != OACC_FUNCTION_NONE
+ && !seen_error)
+ {
+ gfc_error ("!$ACC ROUTINE already applied at %C");
+ goto cleanup;
+ }
+
if (!gfc_add_omp_declare_target (&gfc_current_ns->proc_name->attr,
gfc_current_ns->proc_name->name,
&old_loc))
goto cleanup;
- gfc_current_ns->proc_name->attr.oacc_function = dims;
+
+ gfc_current_ns->proc_name->attr.oacc_function
+ = seen_error ? OACC_FUNCTION_SEQ : dims;
gfc_current_ns->proc_name->attr.oacc_function_nohost
= c ? c->nohost : false;
+
+ if (seen_error)
+ goto cleanup;
}
else
gcc_unreachable ();
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 5271268..785212f 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see
/* Only for gfc_trans_code. Shouldn't need to include this. */
#include "trans-stmt.h"
#include "gomp-constants.h"
+#include "omp-low.h"
#define MAX_LABEL_VALUE 99999
@@ -1329,29 +1330,27 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list)
if (sym_attr.oacc_function != OACC_FUNCTION_NONE)
{
- tree dims = NULL_TREE;
- int ix;
- int level = GOMP_DIM_MAX;
-
+ omp_clause_code code = OMP_CLAUSE_ERROR;
+ tree clause, dims;
+
switch (sym_attr.oacc_function)
{
case OACC_FUNCTION_GANG:
- level = GOMP_DIM_GANG;
+ code = OMP_CLAUSE_GANG;
break;
case OACC_FUNCTION_WORKER:
- level = GOMP_DIM_WORKER;
+ code = OMP_CLAUSE_WORKER;
break;
case OACC_FUNCTION_VECTOR:
- level = GOMP_DIM_VECTOR;
+ code = OMP_CLAUSE_VECTOR;
break;
case OACC_FUNCTION_SEQ:
- default:;
+ default:
+ code = OMP_CLAUSE_SEQ;
}
- for (ix = GOMP_DIM_MAX; ix--;)
- dims = tree_cons (build_int_cst (boolean_type_node, ix >= level),
- integer_zero_node, dims);
-
+ clause = build_omp_clause (UNKNOWN_LOCATION, code);
+ dims = build_oacc_routine_dims (clause);
list = tree_cons (get_identifier ("oacc function"),
dims, list);
}
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr72741-2.f b/gcc/testsuite/gfortran.dg/goacc/pr72741-2.f
new file mode 100644
index 0000000..5865144
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/pr72741-2.f
@@ -0,0 +1,39 @@
+ SUBROUTINE v_1
+!$ACC ROUTINE
+!$ACC ROUTINE ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE GANG ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE SEQ ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE WORKER ! { dg-error "ACC ROUTINE already applied" }
+ END SUBROUTINE v_1
+
+ SUBROUTINE sub_1
+ IMPLICIT NONE
+ EXTERNAL :: g_1
+!$ACC ROUTINE (g_1) GANG
+!$ACC ROUTINE (g_1) VECTOR ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) SEQ ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) ! { dg-error "ACC ROUTINE already applied" }
+
+ CALL v_1
+ CALL g_1
+ CALL ABORT
+ END SUBROUTINE sub_1
+
+ MODULE m_w_1
+ IMPLICIT NONE
+ EXTERNAL :: w_1
+!$ACC ROUTINE (w_1) WORKER
+!$ACC ROUTINE (w_1) ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) SEQ ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) ! { dg-error "ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) VECTOR ! { dg-error "ACC ROUTINE already applied" }
+
+ CONTAINS
+ SUBROUTINE sub_2
+ CALL v_1
+ CALL w_1
+ CALL ABORT
+ END SUBROUTINE sub_2
+ END MODULE m_w_1
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
index 4bff3e3..d84cdf9 100644
--- a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
+++ b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
@@ -1,17 +1,13 @@
-! Check for valid clauses with intrinsic function specified in !$ACC ROUTINE ( NAME ).
-
SUBROUTINE sub_1
IMPLICIT NONE
-!$ACC ROUTINE (ABORT)
-!$ACC ROUTINE (ABORT) SEQ
+!$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
CALL ABORT
END SUBROUTINE sub_1
MODULE m_w_1
IMPLICIT NONE
-!$ACC ROUTINE (ABORT) SEQ
-!$ACC ROUTINE (ABORT)
+!$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
CONTAINS
SUBROUTINE sub_2
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-2.f b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-2.f
index fed8e76..e5e3794 100644
--- a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-2.f
+++ b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-2.f
@@ -2,18 +2,18 @@
SUBROUTINE sub_1
IMPLICIT NONE
-!$ACC ROUTINE (ABORT) WORKER ! { dg-error "Intrinsic function specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible GANG, WORKER, or VECTOR clause" }
-!$ACC ROUTINE (ABORT) GANG ! { dg-error "Intrinsic function specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible GANG, WORKER, or VECTOR clause" }
-!$ACC ROUTINE (ABORT) VECTOR ! { dg-error "Intrinsic function specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible GANG, WORKER, or VECTOR clause" }
+!$ACC ROUTINE (ABORT) WORKER ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
+!$ACC ROUTINE (ABORT) GANG ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
+!$ACC ROUTINE (ABORT) VECTOR ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
CALL ABORT
END SUBROUTINE sub_1
MODULE m_w_1
IMPLICIT NONE
-!$ACC ROUTINE (ABORT) VECTOR ! { dg-error "Intrinsic function specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible GANG, WORKER, or VECTOR clause" }
-!$ACC ROUTINE (ABORT) WORKER ! { dg-error "Intrinsic function specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible GANG, WORKER, or VECTOR clause" }
-!$ACC ROUTINE (ABORT) GANG ! { dg-error "Intrinsic function specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible GANG, WORKER, or VECTOR clause" }
+!$ACC ROUTINE (ABORT) VECTOR ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
+!$ACC ROUTINE (ABORT) WORKER ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
+!$ACC ROUTINE (ABORT) GANG ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
CONTAINS
SUBROUTINE sub_2
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr72741.f90 b/gcc/testsuite/gfortran.dg/goacc/pr72741.f90
index cf89727..3fbd94f 100644
--- a/gcc/testsuite/gfortran.dg/goacc/pr72741.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/pr72741.f90
@@ -1,12 +1,24 @@
SUBROUTINE v_1
!$ACC ROUTINE VECTOR WORKER ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE VECTOR ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE GANG VECTOR ! { dg-error "Multiple loop axes" }
END SUBROUTINE v_1
+SUBROUTINE v_2
+ !$ACC ROUTINE(v_2) VECTOR WORKER ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE(v_2) VECTOR ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE(v_2) ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE(v_2) GANG VECTOR ! { dg-error "Multiple loop axes" }
+END SUBROUTINE v_2
+
SUBROUTINE sub_1
IMPLICIT NONE
EXTERNAL :: g_1
!$ACC ROUTINE (g_1) GANG WORKER ! { dg-error "Multiple loop axes" }
- !$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE (g_1) GANG ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE (g_1) ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE (g_1) VECTOR GANG ! { dg-error "Multiple loop axes" }
CALL v_1
CALL g_1
@@ -17,7 +29,9 @@ MODULE m_w_1
IMPLICIT NONE
EXTERNAL :: w_1
!$ACC ROUTINE (w_1) WORKER SEQ ! { dg-error "Multiple loop axes" }
- !$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Multiple loop axes" }
+ !$ACC ROUTINE (w_1) WORKER ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE (w_1) ! { dg-error "ACC ROUTINE already applied" }
+ !$ACC ROUTINE (w_1) VECTOR WORKER ! { dg-error "Multiple loop axes" }
CONTAINS
SUBROUTINE sub_2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling
2016-08-16 1:55 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
@ 2016-08-16 22:17 ` Thomas Schwinge
2019-02-28 20:37 ` [PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive Thomas Schwinge
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Schwinge @ 2016-08-16 22:17 UTC (permalink / raw)
To: Cesar Philippidis; +Cc: gcc-patches, Fortran List, Tobias Burnus, Jakub Jelinek
Hi!
On Mon, 15 Aug 2016 18:54:49 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> For the moment, I'm ignoring the
> device_type problem and handling all of the matching errors in
> gfc_match_oacc_routine.
OK for the moment; my idea has been to do it generally enough already
now, using generic infrastructure I have been/will be adding for C/C++,
so that device_type support will later be simple to implement for all
three front ends. But, let's leave that aside for the moment.
> You're patch was handling those errors in
> add_attributes_to_decls, which I think is too late.
I can't tell why that's "too late". Anyway, we can save this discussion
for later. ;-)
> Thomas, does this patch ok to you for gomp4?
Yes, please commit, so that we can move this whole thing forward. :-)
A few quick comments anyway:
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -1993,19 +2002,24 @@ gfc_match_oacc_routine (void)
> dims = gfc_oacc_routine_dims (c);
> if (dims == OACC_FUNCTION_NONE)
> {
> gfc_error ("Multiple loop axes specified in !$ACC ROUTINE at %C");
> - goto cleanup;
> +
> + /* Don't abort early, because it's important to let the user
> + know of any potential duplicate routine directives. */
> + seen_error = true;
> }
Hmm, I don't know if that's really important? I mean, if we run into
"Multiple loop axes specified", that is a hard semantic error already?
Anyway, this can be reconsidered later.
> if (isym != NULL)
> {
> if (c && (c->gang || c->worker || c->vector))
> {
> - gfc_error ("Intrinsic function specified in !$ACC ROUTINE ( NAME )"
> - " at %C, with incompatible GANG, WORKER, or VECTOR clause");
> + gfc_error ("Intrinsic symbol specified in !$ACC ROUTINE ( NAME ) "
> + "at %C, with incompatible clauses specifying the level "
> + "of parallelism");
> goto cleanup;
> }
You're re-introducing the wording I had used earlier, before I changed
that to the more specific one mentioning the clause names. Why change
that again? Also something the can be reconsidered later. (Goes
together with the gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-2.f
changes.)
> --- a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
> +++ b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
> @@ -1,17 +1,13 @@
> -! Check for valid clauses with intrinsic function specified in !$ACC ROUTINE ( NAME ).
> -
> SUBROUTINE sub_1
> IMPLICIT NONE
> -!$ACC ROUTINE (ABORT)
> -!$ACC ROUTINE (ABORT) SEQ
> +!$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
>
> CALL ABORT
> END SUBROUTINE sub_1
>
> MODULE m_w_1
> IMPLICIT NONE
> -!$ACC ROUTINE (ABORT) SEQ
> -!$ACC ROUTINE (ABORT)
> +!$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
This changes the intention of this test file? Another thing that can be
reconsidered later.
So, please commit as-is, and I'll then base my other changes on top of
that.
Grüße
Thomas
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PR72741] For all Fortran OpenACC 'routine' directive variants check for multiple clauses specifying the level of parallelism
2016-07-29 4:21 ` [gomp4] Fix PR72741 Cesar Philippidis
2016-07-01 20:41 ` [PATCH] OpenACC routines in fortran modules Cesar Philippidis
@ 2019-02-28 20:35 ` Thomas Schwinge
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Schwinge @ 2019-02-28 20:35 UTC (permalink / raw)
To: gcc-patches, fortran
[-- Attachment #1.1: Type: text/plain, Size: 540 bytes --]
Hi!
On Thu, 28 Jul 2016 21:21:29 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> Thomas found a bug in the fortran routine parser where errors involving
> invalid combinations of gang, worker, vector and seq clauses were
> getting suppressed. [...]
> This bug is also present in trunk, but [...]
Re-worked a bit, and committed to trunk in r269286 "[PR72741] For all
Fortran OpenACC 'routine' directive variants check for multiple clauses
specifying the level of parallelism", as attached.
Grüße
Thomas
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0002-PR72741-For-all-Fortran-OpenACC-routine-directive-va.patch --]
[-- Type: text/x-diff, Size: 5987 bytes --]
From 7378dd70e000e78ba7a266349077ab6ef36b5c62 Mon Sep 17 00:00:00 2001
From: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Thu, 28 Feb 2019 20:31:23 +0000
Subject: [PATCH 2/3] [PR72741] For all Fortran OpenACC 'routine' directive
variants check for multiple clauses specifying the level of parallelism
gcc/fortran/
PR fortran/72741
* gfortran.h (enum oacc_routine_lop): Add OACC_ROUTINE_LOP_ERROR.
* openmp.c (gfc_oacc_routine_lop, gfc_match_oacc_routine): Use it.
* trans-decl.c (add_attributes_to_decl): Likewise.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90: New file.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269286 138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/fortran/ChangeLog | 5 +++
gcc/fortran/gfortran.h | 3 +-
gcc/fortran/openmp.c | 13 ++++++--
gcc/fortran/trans-decl.c | 1 +
gcc/testsuite/ChangeLog | 3 ++
.../goacc/routine-multiple-lop-clauses-1.f90 | 32 +++++++++++++++++++
6 files changed, 53 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 78c6324d1b83..1c8f71252980 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,6 +1,11 @@
2019-02-28 Thomas Schwinge <thomas@codesourcery.com>
Cesar Philippidis <cesar@codesourcery.com>
+ PR fortran/72741
+ * gfortran.h (enum oacc_routine_lop): Add OACC_ROUTINE_LOP_ERROR.
+ * openmp.c (gfc_oacc_routine_lop, gfc_match_oacc_routine): Use it.
+ * trans-decl.c (add_attributes_to_decl): Likewise.
+
PR fortran/72741
PR fortran/89433
* openmp.c (gfc_match_oacc_routine): Accept intrinsic symbols.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index f0258b39ffd1..3e0f634c3a8e 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -323,7 +323,8 @@ enum oacc_routine_lop
OACC_ROUTINE_LOP_GANG,
OACC_ROUTINE_LOP_WORKER,
OACC_ROUTINE_LOP_VECTOR,
- OACC_ROUTINE_LOP_SEQ
+ OACC_ROUTINE_LOP_SEQ,
+ OACC_ROUTINE_LOP_ERROR
};
/* Strings for all symbol attributes. We use these for dumping the
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 6999ac34a1a9..50b91f2150ab 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2265,7 +2265,7 @@ gfc_oacc_routine_lop (gfc_omp_clauses *clauses)
}
if (n_lop_clauses > 1)
- gfc_error ("Multiple loop axes specified for routine");
+ ret = OACC_ROUTINE_LOP_ERROR;
}
return ret;
@@ -2280,6 +2280,7 @@ gfc_match_oacc_routine (void)
gfc_symbol *sym = NULL;
gfc_omp_clauses *c = NULL;
gfc_oacc_routine_name *n = NULL;
+ oacc_routine_lop lop = OACC_ROUTINE_LOP_NONE;
old_loc = gfc_current_locus;
@@ -2352,6 +2353,13 @@ gfc_match_oacc_routine (void)
!= MATCH_YES))
return MATCH_ERROR;
+ lop = gfc_oacc_routine_lop (c);
+ if (lop == OACC_ROUTINE_LOP_ERROR)
+ {
+ gfc_error ("Multiple loop axes specified for routine at %C");
+ goto cleanup;
+ }
+
if (isym != NULL)
{
/* Diagnose any OpenACC 'routine' directive that doesn't match the
@@ -2381,8 +2389,7 @@ gfc_match_oacc_routine (void)
gfc_current_ns->proc_name->name,
&old_loc))
goto cleanup;
- gfc_current_ns->proc_name->attr.oacc_routine_lop
- = gfc_oacc_routine_lop (c);
+ gfc_current_ns->proc_name->attr.oacc_routine_lop = lop;
}
else
/* Something has gone wrong, possibly a syntax error. */
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 20d453051a29..36b7fdd2701f 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -1425,6 +1425,7 @@ add_attributes_to_decl (symbol_attribute sym_attr, tree list)
code = OMP_CLAUSE_SEQ;
break;
case OACC_ROUTINE_LOP_NONE:
+ case OACC_ROUTINE_LOP_ERROR:
default:
gcc_unreachable ();
}
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index c45e7b7546a9..9f4c598951c3 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,6 +1,9 @@
2019-02-28 Thomas Schwinge <thomas@codesourcery.com>
Cesar Philippidis <cesar@codesourcery.com>
+ PR fortran/72741
+ * gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90: New file.
+
PR fortran/72741
PR fortran/89433
* gfortran.dg/goacc/routine-6.f90: Update
diff --git a/gcc/testsuite/gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90 b/gcc/testsuite/gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90
new file mode 100644
index 000000000000..8ca9be822ea5
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90
@@ -0,0 +1,32 @@
+! Check for multiple clauses specifying the level of parallelism.
+
+SUBROUTINE v_1
+ !$ACC ROUTINE VECTOR WORKER ! { dg-error "Multiple loop axes specified for routine" }
+END SUBROUTINE v_1
+
+SUBROUTINE sub_1
+ IMPLICIT NONE
+ EXTERNAL :: g_1
+ !$ACC ROUTINE (g_1) GANG WORKER ! { dg-error "Multiple loop axes specified for routine" }
+ !$ACC ROUTINE (ABORT) SEQ WORKER GANG VECTOR ! { dg-error "Multiple loop axes specified for routine" }
+ !$ACC ROUTINE WORKER SEQ ! { dg-error "Multiple loop axes specified for routine" }
+
+ CALL v_1
+ CALL g_1
+ CALL ABORT
+END SUBROUTINE sub_1
+
+MODULE m_w_1
+ IMPLICIT NONE
+ EXTERNAL :: w_1
+ !$ACC ROUTINE VECTOR GANG SEQ ! { dg-error "Multiple loop axes specified for routine" }
+ !$ACC ROUTINE (w_1) GANG WORKER SEQ ! { dg-error "Multiple loop axes specified for routine" }
+ !$ACC ROUTINE (ABORT) VECTOR GANG ! { dg-error "Multiple loop axes specified for routine" }
+
+CONTAINS
+ SUBROUTINE sub_2
+ CALL v_1
+ CALL w_1
+ CALL ABORT
+ END SUBROUTINE sub_2
+END MODULE m_w_1
--
2.17.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive
2016-08-16 1:55 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
2016-08-16 22:17 ` Thomas Schwinge
@ 2019-02-28 20:37 ` Thomas Schwinge
2019-03-21 19:57 ` Thomas Schwinge
1 sibling, 1 reply; 15+ messages in thread
From: Thomas Schwinge @ 2019-02-28 20:37 UTC (permalink / raw)
Cc: gcc-patches, fortran
[-- Attachment #1.1: Type: text/plain, Size: 3421 bytes --]
Hi!
On Mon, 15 Aug 2016 18:54:49 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> [...]
>
> Note that besides for checking for multiple acc routine directives, this
> patch also handles the case where the optional name argument in 'acc
> routine (NAME)' is the name of the current procedure. This was a TODO
> item in gomp4.
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -1969,6 +1971,13 @@ gfc_match_oacc_routine (void)
> gfc_current_locus = old_loc;
> return MATCH_ERROR;
> }
> +
> + /* Set sym to NULL if it matches the current procedure's
> + name. This will simplify the check for duplicate ACC
> + ROUTINE attributes. */
> + if (gfc_current_ns->proc_name
> + && !strcmp (buffer, gfc_current_ns->proc_name->name))
> + sym = NULL;
> }
> else
> {
I re-worked the code a bit, didn't find this necessary.
> dims = gfc_oacc_routine_dims (c);
> if (dims == OACC_FUNCTION_NONE)
> {
> gfc_error ("Multiple loop axes specified in !$ACC ROUTINE at %C");
> - goto cleanup;
> +
> + /* Don't abort early, because it's important to let the user
> + know of any potential duplicate routine directives. */
> + seen_error = true;
> }
Same for this.
> + bool needs_entry = true;
> +
> + /* Scan for any repeated routine directives on 'sym' and report
> + an error if necessary. TODO: Extend this function to scan
> + for compatible DEVICE_TYPE dims. */
> + for (n = gfc_current_ns->oacc_routine_names; n; n = n->next)
> + if (n->sym == sym)
> + {
> + needs_entry = false;
> + if (dims != gfc_oacc_routine_dims (n->clauses))
> + {
> + gfc_error ("$!ACC ROUTINE already applied at %C");
> + goto cleanup;
> + }
> + }
> +
> + if (needs_entry)
> + {
> + n = gfc_get_oacc_routine_name ();
This would leave us with a stray non-NULL 'n' in the '!needs_entry' case
(which potentially could confuse later processing?).
> + n->next = NULL;
> +
> + if (gfc_current_ns->oacc_routine_names != NULL)
> + n->next = gfc_current_ns->oacc_routine_names;
That's just 'n->next = gfc_current_ns->oacc_routine_names;'. ;-)
> else if (gfc_current_ns->proc_name)
> {
> + if (gfc_current_ns->proc_name->attr.oacc_function != OACC_FUNCTION_NONE
> + && !seen_error)
> + {
> + gfc_error ("!$ACC ROUTINE already applied at %C");
> + goto cleanup;
> + }
That need not emit an error if the previous is equal to current clause
specifying the level of parallelism.
> --- a/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
> +++ b/gcc/testsuite/gfortran.dg/goacc/pr72741-intrinsic-1.f
> @@ -1,17 +1,13 @@
> -! Check for valid clauses with intrinsic function specified in !$ACC ROUTINE ( NAME ).
> -
> SUBROUTINE sub_1
> IMPLICIT NONE
> -!$ACC ROUTINE (ABORT)
> -!$ACC ROUTINE (ABORT) SEQ
> +!$ACC ROUTINE (ABORT) SEQ VECTOR ! { dg-error "Intrinsic symbol specified in \\!\\\$ACC ROUTINE \\( NAME \\) at \\(1\\), with incompatible clauses specifying the level of parallelism" }
That changes what this test cases is supposed to be testing.
All that re-worked, and now committed to trunk in r269287 "[PR72741,
PR89433] Repeated use of the Fortran OpenACC 'routine' directive", as
attached.
Grüße
Thomas
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0003-PR72741-PR89433-Repeated-use-of-the-Fortran-OpenACC-.patch --]
[-- Type: text/x-diff, Size: 9296 bytes --]
From 35e99d5d3bd98eb2e2cee5d94ba09b6166dbeab2 Mon Sep 17 00:00:00 2001
From: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Thu, 28 Feb 2019 20:31:36 +0000
Subject: [PATCH 3/3] [PR72741, PR89433] Repeated use of the Fortran OpenACC
'routine' directive
gcc/fortran/
PR fortran/72741
PR fortran/89433
* openmp.c (gfc_match_oacc_routine): Handle repeated use of the
Fortran OpenACC 'routine' directive.
gcc/testsuite/
PR fortran/72741
PR fortran/89433
* gfortran.dg/goacc/routine-multiple-directives-1.f90: New file.
* gfortran.dg/goacc/routine-multiple-directives-2.f90: Likewise.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269287 138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/fortran/ChangeLog | 5 ++
gcc/fortran/openmp.c | 43 ++++++++--
gcc/testsuite/ChangeLog | 5 ++
.../goacc/routine-multiple-directives-1.f90 | 58 +++++++++++++
.../goacc/routine-multiple-directives-2.f90 | 82 +++++++++++++++++++
5 files changed, 185 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-1.f90
create mode 100644 gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-2.f90
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 1c8f71252980..6adb90aa4c01 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,6 +1,11 @@
2019-02-28 Thomas Schwinge <thomas@codesourcery.com>
Cesar Philippidis <cesar@codesourcery.com>
+ PR fortran/72741
+ PR fortran/89433
+ * openmp.c (gfc_match_oacc_routine): Handle repeated use of the
+ Fortran OpenACC 'routine' directive.
+
PR fortran/72741
* gfortran.h (enum oacc_routine_lop): Add OACC_ROUTINE_LOP_ERROR.
* openmp.c (gfc_oacc_routine_lop, gfc_match_oacc_routine): Use it.
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 50b91f2150ab..7a06eb58f5cf 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2374,17 +2374,44 @@ gfc_match_oacc_routine (void)
}
else if (sym != NULL)
{
- n = gfc_get_oacc_routine_name ();
- n->sym = sym;
- n->clauses = NULL;
- n->next = NULL;
- if (gfc_current_ns->oacc_routine_names != NULL)
- n->next = gfc_current_ns->oacc_routine_names;
-
- gfc_current_ns->oacc_routine_names = n;
+ bool add = true;
+
+ /* For a repeated OpenACC 'routine' directive, diagnose if it doesn't
+ match the first one. */
+ for (gfc_oacc_routine_name *n_p = gfc_current_ns->oacc_routine_names;
+ n_p;
+ n_p = n_p->next)
+ if (n_p->sym == sym)
+ {
+ add = false;
+ if (lop != gfc_oacc_routine_lop (n_p->clauses))
+ {
+ gfc_error ("!$ACC ROUTINE already applied at %C");
+ goto cleanup;
+ }
+ }
+
+ if (add)
+ {
+ n = gfc_get_oacc_routine_name ();
+ n->sym = sym;
+ n->clauses = c;
+ n->next = gfc_current_ns->oacc_routine_names;
+ gfc_current_ns->oacc_routine_names = n;
+ }
}
else if (gfc_current_ns->proc_name)
{
+ /* For a repeated OpenACC 'routine' directive, diagnose if it doesn't
+ match the first one. */
+ oacc_routine_lop lop_p = gfc_current_ns->proc_name->attr.oacc_routine_lop;
+ if (lop_p != OACC_ROUTINE_LOP_NONE
+ && lop != lop_p)
+ {
+ gfc_error ("!$ACC ROUTINE already applied at %C");
+ goto cleanup;
+ }
+
if (!gfc_add_omp_declare_target (&gfc_current_ns->proc_name->attr,
gfc_current_ns->proc_name->name,
&old_loc))
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9f4c598951c3..8a36b1f802e1 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,6 +1,11 @@
2019-02-28 Thomas Schwinge <thomas@codesourcery.com>
Cesar Philippidis <cesar@codesourcery.com>
+ PR fortran/72741
+ PR fortran/89433
+ * gfortran.dg/goacc/routine-multiple-directives-1.f90: New file.
+ * gfortran.dg/goacc/routine-multiple-directives-2.f90: Likewise.
+
PR fortran/72741
* gfortran.dg/goacc/routine-multiple-lop-clauses-1.f90: New file.
diff --git a/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-1.f90 b/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-1.f90
new file mode 100644
index 000000000000..6e12ee92155c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-1.f90
@@ -0,0 +1,58 @@
+! Check for valid cases of multiple OpenACC 'routine' directives.
+
+ SUBROUTINE s_1
+!$ACC ROUTINE(s_1)
+!$ACC ROUTINE(s_1) SEQ
+!$ACC ROUTINE SEQ
+ END SUBROUTINE s_1
+
+ SUBROUTINE s_2
+!$ACC ROUTINE
+!$ACC ROUTINE SEQ
+!$ACC ROUTINE(s_2)
+ END SUBROUTINE s_2
+
+ SUBROUTINE v_1
+!$ACC ROUTINE VECTOR
+!$ACC ROUTINE VECTOR
+!$ACC ROUTINE(v_1) VECTOR
+!$ACC ROUTINE VECTOR
+ END SUBROUTINE v_1
+
+ SUBROUTINE v_2
+!$ACC ROUTINE(v_2) VECTOR
+!$ACC ROUTINE VECTOR
+!$ACC ROUTINE(v_2) VECTOR
+ END SUBROUTINE v_2
+
+ SUBROUTINE sub_1
+ IMPLICIT NONE
+ EXTERNAL :: g_1
+!$ACC ROUTINE (g_1) GANG
+!$ACC ROUTINE (g_1) GANG
+!$ACC ROUTINE (g_1) GANG
+
+ CALL s_1
+ CALL s_2
+ CALL v_1
+ CALL v_2
+ CALL g_1
+ CALL ABORT
+ END SUBROUTINE sub_1
+
+ MODULE m_w_1
+ IMPLICIT NONE
+ EXTERNAL :: w_1
+!$ACC ROUTINE (w_1) WORKER
+!$ACC ROUTINE (w_1) WORKER
+
+ CONTAINS
+ SUBROUTINE sub_2
+ CALL s_1
+ CALL s_2
+ CALL v_1
+ CALL v_2
+ CALL w_1
+ CALL ABORT
+ END SUBROUTINE sub_2
+ END MODULE m_w_1
diff --git a/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-2.f90 b/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-2.f90
new file mode 100644
index 000000000000..54365ae3f4eb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/routine-multiple-directives-2.f90
@@ -0,0 +1,82 @@
+! Check for invalid (and some valid) cases of multiple OpenACC 'routine'
+! directives.
+
+ SUBROUTINE s_1
+!$ACC ROUTINE VECTOR WORKER ! { dg-error "Multiple loop axes specified for routine" }
+!$ACC ROUTINE(s_1)
+!$ACC ROUTINE GANG ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE(s_1) SEQ
+!$ACC ROUTINE
+!$ACC ROUTINE(s_1) WORKER ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE GANG VECTOR ! { dg-error "Multiple loop axes specified for routine" }
+ END SUBROUTINE s_1
+
+ SUBROUTINE s_2
+!$ACC ROUTINE(s_2) VECTOR WORKER ! { dg-error "Multiple loop axes specified for routine" }
+!$ACC ROUTINE
+!$ACC ROUTINE(s_2) GANG ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE SEQ
+!$ACC ROUTINE(s_2)
+!$ACC ROUTINE WORKER ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE(s_2) GANG VECTOR ! { dg-error "Multiple loop axes specified for routine" }
+ END SUBROUTINE s_2
+
+ SUBROUTINE v_1
+!$ACC ROUTINE VECTOR WORKER ! { dg-error "Multiple loop axes specified for routine" }
+!$ACC ROUTINE VECTOR
+!$ACC ROUTINE GANG ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE SEQ ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE(v_1) VECTOR
+!$ACC ROUTINE WORKER ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE GANG VECTOR ! { dg-error "Multiple loop axes specified for routine" }
+ END SUBROUTINE v_1
+
+ SUBROUTINE v_2
+!$ACC ROUTINE(v_2) VECTOR
+!$ACC ROUTINE(v_2) VECTOR WORKER ! { dg-error "Multiple loop axes specified for routine" }
+!$ACC ROUTINE(v_2) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE VECTOR
+!$ACC ROUTINE(v_2) GANG VECTOR ! { dg-error "Multiple loop axes specified for routine" }
+ END SUBROUTINE v_2
+
+ SUBROUTINE sub_1
+ IMPLICIT NONE
+ EXTERNAL :: g_1
+!$ACC ROUTINE (g_1) GANG
+!$ACC ROUTINE (g_1) GANG WORKER ! { dg-error "Multiple loop axes specified for routine" }
+!$ACC ROUTINE (g_1) VECTOR ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) SEQ ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (g_1) GANG
+!$ACC ROUTINE (g_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+
+ CALL s_1
+ CALL s_2
+ CALL v_1
+ CALL v_2
+ CALL g_1
+ CALL ABORT
+ END SUBROUTINE sub_1
+
+ MODULE m_w_1
+ IMPLICIT NONE
+ EXTERNAL :: w_1
+!$ACC ROUTINE (w_1) WORKER
+!$ACC ROUTINE (w_1) WORKER SEQ ! { dg-error "Multiple loop axes specified for routine" }
+!$ACC ROUTINE (w_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) WORKER
+!$ACC ROUTINE (w_1) SEQ ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+!$ACC ROUTINE (w_1) VECTOR ! { dg-error "\\!\\\$ACC ROUTINE already applied" }
+
+ CONTAINS
+ SUBROUTINE sub_2
+ CALL s_1
+ CALL s_2
+ CALL v_1
+ CALL v_2
+ CALL w_1
+ CALL ABORT
+ END SUBROUTINE sub_2
+ END MODULE m_w_1
--
2.17.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive
2019-02-28 20:37 ` [PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive Thomas Schwinge
@ 2019-03-21 19:57 ` Thomas Schwinge
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Schwinge @ 2019-03-21 19:57 UTC (permalink / raw)
To: gcc-patches, fortran
[-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --]
Hi!
On Thu, 28 Feb 2019 21:37:21 +0100, I wrote:
> On Mon, 15 Aug 2016 18:54:49 -0700, Cesar Philippidis <cesar@codesourcery.com> wrote:
> > [...]
> >
> > Note that besides for checking for multiple acc routine directives, this
> > patch also handles the case where the optional name argument in 'acc
> > routine (NAME)' is the name of the current procedure. This was a TODO
> > item in gomp4.
>
> > --- a/gcc/fortran/openmp.c
> > +++ b/gcc/fortran/openmp.c
>
> > @@ -1969,6 +1971,13 @@ gfc_match_oacc_routine (void)
> > gfc_current_locus = old_loc;
> > return MATCH_ERROR;
> > }
> > +
> > + /* Set sym to NULL if it matches the current procedure's
> > + name. This will simplify the check for duplicate ACC
> > + ROUTINE attributes. */
> > + if (gfc_current_ns->proc_name
> > + && !strcmp (buffer, gfc_current_ns->proc_name->name))
> > + sym = NULL;
> > }
> > else
> > {
>
> I re-worked the code a bit, didn't find this necessary.
Specifically, a very similar check has already been present, comparing to
'sym->name' instead of 'buffer'. (Not sure, if one is to be preferred
over the other, when/if they would ever be different. It feels like
instead of these strings, we should be comparing some kind of symbolic
"resolved" handle, "sym". And, as it should turn out, I have a cleanup
patch for next GCC development stage 1 to clean up that and other stuff
in 'gfc_match_oacc_routine'.)
Anyway, to clarify, I committed to trunk r269856 "[PR72741] The name in a
Fortran OpenACC 'routine' directive refers to the containing subroutine
or function", see attached.
Grüße
Thomas
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-PR72741-The-name-in-a-Fortran-OpenACC-routine-.trunk.patch --]
[-- Type: text/x-diff, Size: 2980 bytes --]
From 467b1bdb6c33711416a3ca270ac51b2b99f2f85b Mon Sep 17 00:00:00 2001
From: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Thu, 21 Mar 2019 19:54:51 +0000
Subject: [PATCH] [PR72741] The name in a Fortran OpenACC 'routine' directive
refers to the containing subroutine or function
gcc/fortran/
PR fortran/72741
* openmp.c (gfc_match_oacc_routine): Clarify.
gcc/testsuite/
PR fortran/72741
* gfortran.dg/goacc/routine-module-mod-1.f90: Update.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269856 138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/fortran/ChangeLog | 3 +++
gcc/fortran/openmp.c | 3 +++
gcc/testsuite/ChangeLog | 3 +++
gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90 | 4 ++--
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 2afab3920bda..111e3a266e9b 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,5 +1,8 @@
2019-03-21 Thomas Schwinge <thomas@codesourcery.com>
+ PR fortran/72741
+ * openmp.c (gfc_match_oacc_routine): Clarify.
+
PR fortran/72741
* module.c (verify_OACC_ROUTINE_LOP_NONE): New function.
(enum ab_attribute): Add AB_OACC_ROUTINE_LOP_GANG,
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 7a06eb58f5cf..1b1a0b4108fd 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2314,6 +2314,9 @@ gfc_match_oacc_routine (void)
if (st)
{
sym = st->n.sym;
+ /* If the name in a 'routine' directive refers to the containing
+ subroutine or function, then make sure that we'll later handle
+ this accordingly. */
if (gfc_current_ns->proc_name != NULL
&& strcmp (sym->name, gfc_current_ns->proc_name->name) == 0)
sym = NULL;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 8afdf3e980e9..0c94f6bcacf8 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,8 @@
2019-03-21 Thomas Schwinge <thomas@codesourcery.com>
+ PR fortran/72741
+ * gfortran.dg/goacc/routine-module-mod-1.f90: Update.
+
PR fortran/72741
* gfortran.dg/goacc/routine-module-1.f90: New file.
* gfortran.dg/goacc/routine-module-2.f90: Likewise.
diff --git a/gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90 b/gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90
index 3855b8c88596..23c673fe3bd1 100644
--- a/gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/routine-module-mod-1.f90
@@ -18,7 +18,7 @@ contains
subroutine s_2
implicit none
- !$acc routine seq
+ !$acc routine (s_2) seq
integer :: i
@@ -41,7 +41,7 @@ contains
subroutine w_1
implicit none
- !$acc routine worker
+ !$acc routine (w_1) worker
integer :: i
--
2.17.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-03-21 19:57 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-28 2:54 [gomp4] encode acc routine clauses inside fortran module files Cesar Philippidis
2016-07-29 4:21 ` [gomp4] Fix PR72741 Cesar Philippidis
2016-07-01 20:41 ` [PATCH] OpenACC routines in fortran modules Cesar Philippidis
2016-07-28 9:55 ` Tobias Burnus
2016-07-28 21:33 ` Cesar Philippidis
2016-08-11 15:19 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling (was: [PATCH] OpenACC routines in fortran modules) Thomas Schwinge
2016-08-11 15:40 ` Jakub Jelinek
2016-08-11 16:27 ` Thomas Schwinge
2016-08-11 16:42 ` Jakub Jelinek
2016-08-16 1:55 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
2016-08-16 22:17 ` Thomas Schwinge
2019-02-28 20:37 ` [PR72741, PR89433] Repeated use of the Fortran OpenACC 'routine' directive Thomas Schwinge
2019-03-21 19:57 ` Thomas Schwinge
2016-08-11 16:44 ` [WIP] [PR fortran/72741] Rework Fortran OpenACC routine clause handling Cesar Philippidis
2019-02-28 20:35 ` [PR72741] For all Fortran OpenACC 'routine' directive variants check for multiple clauses specifying the level of parallelism Thomas Schwinge
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).