Re: [Patch] Fortran/OpenMP: Fix mapping of array descriptors and deferred-length strings
Hi Tobias!
On 2023-02-23T17:42:08+0100, Tobias Burnus wrote:
> (Side note: this patch has been committed to OG12 as
> http://gcc.gnu.org/g:55a18d47442 )
I see og12 commit 55a18d4744258e3909568e425f9f473c49f9d13f
"Fortran/OpenMP: Fix mapping of array descriptors and deferred-length strings"
regress existing test cases as follows:
[-PASS:-]{+FAIL:+} gfortran.dg/goacc/finalize-1.f -O
scan-tree-dump-times gimple "(?n)#pragma omp target oacc_exit_data
map\\(delete:MEM <[^>]+> \\[\\(integer\\(kind=.\\)\\[0:\\] \\*\\)_[0-9]+\\]
\\[len: [^\\]]+\\]\\) map\\(to:del_f_p \\[pointer set, len: [0-9]+\\]\\)
map\\(alloc:del_f_p\\.data \\[pointer assign, bias: [^\\]]+\\]\\) finalize$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple
"(?n)#pragma omp target oacc_exit_data map\\(delete:del_f \\[len: [0-9]+\\]\\)
finalize$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple
"(?n)#pragma omp target oacc_exit_data map\\(force_from:MEM <[^>]+>
\\[\\(integer\\(kind=.\\)\\[0:\\] \\*\\)_[0-9]+\\] \\[len: [^\\]]+\\]\\)
map\\(to:cpo_f_p \\[pointer set, len: [0-9]+\\]\\) map\\(alloc:cpo_f_p\\.data
\\[pointer assign, bias: [^\\]]+\\]\\) finalize$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple
"(?n)#pragma omp target oacc_exit_data map\\(force_from:cpo_f \\[len:
[0-9]+\\]\\) finalize$" 1
@@ -54679,7 +54679,7 @@ PASS: gfortran.dg/goacc/finalize-1.f -O
scan-tree-dump-times gimple "(?n)#pr
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times original
"(?n)#pragma acc exit data map\\(from:\\*\\(integer\\(kind=.\\)\\[0:\\] \\*\\)
parm\\.1\\.data \\[len: [^\\]]+\\]\\) map\\(to:cpo_f_p \\[pointer set, len:
[0-9]+\\]\\) map\\(alloc:\\(integer\\(kind=1\\)\\[0:\\] \\* restrict\\)
cpo_f_p\\.data \\[pointer assign, bias: \\(.*int.*\\) parm\\.1\\.data -
\\(.*int.*\\) cpo_f_p\\.data\\]\\) finalize;$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times original
"(?n)#pragma acc exit data map\\(from:cpo_f\\) finalize;$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times original
"(?n)#pragma acc exit data map\\(from:cpo_r\\);$" 1
[-PASS:-]{+FAIL:+} gfortran.dg/goacc/finalize-1.f -O
scan-tree-dump-times original "(?n)#pragma acc exit data
map\\(release:\\*\\(integer\\(kind=.\\)\\[0:\\] \\*\\) parm\\.0\\.data \\[len:
[^\\]]+\\]\\) map\\(to:del_f_p \\[pointer set, len: [0-9]+\\]\\)
map\\(alloc:\\(integer\\(kind=1\\)\\[0:\\] \\* restrict\\) del_f_p\\.data
\\[pointer assign, bias: \\(.*int.*\\) parm\\.0\\.data - \\(.*int.*\\)
del_f_p\\.data\\]\\) finalize;$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times original
"(?n)#pragma acc exit data map\\(release:del_f\\) finalize;$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times original
"(?n)#pragma acc exit data map\\(release:del_r\\);$" 1
PASS: gfortran.dg/goacc/finalize-1.f -O (test for excess errors)
[-PASS:-]{+FAIL:+} gfortran.dg/gomp/pr78260-2.f90 -O
scan-tree-dump-times original "#pragma omp target data
map\\(tofrom:\\*\\(integer\\(kind=4\\)\\[0:\\] \\* restrict\\) __result->data
\\[len: D.[0-9]+ \\* 4\\]\\) map\\(to:\\*__result \\[pointer set, len: ..\\]\\)
map\\(alloc:\\(integer\\(kind=4\\)\\[0:\\] \\* restrict\\) __result->data
\\[pointer assign, bias: 0\\]\\) map\\(alloc:__result \\[pointer assign, bias:
0\\]\\)" 1
[-PASS:-]{+FAIL:+} gfortran.dg/gomp/pr78260-2.f90 -O
scan-tree-dump-times original "#pragma omp target data
map\\(tofrom:\\*\\(integer\\(kind=4\\)\\[0:\\] \\* restrict\\) arr.data \\[len:
D.[0-9]+ \\* 4\\]\\) map\\(to:arr \\[pointer set, len: ..\\]\\)
map\\(alloc:\\(integer\\(kind=4\\)\\[0:\\] \\* restrict\\) arr.data \\[pointer
assign, bias: 0\\]\\)" 1
PASS: gfortran.dg/gomp/pr78260-2.f90 -O scan-tree-dump-times original
"#pragma omp target data map\\(tofrom:\\*__result.0\\) map\\(alloc:__result.0
\\[pointer assign, bias: 0\\]\\)" 2
PASS: gfortran.dg/gomp/pr78260-2.f90 -O scan-tree-dump-times original
"#pragma omp target data map\\(tofrom:__result_f1\\)" 1
PASS: gfortran.dg/gomp/pr78260-2.f90 -O scan-tree-dump-times original
"#pragma omp target update to\\(\\*\\(integer\\(kind=4\\)\\[0:\\] \\*
restrict\\) __result->data \\[len: D.[0-9]+ \\* 4\\]\\)" 1
Do to the scan patterns need adjusting, or is something wrong?
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
Re: [Patch] Fortran/OpenMP: Fix mapping of array descriptors and deferred-length strings
Minor change to previous patch – it did not affect the mainline build
but it makes more sense this way and on OG12, i.e. with mapping
allocatable components (patch posted last year in Q1 during Stage 4),
it gave an ICE without this change as one '*' wasn't stripped.
--- b/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -3439,5 +3439,5 @@
&& n->sym->ts.deferred)
{
- if (!present && !DECL_P (decl))
+ if (!DECL_P (decl))
{
gcc_assert (TREE_CODE (decl) == INDIRECT_REF);
Otherwise, the same written before applies, see below.
(Side note: this patch has been committed to OG12 as
http://gcc.gnu.org/g:55a18d47442 )
On 21.02.23 12:57, Tobias Burnus wrote:
This patch moves some generic code for Fortran out of gimplify.cc
to trans-openmp.cc and fixes several issues related to mapping.
Tested with nvptx offloading.
OK for mainline?
Tobias
Caveats:
Besides the issues shown in the comment-out code, there remains also an
issue with implicit mapping - at least for deferred-length strings,
but I wouldn't be surprised if - at least depending on the used
'defaultmap' value (e.g. 'alloc') - there are also issues with array
descriptors.
Note:
Regarding the declare target check for mapping: Without declare
target, my assumption is that the hidden length variable will
get implicitly mapped if needed. Independent of deferred-length
or not, there is probably an issue with 'defaultmap(none)' and
the hidden variable. - In any case, I prefer to defer all those
issues to later (by having them captured in one/several PR).
Tobias
PS: This patch is a follow up to
[Patch] Fortran/OpenMP: Fix DT struct-component with 'alloc' and
array descr
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604887.html
which fixed part of the problems. But as discussed on IRC, it did
treat 'alloc'
as special and missed some other map types. - In addition, this patch
has a
much extended test coverage and fixes some more issues found that way.
-
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 mapping of array descriptors and deferred-length strings
Previously, array descriptors might have been mapped as 'alloc'
instead of 'to' for 'alloc', not updating the array bounds. The
'alloc' could also appear for 'data exit', failing with a libgomp
assert. In some cases, either array descriptors or deferred-length
string's length variable was not mapped. And, finally, some offset
calculations with array-sections mappings went wrong.
The testcases contain some comment-out tests which require follow-up
work and for which PR exist. Those mostly relate to deferred-length
strings which have several issues beyong OpenMP support.
gcc/fortran/ChangeLog:
* trans-decl.cc (gfc_get_symbol_decl): Add attributes
such as 'declare target' also to hidden artificial
variable for deferred-length character variables.
* trans-openmp.cc (gfc_trans_omp_array_section,
gfc_trans_omp_clauses, gfc_trans_omp_target_exit_data):
Improve mapping of array descriptors and deferred-length
string variables.
gcc/ChangeLog:
* gimplify.cc (gimplify_scan_omp_clauses): Remove Fortran
special case.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/target-enter-data-3.f90: Uncomment
'target exit data'.
* testsuite/libgomp.fortran/target-enter-data-4.f90: New test.
* testsuite/libgomp.fortran/target-enter-data-5.f90: New test.
* testsuite/libgomp.fortran/target-enter-data-6.f90: New test.
* testsuite/libgomp.fortran/target-enter-data-7.f90: New test.
gcc/fortran/trans-decl.cc | 2 +
gcc/fortran/trans-openmp.cc| 323
gcc/gimplify.cc| 42 +-
.../libgomp.fortran/target-enter-data-3.f90| 2 +-
.../libgomp.fortran/target-enter-data-4.f90| 540 +
.../libgomp.fortran/target-enter-data-5.f90| 540 +
.../libgomp.fortran/target-enter-data-6.f90| 392 +++
.../libgomp.fortran/target-enter-data-7.f90| 78 +++
8 files changed, 1783 insertions(+), 136 deletions(-)
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index ff64588..c46ffc1 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -1820,6 +1820,8 @@ gfc_get_symbol_decl (gfc_symbol * sym)
/* Add attributes to variables. Functions are handled elsewhere. */
attributes = add_attributes_to_decl (sym->attr, NULL_TREE);
decl_attributes (, attributes, 0);
+ if (sym->ts.deferred)
+decl_attributes (, attributes, 0);
/* Symbols from modules should have their assembler names mangled.
This is done here
