[committed] [OG10] Re: Re: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
If I got my tracking right, the og10 commit
4677091db1aa9d2a52e4839812bd73f47cc5e421 "[OpenMP, Fortran] Add
structure/derived-type element mapping" regresses:
[-PASS:-]{+FAIL:+} gfortran.dg/goacc/pr70828.f90 -O scan-tree-dump-times
gimple "omp target oacc_data map\\(tofrom:MEM\\[\\(c_char \\*\\)_[0-9]+\\]
\\[len: _[0-9]+\\]\\) map\\(alloc:data \\[pointer assign, b
ias: _[0-9]+\\]\\)" 1
[-PASS:-]{+FAIL:+} gfortran.dg/goacc/pr70828.f90 -O scan-tree-dump-times
gimple "omp target oacc_parallel map\\(force_present:MEM\\[\\(c_char
\\*\\)D\\.[0-9]+\\] \\[len: D\\.[0-9]+\\]\\) map\\(alloc:data \\[
pointer assign, bias: D\\.[0-9]+\\]\\)" 1
PASS: gfortran.dg/goacc/pr70828.f90 -O (test for excess errors)
The mapping in the Gimple dump has changed from:
_6 = parm.0.data;
...
#pragma omp target oacc_data map(tofrom:MEM[(c_char *)_6] [len: _5])
map(alloc:data [pointer assign, bias: _10])
to:
_6 = parm.0.data;
_7 = (integer(kind=8)) _6
data.2_8 = (integer(kind=8))
_9 = _7 - data.2_8;
_10 = _9 / 4;
...
#pragma omp target oacc_data map(tofrom:data[_10] [len: _5]) map(alloc:data
[pointer assign, bias: _14])
i.e. It is now mapping the array data starting from the offset that parm.0.data
is at relative to data, rather than from the memory at parm.0.data directly.
This is due to the change in array handling in this part of the patch:
if (TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
- ptr2 = build_fold_addr_expr (decl);
+ {
+ tree offset;
+ ptr2 = build_fold_addr_expr (decl);
+ offset = fold_build2 (MINUS_EXPR, ptrdiff_type_node, ptr,
+ fold_convert (ptrdiff_type_node, ptr2));
+ offset = build2 (TRUNC_DIV_EXPR, ptrdiff_type_node,
+ offset, fold_convert (ptrdiff_type_node, elemsz));
+ offset = build4_loc (input_location, ARRAY_REF,
+ TREE_TYPE (TREE_TYPE (decl)),
+ decl, offset, NULL_TREE, NULL_TREE);
+ OMP_CLAUSE_DECL (node) = offset;
+ }
As this does not cause a change in program behaviour, I believe it is enough to
just change the expected output in the testcase. I have committed the attached
patch to the OG10 branch as 'obvious'.
Kwok
From 0b999612a0205da31e3948f67cc754e9208e85fb Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung
Date: Wed, 19 Aug 2020 12:50:42 -0700
Subject: [PATCH] Fix gfortran.dg/goacc/pr70828.f90 testcase
Array mapping was changed by the patch '[OpenMP, Fortran] Add
structure/derived-type element mapping'.
2020-08-19 Kwok Cheung Yeung
gcc/testsuite/
* gfortran.dg/goacc/pr70828.f90: Update expected output in Gimple
dump.
---
gcc/testsuite/ChangeLog.omp | 5 +
gcc/testsuite/gfortran.dg/goacc/pr70828.f90 | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/ChangeLog.omp b/gcc/testsuite/ChangeLog.omp
index 19395cf..ffc8f63 100644
--- a/gcc/testsuite/ChangeLog.omp
+++ b/gcc/testsuite/ChangeLog.omp
@@ -1,3 +1,8 @@
+2020-08-19 Kwok Cheung Yeung
+
+ * gfortran.dg/goacc/pr70828.f90: Update expected output in Gimple
+ dump.
+
2020-08-18 Kwok Cheung Yeung
Backport from mainline
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr70828.f90
b/gcc/testsuite/gfortran.dg/goacc/pr70828.f90
index 2e58120..fcfe086 100644
--- a/gcc/testsuite/gfortran.dg/goacc/pr70828.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/pr70828.f90
@@ -18,5 +18,5 @@ program test
!$acc end data
end program test
-! { dg-final { scan-tree-dump-times "omp target oacc_data
map\\(tofrom:MEM\\\[\\(c_char \\*\\)\_\[0-9\]+\\\] \\\[len: _\[0-9\]+\\\]\\)
map\\(alloc:data \\\[pointer assign, bias: _\[0-9\]+\\\]\\)" 1 "gimple" } }
-! { dg-final { scan-tree-dump-times "omp target oacc_parallel
map\\(force_present:MEM\\\[\\(c_char \\*\\)D\\.\[0-9\]+\\\] \\\[len:
D\\.\[0-9\]+\\\]\\) map\\(alloc:data \\\[pointer assign, bias:
D\\.\[0-9\]+\\\]\\)" 1 "gimple" } }
+! { dg-final { scan-tree-dump-times "omp target oacc_data
map\\(tofrom:data\\\[\_\[0-9\]+\\\] \\\[len: _\[0-9\]+\\\]\\) map\\(alloc:data
\\\[pointer assign, bias: _\[0-9\]+\\\]\\)" 1 "gimple" } }
+! { dg-final { scan-tree-dump-times "omp target oacc_parallel
map\\(force_present:data\\\[D\\.\[0-9\]+\\\] \\\[len: D\\.\[0-9\]+\\\]\\)
map\\(alloc:data \\\[pointer assign, bias: D\\.\[0-9\]+\\\]\\)" 1 "gimple" } }
--
2.8.1
Re: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
Hi!
If I got my tracking right, the og10 commit
4677091db1aa9d2a52e4839812bd73f47cc5e421 "[OpenMP, Fortran] Add
structure/derived-type element mapping" regresses:
[-PASS:-]{+FAIL:+} gfortran.dg/goacc/pr70828.f90 -O
scan-tree-dump-times gimple "omp target oacc_data map\\(tofrom:MEM\\[\\(c_char
\\*\\)_[0-9]+\\] \\[len: _[0-9]+\\]\\) map\\(alloc:data \\[pointer assign, b
ias: _[0-9]+\\]\\)" 1
[-PASS:-]{+FAIL:+} gfortran.dg/goacc/pr70828.f90 -O
scan-tree-dump-times gimple "omp target oacc_parallel
map\\(force_present:MEM\\[\\(c_char \\*\\)D\\.[0-9]+\\] \\[len:
D\\.[0-9]+\\]\\) map\\(alloc:data \\[
pointer assign, bias: D\\.[0-9]+\\]\\)" 1
PASS: gfortran.dg/goacc/pr70828.f90 -O (test for excess errors)
Tobias, please have a look.
And then, the issues mentioned before (Julian, Tobias -- that's what we
talked about on the phone call earlier today):
On 2020-07-15T08:33:00+0200, I wrote:
> On 2020-06-24T19:32:09+0200, Tobias Burnus wrote:
>> (OpenMP 5 extends this a lot, but this is about OpenMP 4.5.
>> It touches code which is also used by OpenACC's attach/detach.)
>>
>> @OpenACC/Julian: I think the character attach/detach for
>> deferred-length strings does not work properly with OpenACC;
>> I did not touch this code – but I think it needs some love.
>
> Please file a PR.
>
>> This code adds support for
>>map(dt%comp, dt%comp2)
>> where "comp" can be either a nonpointer, nonallocatable element
>> scalar, array or array section. Or it can be a pointer - where
>> character strings are one complication as for deferred-length
>> ones, the length is stored in an extra DT component.
>>
>> While testing, I encountered two bugs, one relating to kind=4
>> character string (patch pending review; PR95837)
>> not part of testcase) and one related to deferred-length
>> character strings (commented in the test case; larger issue;
>> PR95868).
>>
>> Like always, some more tests/testcase probably would not harm.
>>
>> Regarding the patch:
>>
>> (a) openmp.c:
>> This enabled component matching for 'map(' and
>> piggybacks on the OpenACC code for the checks. I think that
>> some additional checks might be useful – and I hope that no
>> check is too strict.
>> The "depend" clause was excluded as one otherwise gets a
>> testsuite fails due to the is-contiguous check.
>>
>> (b) trans-openmp.c:
>> - gfc_trans_omp_clauses now has a "bool openacc".
>> - GOMP_MAP_ATTACH_DETACH is replaced by GOMP_MAP_ALWAYS_POINTER
>> - For arrays, the mapping of the descriptor is squeezed before
>>"node" which contains the data transfer (var.desc.data mapping
>>followed by the always_pointer for the mapping).
>>In this array case, the latter gets a pointless cast in order
>>to prevent that for both var.desc and var.desc.data memory gets
>>allocated in the struct.
>>→ That's also the reason the big switch table is moved up.
>> - For deferred-length strings, the string-length is in an extra
>>struct element (derived-type component) and will be mapped in
>>addition.
>> - Bugs in the previous version:
>>* gfc_trans_omp_array_section for "element == true", the size
>> of a pointer instead of the size of the element was mapped.
>>* For string variables (with constant length) the kind=4 was
>> not properly handled.
>>* Allocatable scalars were not handled – missing second clause
>> for the always_pointer (and attach_detach, I assume)
>
> I understand correctly that your remark "Bugs in the previous version"
> translates to "bugs still existing on releases/gcc-10 branch for OpenACC
> 'attach'/'detach'"? Should we thus backport to releases/gcc-10 branch
> this commit 102502e32ea4e8a75d6b252ba319d09d735d9aa7 "[OpenMP, Fortran]
> Add structure/derived-type element mapping", and fixup commit
> 524862db444b6544c6dc87c5f06f351100ecf50d "Fix goacc/finalize-1.f tree
> dump-scanning for -m32"?
Grüße
Thomas
>> Comments, remarks, suggestions?
>> Otherwise: OK for the trunk?
>
>> [OpenMP, Fortran] Add structure/derived-type element mapping
>>
>> gcc/fortran/ChangeLog:
>>
>> * openmp.c (gfc_match_omp_clauses): Match also derived-type
>> component refs in OMP_CLAUSE_MAP.
>> (resolve_omp_clauses): Resolve those.
>> * trans-openmp.c (gfc_trans_omp_array_section, gfc_trans_omp_clauses):
>> Handle OpenMP structure-element mapping.
>> (gfc_trans_oacc_construct, gfc_trans_oacc_executable_directive,
>> (gfc_trans_oacc_combined_directive, gfc_trans_oacc_declare): Update
>> add openacc=true in gfc_trans_omp_clauses call.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gfortran.dg/goacc/finalize-1.f: Update dump scan pattern.
>> * gfortran.dg/gomp/map-1.f90: Update dg-error.
>> * gfortran.dg/gomp/map-2.f90: New test.
>>
>>
>> libgomp/ChangeLog:
>>
>> * testsuite/libgomp.fortran/struct-elem-map-1.f90: New test.
>>
>> gcc/fortran/openmp.c | 5 +-
>> gcc/fortran/trans-openmp.c
Re: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
On 6/24/20 7:32 PM, Tobias Burnus wrote: While testing, I encountered two bugs, one relating to kind=4 character string (patch pending review; PR95837) not part of testcase) As that PR has been committed, I updated the testcase to check character(kind=4) as well. (I also removed the unused variables to silence -Wall warnings when running manually.) Committed as obvious. Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter commit e0685fadb6aa7c9cc895bc14cbbe2b9026fa3a94 Author: Tobias Burnus Date: Wed Jul 15 12:29:44 2020 +0200 libgomp.fortran/struct-elem-map-1.f90: Add char kind=4 tests As the Fortran PR 95837 has been fixed, the test could be be added. libgomp/ChangeLog: * testsuite/libgomp.fortran/struct-elem-map-1.f90: Remove unused variables; add character(kind=4) tests; update TODO comment. diff --git a/libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90 b/libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90 index f18eeb90165..58550c79d69 100644 --- a/libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90 +++ b/libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90 @@ -2,11 +2,9 @@ ! ! Test OpenMP 4.5 structure-element mapping -! TODO: character(kind=4,...) needs to be tested, but depends on -! PR fortran/95837 -! TODO: ...%str4 should be tested but that currently fails due to +! TODO: ...%str4 + %uni4 should be tested but that currently fails due to ! PR fortran/95868 (see commented lined) -! TODO: Test also array-valued var, nested derived types, +! TODO: Test also 'var' as array and/or pointer; nested derived types, ! type-extended types. program main @@ -22,6 +20,10 @@ program main character(len=5) :: str2(4) character(len=:), pointer :: str3 => null() character(len=:), pointer :: str4(:) => null() +character(kind=4, len=5) :: uni1 +character(kind=4, len=5) :: uni2(4) +character(kind=4, len=:), pointer :: uni3 => null() +character(kind=4, len=:), pointer :: uni4(:) => null() end type t2 integer :: i @@ -38,8 +40,7 @@ program main contains ! Implicitly mapped – but no pointers are mapped subroutine one() -type(t2) :: var, var2(4) -type(t2), pointer :: var3, var4(:) +type(t2) :: var print '(g0)', ' TESTCASE "one" ' @@ -47,11 +48,15 @@ contains b = 2, c = cmplx(-1.0_8, 2.0_8,kind=8), & d = [(-3*i, i = 1, 10)], & str1 = "abcde", & - str2 = ["12345", "67890", "ABCDE", "FGHIJ"]) + str2 = ["12345", "67890", "ABCDE", "FGHIJ"], & + uni1 = 4_"abcde", & + uni2 = [4_"12345", 4_"67890", 4_"ABCDE", 4_"FGHIJ"]) allocate (var%e, source=99) allocate (var%f, source=[22, 33, 44, 55]) allocate (var%str3, source="HelloWorld") allocate (var%str4, source=["Let's", "Go!!!"]) +allocate (var%uni3, source=4_"HelloWorld") +allocate (var%uni4, source=[4_"Let's", 4_"Go!!!"]) !$omp target map(tofrom:var) if (var%a /= 1) stop 1 @@ -60,15 +65,16 @@ contains if (any (var%d /= [(-3*i, i = 1, 10)])) stop 4 if (var%str1 /= "abcde") stop 5 if (any (var%str2 /= ["12345", "67890", "ABCDE", "FGHIJ"])) stop 6 + if (var%uni1 /= 4_"abcde") stop 7 + if (any (var%uni2 /= [4_"12345", 4_"67890", 4_"ABCDE", 4_"FGHIJ"])) stop 8 !$omp end target -deallocate(var%e, var%f, var%str3, var%str4) +deallocate(var%e, var%f, var%str3, var%str4, var%uni3, var%uni4) end subroutine one ! Explicitly mapped – all and full arrays subroutine two() -type(t2) :: var, var2(4) -type(t2), pointer :: var3, var4(:) +type(t2) :: var print '(g0)', ' TESTCASE "two" ' @@ -76,14 +82,19 @@ contains b = 2, c = cmplx(-1.0_8, 2.0_8,kind=8), & d = [(-3*i, i = 1, 10)], & str1 = "abcde", & - str2 = ["12345", "67890", "ABCDE", "FGHIJ"]) + str2 = ["12345", "67890", "ABCDE", "FGHIJ"], & + uni1 = 4_"abcde", & + uni2 = [4_"12345", 4_"67890", 4_"ABCDE", 4_"FGHIJ"]) allocate (var%e, source=99) allocate (var%f, source=[22, 33, 44, 55]) allocate (var%str3, source="HelloWorld") allocate (var%str4, source=["Let's", "Go!!!"]) +allocate (var%uni3, source=4_"HelloWorld") +allocate (var%uni4, source=[4_"Let's", 4_"Go!!!"]) !$omp target map(tofrom: var%a, var%b, var%c, var%d, var%e, var%f, & -!$omp& var%str1, var%str2, var%str3, var%str4) +!$omp& var%str1, var%str2, var%str3, var%str4, & +!$omp& var%uni1, var%uni2, var%uni3, var%uni4) if (var%a /= 1) stop 1 if (var%b /= 2) stop 2 if (var%c%re /= -1.0_8 .or. var%c%im /= 2.0_8) stop 3 @@ -103,15 +114,24 @@ contains
Re: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
Hi Julian, Tobias!
On 2020-06-24T19:32:09+0200, Tobias Burnus wrote:
> (OpenMP 5 extends this a lot, but this is about OpenMP 4.5.
> It touches code which is also used by OpenACC's attach/detach.)
>
> @OpenACC/Julian: I think the character attach/detach for
> deferred-length strings does not work properly with OpenACC;
> I did not touch this code – but I think it needs some love.
Please file a PR.
> This code adds support for
>map(dt%comp, dt%comp2)
> where "comp" can be either a nonpointer, nonallocatable element
> scalar, array or array section. Or it can be a pointer - where
> character strings are one complication as for deferred-length
> ones, the length is stored in an extra DT component.
>
> While testing, I encountered two bugs, one relating to kind=4
> character string (patch pending review; PR95837)
> not part of testcase) and one related to deferred-length
> character strings (commented in the test case; larger issue;
> PR95868).
>
> Like always, some more tests/testcase probably would not harm.
>
> Regarding the patch:
>
> (a) openmp.c:
> This enabled component matching for 'map(' and
> piggybacks on the OpenACC code for the checks. I think that
> some additional checks might be useful – and I hope that no
> check is too strict.
> The "depend" clause was excluded as one otherwise gets a
> testsuite fails due to the is-contiguous check.
>
> (b) trans-openmp.c:
> - gfc_trans_omp_clauses now has a "bool openacc".
> - GOMP_MAP_ATTACH_DETACH is replaced by GOMP_MAP_ALWAYS_POINTER
> - For arrays, the mapping of the descriptor is squeezed before
>"node" which contains the data transfer (var.desc.data mapping
>followed by the always_pointer for the mapping).
>In this array case, the latter gets a pointless cast in order
>to prevent that for both var.desc and var.desc.data memory gets
>allocated in the struct.
>→ That's also the reason the big switch table is moved up.
> - For deferred-length strings, the string-length is in an extra
>struct element (derived-type component) and will be mapped in
>addition.
> - Bugs in the previous version:
>* gfc_trans_omp_array_section for "element == true", the size
> of a pointer instead of the size of the element was mapped.
>* For string variables (with constant length) the kind=4 was
> not properly handled.
>* Allocatable scalars were not handled – missing second clause
> for the always_pointer (and attach_detach, I assume)
I understand correctly that your remark "Bugs in the previous version"
translates to "bugs still existing on releases/gcc-10 branch for OpenACC
'attach'/'detach'"? Should we thus backport to releases/gcc-10 branch
this commit 102502e32ea4e8a75d6b252ba319d09d735d9aa7 "[OpenMP, Fortran]
Add structure/derived-type element mapping", and fixup commit
524862db444b6544c6dc87c5f06f351100ecf50d "Fix goacc/finalize-1.f tree
dump-scanning for -m32"?
Grüße
Thomas
> Comments, remarks, suggestions?
> Otherwise: OK for the trunk?
> [OpenMP, Fortran] Add structure/derived-type element mapping
>
> gcc/fortran/ChangeLog:
>
> * openmp.c (gfc_match_omp_clauses): Match also derived-type
> component refs in OMP_CLAUSE_MAP.
> (resolve_omp_clauses): Resolve those.
> * trans-openmp.c (gfc_trans_omp_array_section, gfc_trans_omp_clauses):
> Handle OpenMP structure-element mapping.
> (gfc_trans_oacc_construct, gfc_trans_oacc_executable_directive,
> (gfc_trans_oacc_combined_directive, gfc_trans_oacc_declare): Update
> add openacc=true in gfc_trans_omp_clauses call.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/goacc/finalize-1.f: Update dump scan pattern.
> * gfortran.dg/gomp/map-1.f90: Update dg-error.
> * gfortran.dg/gomp/map-2.f90: New test.
>
>
> libgomp/ChangeLog:
>
> * testsuite/libgomp.fortran/struct-elem-map-1.f90: New test.
>
> gcc/fortran/openmp.c | 5 +-
> gcc/fortran/trans-openmp.c | 332
> +++--
> gcc/testsuite/gfortran.dg/goacc/finalize-1.f | 4 +-
> gcc/testsuite/gfortran.dg/gomp/map-1.f90 | 35 +--
> gcc/testsuite/gfortran.dg/gomp/map-2.f90 | 6 +
> .../libgomp.fortran/struct-elem-map-1.f90 | 331
> 6 files changed, 595 insertions(+), 118 deletions(-)
>
> diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
> index e681903c7c2..7de2f6e1b1d 100644
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -1464,7 +1464,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const
> omp_mask mask,
> head = NULL;
> if (gfc_match_omp_variable_list ("", >lists[OMP_LIST_MAP],
> false, NULL, ,
> -true) == MATCH_YES)
> +true, true) == MATCH_YES)
> {
> gfc_omp_namelist *n;
> for (n =
[Patch, committed] [OpenMP, Fortran] Fix goacc/finalize-1.f tree dump-scanning for -m32 (was: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping)
As pointed out by Jakub, this fails with -m32 as the dump-scanning
assumed ptrdiff node = integer(kind=8).
Committed as obvious (and as pre-approved) to mainline + OG10 after testing.
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
commit 524862db444b6544c6dc87c5f06f351100ecf50d
Author: Tobias Burnus
Date: Tue Jul 14 16:28:37 2020 +0200
Fix goacc/finalize-1.f tree dump-scanning for -m32
gcc/testsuite/ChangeLog:
* gfortran.dg/goacc/finalize-1.f: Relax scan-tree-dump-times
pattern to work on 32bit-pointer systems.
diff --git a/gcc/testsuite/gfortran.dg/goacc/finalize-1.f b/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
index 74fa408082b..266ead35192 100644
--- a/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
+++ b/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
@@ -20,7 +20,7 @@
! { dg-final { scan-tree-dump-times "(?n)#pragma omp target oacc_enter_exit_data map\\(delete:del_f \\\[len: \[0-9\]+\\\]\\) finalize$" 1 "gimple" } }
!$ACC EXIT DATA FINALIZE DELETE (del_f_p(2:5))
-! { dg-final { scan-tree-dump-times "(?n)#pragma acc exit data map\\(release:\\*\\(c_char \\*\\) 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: \\(integer\\(kind=8\\)\\) parm\\.0\\.data - \\(integer\\(kind=8\\)\\) del_f_p\\.data\\\]\\) finalize;$" 1 "original" } }
+! { dg-final { scan-tree-dump-times "(?n)#pragma acc exit data map\\(release:\\*\\(c_char \\*\\) 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: \\(integer\\(kind=.\\)\\) parm\\.0\\.data - \\(integer\\(kind=.\\)\\) del_f_p\\.data\\\]\\) finalize;$" 1 "original" } }
! { dg-final { scan-tree-dump-times "(?n)#pragma omp target oacc_enter_exit_data map\\(delete:MEM\\\[\\(c_char \\*\\)\[^\\\]\]+\\\] \\\[len: \[^\\\]\]+\\\]\\) map\\(to:del_f_p \\\[pointer set, len: \[0-9\]+\\\]\\) map\\(alloc:del_f_p\\.data \\\[pointer assign, bias: \[^\\\]\]+\\\]\\) finalize$" 1 "gimple" } }
!$ACC EXIT DATA COPYOUT (cpo_r)
@@ -32,6 +32,6 @@
! { dg-final { scan-tree-dump-times "(?n)#pragma omp target oacc_enter_exit_data map\\(force_from:cpo_f \\\[len: \[0-9\]+\\\]\\) finalize$" 1 "gimple" } }
!$ACC EXIT DATA COPYOUT (cpo_f_p(4:10)) FINALIZE
-! { dg-final { scan-tree-dump-times "(?n)#pragma acc exit data map\\(from:\\*\\(c_char \\*\\) 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: \\(integer\\(kind=8\\)\\) parm\\.1\\.data - \\(integer\\(kind=8\\)\\) cpo_f_p\\.data\\\]\\) finalize;$" 1 "original" } }
+! { dg-final { scan-tree-dump-times "(?n)#pragma acc exit data map\\(from:\\*\\(c_char \\*\\) 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: \\(integer\\(kind=.\\)\\) parm\\.1\\.data - \\(integer\\(kind=.\\)\\) cpo_f_p\\.data\\\]\\) finalize;$" 1 "original" } }
! { dg-final { scan-tree-dump-times "(?n)#pragma omp target oacc_enter_exit_data map\\(force_from:MEM\\\[\\(c_char \\*\\)\[^\\\]\]+\\\] \\\[len: \[^\\\]\]+\\\]\\) map\\(to:cpo_f_p \\\[pointer set, len: \[0-9\]+\\\]\\) map\\(alloc:cpo_f_p\\.data \\\[pointer assign, bias: \[^\\\]\]+\\\]\\) finalize$" 1 "gimple" } }
END SUBROUTINE f
Re: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
On Wed, Jun 24, 2020 at 07:32:09PM +0200, Tobias Burnus wrote: > Comments, remarks, suggestions? > Otherwise: OK for the trunk? LGTM, thanks. > [OpenMP, Fortran] Add structure/derived-type element mapping > > gcc/fortran/ChangeLog: > > * openmp.c (gfc_match_omp_clauses): Match also derived-type > component refs in OMP_CLAUSE_MAP. > (resolve_omp_clauses): Resolve those. > * trans-openmp.c (gfc_trans_omp_array_section, gfc_trans_omp_clauses): > Handle OpenMP structure-element mapping. > (gfc_trans_oacc_construct, gfc_trans_oacc_executable_directive, > (gfc_trans_oacc_combined_directive, gfc_trans_oacc_declare): Update > add openacc=true in gfc_trans_omp_clauses call. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/goacc/finalize-1.f: Update dump scan pattern. > * gfortran.dg/gomp/map-1.f90: Update dg-error. > * gfortran.dg/gomp/map-2.f90: New test. > > > libgomp/ChangeLog: > > * testsuite/libgomp.fortran/struct-elem-map-1.f90: New test. Jakub
*PING* Re: [Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
*PING*
On 6/24/20 7:32 PM, Tobias Burnus wrote:
(OpenMP 5 extends this a lot, but this is about OpenMP 4.5.
It touches code which is also used by OpenACC's attach/detach.)
@OpenACC/Julian: I think the character attach/detach for
deferred-length strings does not work properly with OpenACC;
I did not touch this code – but I think it needs some love.
This code adds support for
map(dt%comp, dt%comp2)
where "comp" can be either a nonpointer, nonallocatable element
scalar, array or array section. Or it can be a pointer - where
character strings are one complication as for deferred-length
ones, the length is stored in an extra DT component.
While testing, I encountered two bugs, one relating to kind=4
character string (patch pending review; PR95837)
not part of testcase) and one related to deferred-length
character strings (commented in the test case; larger issue;
PR95868).
Like always, some more tests/testcase probably would not harm.
Regarding the patch:
(a) openmp.c:
This enabled component matching for 'map(' and
piggybacks on the OpenACC code for the checks. I think that
some additional checks might be useful – and I hope that no
check is too strict.
The "depend" clause was excluded as one otherwise gets a
testsuite fails due to the is-contiguous check.
(b) trans-openmp.c:
- gfc_trans_omp_clauses now has a "bool openacc".
- GOMP_MAP_ATTACH_DETACH is replaced by GOMP_MAP_ALWAYS_POINTER
- For arrays, the mapping of the descriptor is squeezed before
"node" which contains the data transfer (var.desc.data mapping
followed by the always_pointer for the mapping).
In this array case, the latter gets a pointless cast in order
to prevent that for both var.desc and var.desc.data memory gets
allocated in the struct.
→ That's also the reason the big switch table is moved up.
- For deferred-length strings, the string-length is in an extra
struct element (derived-type component) and will be mapped in
addition.
- Bugs in the previous version:
* gfc_trans_omp_array_section for "element == true", the size
of a pointer instead of the size of the element was mapped.
* For string variables (with constant length) the kind=4 was
not properly handled.
* Allocatable scalars were not handled – missing second clause
for the always_pointer (and attach_detach, I assume)
Comments, remarks, suggestions?
Otherwise: OK for the trunk?
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
[Patch] [OpenMP, Fortran] Add structure/derived-type element mapping
(OpenMP 5 extends this a lot, but this is about OpenMP 4.5.
It touches code which is also used by OpenACC's attach/detach.)
@OpenACC/Julian: I think the character attach/detach for
deferred-length strings does not work properly with OpenACC;
I did not touch this code – but I think it needs some love.
This code adds support for
map(dt%comp, dt%comp2)
where "comp" can be either a nonpointer, nonallocatable element
scalar, array or array section. Or it can be a pointer - where
character strings are one complication as for deferred-length
ones, the length is stored in an extra DT component.
While testing, I encountered two bugs, one relating to kind=4
character string (patch pending review; PR95837)
not part of testcase) and one related to deferred-length
character strings (commented in the test case; larger issue;
PR95868).
Like always, some more tests/testcase probably would not harm.
Regarding the patch:
(a) openmp.c:
This enabled component matching for 'map(' and
piggybacks on the OpenACC code for the checks. I think that
some additional checks might be useful – and I hope that no
check is too strict.
The "depend" clause was excluded as one otherwise gets a
testsuite fails due to the is-contiguous check.
(b) trans-openmp.c:
- gfc_trans_omp_clauses now has a "bool openacc".
- GOMP_MAP_ATTACH_DETACH is replaced by GOMP_MAP_ALWAYS_POINTER
- For arrays, the mapping of the descriptor is squeezed before
"node" which contains the data transfer (var.desc.data mapping
followed by the always_pointer for the mapping).
In this array case, the latter gets a pointless cast in order
to prevent that for both var.desc and var.desc.data memory gets
allocated in the struct.
→ That's also the reason the big switch table is moved up.
- For deferred-length strings, the string-length is in an extra
struct element (derived-type component) and will be mapped in
addition.
- Bugs in the previous version:
* gfc_trans_omp_array_section for "element == true", the size
of a pointer instead of the size of the element was mapped.
* For string variables (with constant length) the kind=4 was
not properly handled.
* Allocatable scalars were not handled – missing second clause
for the always_pointer (and attach_detach, I assume)
Comments, remarks, suggestions?
Otherwise: OK for the trunk?
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
[OpenMP, Fortran] Add structure/derived-type element mapping
gcc/fortran/ChangeLog:
* openmp.c (gfc_match_omp_clauses): Match also derived-type
component refs in OMP_CLAUSE_MAP.
(resolve_omp_clauses): Resolve those.
* trans-openmp.c (gfc_trans_omp_array_section, gfc_trans_omp_clauses):
Handle OpenMP structure-element mapping.
(gfc_trans_oacc_construct, gfc_trans_oacc_executable_directive,
(gfc_trans_oacc_combined_directive, gfc_trans_oacc_declare): Update
add openacc=true in gfc_trans_omp_clauses call.
gcc/testsuite/ChangeLog:
* gfortran.dg/goacc/finalize-1.f: Update dump scan pattern.
* gfortran.dg/gomp/map-1.f90: Update dg-error.
* gfortran.dg/gomp/map-2.f90: New test.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/struct-elem-map-1.f90: New test.
gcc/fortran/openmp.c | 5 +-
gcc/fortran/trans-openmp.c | 332 +++--
gcc/testsuite/gfortran.dg/goacc/finalize-1.f | 4 +-
gcc/testsuite/gfortran.dg/gomp/map-1.f90 | 35 +--
gcc/testsuite/gfortran.dg/gomp/map-2.f90 | 6 +
.../libgomp.fortran/struct-elem-map-1.f90 | 331
6 files changed, 595 insertions(+), 118 deletions(-)
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index e681903c7c2..7de2f6e1b1d 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -1464,7 +1464,7 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const omp_mask mask,
head = NULL;
if (gfc_match_omp_variable_list ("", >lists[OMP_LIST_MAP],
false, NULL, ,
- true) == MATCH_YES)
+ true, true) == MATCH_YES)
{
gfc_omp_namelist *n;
for (n = *head; n; n = n->next)
@@ -4553,7 +4553,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
/* Look through component refs to find last array
reference. */
- if (openacc && resolved)
+ if (resolved)
{
/* The "!$acc cache" directive allows rectangular
subarrays to be specified, with some restrictions
@@ -4563,6 +4563,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
arr(-n:n,-n:n) could be contiguous even if it looks
like it may not be. */
if (list != OMP_LIST_CACHE
+ && list != OMP_LIST_DEPEND
&& !gfc_is_simply_contiguous (n->expr, false, true)
&& gfc_is_not_contiguous (n->expr))
gfc_error ("Array is not
