[Patch] Fortran/OpenMP: Fix depend-clause handling
As found by Marcel, the 'depend' clause was differently handled in
'omp depobj(...) depend(...)' and in 'omp task depend(...)'.
The problem was that for a scalar pointer, depobj depended
on the pointer address - while task depended on the pointer-target address.
If one now combines depobj and direct var dependency, the dependency
is on different addresses, such that the dependency is not honored.
Marcel's example is testsuite/libgomp.fortran/depend-4.f90.
(Thanks for the report!)
I think in the real world, the problem is not that big as most
code either uses depobj or a variable and does not mix them. Likewise,
using the address of a temporary variable (cf. below) will also usually
work in terms of dependency.
The attached patch does:
- depend clause (as used by task etc):
For scalar allocatable/pointer, add another dereference
- For depobj (which handles the depend clause by itself)
- Fix array(element) handling by coping the depend-clause.
Before the result was 'D.123 = var; depobj = &D.123;'
which is really not intended.
- Several issues related to optional and allocatable/pointer.
OK for mainline? Does backporting to GCC 11 make sense?
Thanks,
Tobias
PS:
* I use the address of the pointer target (and not of the internal
pointer) variable, which requires that the address to which the pointer
points makes sense. (Technically, NULL and random value should be also okay,
I think.)
* Likewise for 'allocatable' - which also requires that the memory
is allocated.
* OpenMP uses the word 'storage location', which is defined according to the
last F2F
meeting as: "A data storage block in memory." (OpenMP Spec Pull Req. #3228)
* On the OpenMP side, there is the discussion to have a dependency on an integer
value instead of an address. (Motivated by the question whether ptr = NULL;
(void*)0x123; etc. should be valid or not.) → OpenMP Spec Issue #3226
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Fortran/OpenMP: Fix depend-clause handling
gcc/fortran/ChangeLog:
* trans-openmp.cc (gfc_trans_omp_clauses, gfc_trans_omp_depobj):
Depend on the proper addr, for ptr/alloc depend on pointee.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/depend-4.f90: New test.
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/depend-4.f90: New test.
gcc/fortran/trans-openmp.cc| 45 -
gcc/testsuite/gfortran.dg/gomp/depend-4.f90| 240 +
libgomp/testsuite/libgomp.fortran/depend-4.f90 | 107 +++
3 files changed, 386 insertions(+), 6 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 0eba0b3c3e1..a85f11e52d4 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -2890,7 +2890,9 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
gcc_assert (POINTER_TYPE_P (TREE_TYPE (decl)));
decl = build_fold_indirect_ref (decl);
}
- else if (DECL_P (decl))
+ else if (n->sym->attr.allocatable || n->sym->attr.pointer)
+ decl = build_fold_indirect_ref (decl);
+ if (DECL_P (decl))
TREE_ADDRESSABLE (decl) = 1;
OMP_CLAUSE_DECL (node) = decl;
}
@@ -5508,12 +5510,43 @@ gfc_trans_omp_depobj (gfc_code *code)
if (n)
{
tree var;
- if (n->expr)
-var = gfc_convert_expr_to_tree (&block, n->expr);
+ if (n->expr && n->expr->ref->u.ar.type != AR_FULL)
+ {
+ gfc_init_se (&se, NULL);
+ if (n->expr->ref->u.ar.type == AR_ELEMENT)
+ {
+ gfc_conv_expr_reference (&se, n->expr);
+ var = se.expr;
+ }
+ else
+ {
+ gfc_conv_expr_descriptor (&se, n->expr);
+ var = gfc_conv_array_data (se.expr);
+ }
+ gfc_add_block_to_block (&block, &se.pre);
+ gfc_add_block_to_block (&block, &se.post);
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (var)));
+ }
else
- var = gfc_get_symbol_decl (n->sym);
- if (!POINTER_TYPE_P (TREE_TYPE (var)))
-var = gfc_build_addr_expr (NULL, var);
+ {
+ var = gfc_get_symbol_decl (n->sym);
+ if (POINTER_TYPE_P (TREE_TYPE (var))
+ && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (TREE_TYPE (var
+ var = build_fold_indirect_ref (var);
+ if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (var)))
+ {
+ var = gfc_conv_descriptor_data_get (var);
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (var)));
+ }
+ else if ((n->sym->attr.allocatable || n->sym->attr.pointer)
+ && n->sym->attr.optional)
+ var = build_fold_indirect_ref (var);
+ else if (!POINTER_TYPE_P (TREE_TYPE (var)))
+ {
+ TREE_ADDRESSABLE (var) = 1;
+ var = gfc_build_addr_expr (NULL, var);
+ }
+ }
depobj = save_expr (depobj);
tree r = build_fold_indirect_ref_loc (loc, depobj);
gfc_add_expr_to_block (&block,
diff
Re: [Patch] Fortran/OpenMP: Fix depend-clause handling
On Tue, Feb 15, 2022 at 11:26:12AM +0100, Tobias Burnus wrote: > As found by Marcel, the 'depend' clause was differently handled in > 'omp depobj(...) depend(...)' and in 'omp task depend(...)'. > > The problem was that for a scalar pointer, depobj depended > on the pointer address - while task depended on the pointer-target address. > > If one now combines depobj and direct var dependency, the dependency > is on different addresses, such that the dependency is not honored. > Marcel's example is testsuite/libgomp.fortran/depend-4.f90. > (Thanks for the report!) > > > I think in the real world, the problem is not that big as most > code either uses depobj or a variable and does not mix them. Likewise, > using the address of a temporary variable (cf. below) will also usually > work in terms of dependency. > > > The attached patch does: > - depend clause (as used by task etc): > For scalar allocatable/pointer, add another dereference > > - For depobj (which handles the depend clause by itself) > - Fix array(element) handling by coping the depend-clause. > Before the result was 'D.123 = var; depobj = &D.123;' > which is really not intended. > - Several issues related to optional and allocatable/pointer. > > OK for mainline? Does backporting to GCC 11 make sense? Ok. Dunno about backporting, perhaps later. Jakub
Re: [Patch] Fortran/OpenMP: Fix depend-clause handling
Hi! On 2022-02-15T11:26:12+0100, Tobias Burnus wrote: > As found by Marcel, the 'depend' clause was differently handled in > 'omp depobj(...) depend(...)' and in 'omp task depend(...)'. (Cross-referencing GCC PR104545 "[OpenMP & Fortran] Pointers issue in combination of depobj construct and depend clause with depobj dependence-type".) > The attached patch [...] > gcc/fortran/trans-openmp.cc| 45 - > gcc/testsuite/gfortran.dg/gomp/depend-4.f90| 240 > + > libgomp/testsuite/libgomp.fortran/depend-4.f90 | 107 +++ The actual commit r12-7242-g3939c1b11279dc950d2f160eb940dd791f7b40f1 "Fortran/OpenMP: Fix depend-clause handling" also has: | gcc/testsuite/gfortran.dg/gomp/depend-5.f90| 82 + ... (yay for more test cases!), and that one I see partially FAIL in x86_64 '-m32'/'-mx32' testing: FAIL: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\(\\*\\(integer\\(kind=16\\)\\[0:\\] \\* restrict\\) aaa.data\\)\\[aaa.offset \\+ 2\\]\\)" 1 FAIL: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\(\\*\\(integer\\(kind=16\\)\\[0:\\] \\* restrict\\) daaa->data\\)\\[daaa->offset \\+ 2\\]\\)" 1 FAIL: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\(\\*\\(integer\\(kind=16\\)\\[0:\\] \\* restrict\\) doaaa->data\\)\\[doaaa->offset \\+ 2\\]\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\(\\*daa\\)\\[1\\]\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\(\\*doaa\\)\\[1\\]\\)" 1 FAIL: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\(integer\\(kind=16\\) \\*\\) \\(aap.data \\+ \\(sizetype\\) \\(\\(aap.offset \\+ aap.dim\\[0\\].stride \\* 2\\) \\* aap.span\\)\\)\\)" 1 FAIL: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\(integer\\(kind=16\\) \\*\\) \\(daap->data \\+ \\(sizetype\\) \\(\\(daap->offset \\+ daap->dim\\[0\\].stride \\* 2\\) \\* daap->span\\)\\)\\)" 1 FAIL: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\(integer\\(kind=16\\) \\*\\) \\(doaap->data \\+ \\(sizetype\\) \\(\\(doaap->offset \\+ doaap->dim\\[0\\].stride \\* 2\\) \\* doaap->span\\)\\)\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\*dosa\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\*dosp\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\*dsa\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*\\*dsp\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*doss\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*dss\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*sa\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:\\*sp\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:aa\\[1\\]\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O scan-tree-dump-times original "#pragma omp task depend\\(depobj:ss\\)" 1 PASS: gfortran.dg/gomp/depend-5.f90 -O (test for excess errors) Grüße Thomas - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
[Patch] Fortran/OpenMP: Fix depend-clause handling for c_ptr (was: [Patch] Fortran/OpenMP: Fix depend-clause handling)
On 15.02.22 11:56, Jakub Jelinek wrote:
On Tue, Feb 15, 2022 at 11:26:12AM +0100, Tobias Burnus wrote:
As found by Marcel, the 'depend' clause was differently handled in
'omp depobj(...) depend(...)' and in 'omp task depend(...)'.
As Marcel reported, there was still a problem with c_ptr.
Looking at the dump, I also spotted that for a nonoptional dummy
argument, scalar allocatable/pointers should have a '*' for depobj,
which I fixed. I additionally added a VALUE attribute test.
I then copied the depend-4.f90 to depend-6.f90 and replaced 'integer' by
'type(c_ptr)' as depend-clause variable (and 'integer(kind=4)' by 'void
*' in the expected dump). Otherwise, those two files should be identical.
I hope it now works and I did not miss anything in the dump.
OK?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Fortran/OpenMP: Fix depend-clause handling for c_ptr
gcc/fortran/ChangeLog:
* trans-openmp.cc (gfc_trans_omp_depobj): Fix to alloc/ptr dummy
and for c_ptr.
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/depend-4.f90: Add VALUE test, update scan test.
* gfortran.dg/gomp/depend-5.f90: Fix scan tree for -m32.
* gfortran.dg/gomp/depend-6.f90: New test.
gcc/fortran/trans-openmp.cc | 7 +-
gcc/testsuite/gfortran.dg/gomp/depend-4.f90 | 29 +++-
gcc/testsuite/gfortran.dg/gomp/depend-5.f90 | 12 +-
gcc/testsuite/gfortran.dg/gomp/depend-6.f90 | 259
4 files changed, 295 insertions(+), 12 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index e1c9d46add6..4d56a771349 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -5536,9 +5536,12 @@ gfc_trans_omp_depobj (gfc_code *code)
gcc_assert (POINTER_TYPE_P (TREE_TYPE (var)));
}
else if ((n->sym->attr.allocatable || n->sym->attr.pointer)
- && n->sym->attr.optional)
+ && n->sym->attr.dummy)
var = build_fold_indirect_ref (var);
- else if (!POINTER_TYPE_P (TREE_TYPE (var)))
+ else if (!POINTER_TYPE_P (TREE_TYPE (var))
+ || (n->sym->ts.f90_type == BT_VOID
+ && !POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (var)))
+ && !GFC_ARRAY_TYPE_P (TREE_TYPE (TREE_TYPE (var)
{
TREE_ADDRESSABLE (var) = 1;
var = gfc_build_addr_expr (NULL, var);
diff --git a/gcc/testsuite/gfortran.dg/gomp/depend-4.f90 b/gcc/testsuite/gfortran.dg/gomp/depend-4.f90
index d6686c1e48f..f6cf2fd2dd4 100644
--- a/gcc/testsuite/gfortran.dg/gomp/depend-4.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/depend-4.f90
@@ -7,7 +7,8 @@
! For pointers, it depends on the address of the pointer target
! For allocatable, on the allocated memory address
-subroutine foo(dss, dsp, dsa, daa, daaa, daap, doss, dosp, dosa, doaa, doaaa, doaap)
+subroutine foo(dss, dsp, dsa, daa, daaa, daap, doss, dosp, dosa, doaa, doaaa, doaap, &
+ dssv, dossv)
!use omp_lib
use iso_c_binding, only: c_intptr_t
implicit none (type, external)
@@ -18,8 +19,10 @@ subroutine foo(dss, dsp, dsa, daa, daaa, daap, doss, dosp, dosa, doaa, doaaa, do
optional :: doss, dosp, dosa, doaa, doaaa, doaap
allocatable :: sa, aaa, dsa, daaa, dosa, doaaa
pointer :: sp, aap, dsp, daap, dosp, doaap
+ integer, value :: dssv, dossv
+ optional :: dossv
- integer(omp_depend_kind) :: object(18)
+ integer(omp_depend_kind) :: object(20)
integer(omp_depend_kind) :: elem(9)
!$omp depobj(object(1)) depend(in: ss)
@@ -40,6 +43,8 @@ subroutine foo(dss, dsp, dsa, daa, daaa, daap, doss, dosp, dosa, doaa, doaaa, do
!$omp depobj(object(16)) depend(in: doaa)
!$omp depobj(object(17)) depend(in: doaaa)
!$omp depobj(object(18)) depend(in: doaap)
+ !$omp depobj(object(19)) depend(in: dssv)
+ !$omp depobj(object(20)) depend(in: dossv)
!$omp depobj(elem(1)) depend(in: aa(2))
!$omp depobj(elem(2)) depend(in: aaa(2))
@@ -107,6 +112,12 @@ subroutine foo(dss, dsp, dsa, daa, daaa, daap, doss, dosp, dosa, doaa, doaaa, do
!$omp task depend(out: doaap)
doaap = 4
!$omp end task
+!$omp task depend(out: dossv)
+ dossv = 4
+!$omp end task
+!$omp task depend(out: dssv)
+ dssv = 4
+!$omp end task
!$omp task depend(out: aa(2))
aa(2) = 4
@@ -168,8 +179,8 @@ end
! { dg-final { scan-tree-dump-times "&object\\\[4\\\] = .integer.kind=4.\\\[0:\\\] \\* restrict\\) aaa.data;" 1 "original" } }
! { dg-final { scan-tree-dump-times "&object\\\[5\\\] = .integer.kind=4.\\\[0:\\\] \\*\\) aap.data;" 1 "original" } }
! { dg-final { scan-tree-dump-times "&object\\\[6\\\] = dss;" 1 "original" } }
-! { dg-final { scan-tree-dump-times "&object\\\[7\\\] = dsp;" 1 "original" } }
-! { dg-final { scan-tree-dump-times "&object\\\[8\\\] = dsa;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "&obje
Re: [Patch] Fortran/OpenMP: Fix depend-clause handling for c_ptr (was: [Patch] Fortran/OpenMP: Fix depend-clause handling)
On Tue, Feb 15, 2022 at 06:01:09PM +0100, Tobias Burnus wrote: > OK? Ok. > Fortran/OpenMP: Fix depend-clause handling for c_ptr > > gcc/fortran/ChangeLog: > > * trans-openmp.cc (gfc_trans_omp_depobj): Fix to alloc/ptr dummy > and for c_ptr. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/gomp/depend-4.f90: Add VALUE test, update scan test. > * gfortran.dg/gomp/depend-5.f90: Fix scan tree for -m32. > * gfortran.dg/gomp/depend-6.f90: New test. > > gcc/fortran/trans-openmp.cc | 7 +- > gcc/testsuite/gfortran.dg/gomp/depend-4.f90 | 29 +++- > gcc/testsuite/gfortran.dg/gomp/depend-5.f90 | 12 +- > gcc/testsuite/gfortran.dg/gomp/depend-6.f90 | 259 > > 4 files changed, 295 insertions(+), 12 deletions(-) Jakub
Google Summer of Code 2022 Fortran projects
Hello gfortran contributors, can somebody please confirm they would be willing to mentor the Fortran projects listed at https://gcc.gnu.org/wiki/SummerOfCode ? I would also need to know that they are still up-to date and whether they are large (350 hours) or medium-sized (175 hours). Without all of the above, I will have to move the projects to the "old ideas" section down on the page next Monday. I know there has not been much interest among students in Fortran projects in the last years but perhaps this year, when the program is opened also to non-students, it could be better? Thanks a lot in advance, Martin
