Re: [Patch] Fortran: OpenMP/OpenACC avoid uninit access in size calc for mapping
Hi Tobias!
On 2022-03-08T15:25:07+0100, Tobias Burnus wrote:
> found when working on the deep-mapping patch* with OpenMP code
> (and part of that patch) but it already shows up in an existing
> OpenACC testcase. I think it makes sense to fix it already for GCC 12.
>
> Problem: Also for unallocated allocatables, their size was
> calculated - the 'if(desc.data == NULL)' check was only added
> for pointers.
>
> Result after the patch: When compiling with -O (which is the default
> for goacc.exp), the warning now disappears. Thus, I now use '-O0'
> and the previous "is uninitialized" is now "may be uninitialized".
I recently added that checking in
commit 4bd8b1e881f0c26a5103cd1919809b3d63b60ef2
"Document current '-Wuninitialized'/'-Wmaybe-uninitialized' diagnostics
for OpenACC test cases", to document the status quo.
I'll leave it to you to decide what is more appropriate: (1), as you have
proposed, add '-O0' (but with source code comment, please); something
like:
! { dg-additional-options -Wuninitialized }
+! Trigger "may be used uninitialized".
+! { dg-additional-options -O0 }
..., or (2): update the test cases to simply reflect diagnostics that are
now (no longer) seen with (default) '-O' (rationale: the test cases
haven't originally been written for the '-Wuninitialized' diagnostics;
that's just tested additionally, and using '-O0' instead of '-O' may be
disturbing what they originally meant to test?), or (3): duplicate the
test cases to account for both (1) and (2) (in other words: write
dedicated test cases for your GCC/Fortran front end changes (for example,
based on the ones you've modified here), and for the existing test cases
apply (2)). The latter, (3), would be my approach.
> Unrelated to the patch and the testcase, I added some
> 'allocate'**/'if(allocated())' to the testcase - as otherwise
> uninit vars would be accessed. (Not relevant for the warning
> or the patch - but I prefer no invalid code in testcases,
> if it can be avoided.)
Agreed in principle, but again: I don't know what these test cases
originally have been testing?
> OK for mainline?
I can't comment on the GCC/Fortran front end changes -- so unless
somebody else speaks up, that's an implicit approval for those, I
suppose. ;-)
> Tobias
> * https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591144.html
> ** I am actually not sure whether 'acc update(b)' will/should map a
> previous allocated variable - or whether it should.
(Are the typos here: in "will/should map": 's%map%update', and in
"or whether it should": 's%should%shouldn't'?)
> But that's
> unrelated to this bug fix. See also: https://gcc.gnu.org/PR96668
> for the re-mapping in OpenMP (works for arrays but not scalars).
I don't quickly dig that, sorry. Do we need to first clarify that with
OpenACC Technical Committee, or is this just a GCC/OpenACC implementation
issue?
Grüße
Thomas
> Fortran: OpenMP/OpenACC avoid uninit access in size calc for mapping
>
> gcc/fortran/ChangeLog:
>
> * trans-openmp.cc (gfc_trans_omp_clauses, gfc_omp_finish_clause):
> Obtain size for mapping only if allocatable array is allocated.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/goacc/array-with-dt-1.f90: Run with -O0 and
> update dg-warning.
> * gfortran.dg/goacc/pr93464.f90: Likewise.
>
> gcc/fortran/trans-openmp.cc | 6 --
> gcc/testsuite/gfortran.dg/goacc/array-with-dt-1.f90 | 12 +---
> gcc/testsuite/gfortran.dg/goacc/pr93464.f90 | 8
> 3 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
> index 4d56a771349..fad76a4791f 100644
> --- a/gcc/fortran/trans-openmp.cc
> +++ b/gcc/fortran/trans-openmp.cc
> @@ -1597,7 +1597,8 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p, bool
> openacc)
>tree size = create_tmp_var (gfc_array_index_type);
>tree elemsz = TYPE_SIZE_UNIT (gfc_get_element_type (type));
>elemsz = fold_convert (gfc_array_index_type, elemsz);
> - if (GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER
> + if (GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_ALLOCATABLE
> + || GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER
> || GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER_CONT)
> {
> stmtblock_t cond_block;
> @@ -3208,7 +3209,8 @@ gfc_trans_omp_clauses (stmtblock_t *block,
> gfc_omp_clauses *clauses,
>
> /* We have to check for n->sym->attr.dimension because
>of scalar coarrays. */
> - if (n->sym->attr.pointer && n->sym->attr.dimension)
> + if ((n->sym->attr.pointer || n->sym->attr.allocatable)
> + && n->sym->attr.dimension)
> {
> stmtblock_t cond_block;
> tree size
> diff --git a/gcc/testsuite/gfortran.dg/goacc/array-with-dt-1.f90
> b/gcc/testsuite/gfortran.dg/
Re: [Patch] Fortran: OpenMP/OpenACC avoid uninit access in size calc for mapping
Hi Thomas, hi all,
(Updated patch attached – only changes the goacc testcases.
I intent to commit the patch tomorrow, unless more comments come up.)
On 10.03.22 10:00, Thomas Schwinge wrote:
[OpenACC testcases:]
I recently added that checking in ... [...] to document the status quo.
(1), as you have proposed, add '-O0' (but with source code comment, please);
[...]
..., or (2): update the test cases [...] (rationale: the test cases
haven't originally been written for the '-Wuninitialized' diagnostics; [...]
(3): duplicate the test cases to account for both (1) and (2)
[...] (3), would be my approach.
Attached patch does (3). I also remove the code tweaking, added in previous
patch. - But added a bunch of comments.
And I have to admit that I did not realize that the -Wuninitialized was only
added later. (I did not expect that new flags get added to existing patches.)
** I am actually not sure whether 'acc update(b)' will/should map a
previous allocated variable - or whether it should. [...]
testcases.
Should be: "previously *un*allocated" (+ Thomas s%...%.. comments).
I don't quickly dig that, sorry. Do we need to first clarify that with
OpenACC Technical Committee, or is this just a GCC/OpenACC implementation
issue?
That's mostly an OpenACC spec question.
But I did not check what the spec says. Thus, I don't know
* whether the spec needs to be improved
* what the implications are on the implementation.
I assume that the implementation for OpenMP does also make sense for
OpenACC (i.e. either works as required or does more but in a sensible
way) - but I don't know.
I think either/both of us should check the OpenACC spec.
* * *
On the OpenMP side (clarified in 5.1 or 5.2):
* Map first an unallocated allocatable
=> That one is regarded as mapped/present
* Allocate it and map it again (e.g. implicit/explicit
mapping for a target region)
=> Spec: Implementation may or may not update the item.
However, (only) with 'always' the spec guarantees that it
will also show up as allocated on the device.
=> Sentiment: should also work without 'always' modifier
if previously unallocated. (Discussion postponed.)
* I have not checked 'update' but I think it behaves
like 'map(always,to/from:' (except for ref counting)
OpenMP GCC implementation: there is a known issue for
scalar allocatables (PR96668). It does work for arrays
(also without 'always') - and 'omp update' has not been
checked. (Ref counting issues?)
OpenACC GCC implementation: I think this code is shared
with OpenMP and, thus, works likewise. But I have have
also not checked this.
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/OpenACC avoid uninit access in size calc for mapping
gcc/fortran/ChangeLog:
* trans-openmp.cc (gfc_trans_omp_clauses, gfc_omp_finish_clause):
Obtain size for mapping only if allocatable array is allocated.
gcc/testsuite/ChangeLog:
* gfortran.dg/goacc/array-with-dt-1.f90: Update/add comments;
remove dg-warning for 'is used uninitialized'.
* gfortran.dg/goacc/pr93464.f90: Likewise.
* gfortran.dg/goacc/array-with-dt-1a.f90: New; copied from
gfortran.dg/goacc/array-with-dt-1.f90 but run with -O0. Update
dg-warning for 'may be used uninitialized'.
* gfortran.dg/goacc/pr93464-2.f90: Likewise; copied from
gfortran.dg/goacc/pr93464.f90.
gcc/fortran/trans-openmp.cc| 6 +++--
.../gfortran.dg/goacc/array-with-dt-1.f90 | 18 ---
.../gfortran.dg/goacc/array-with-dt-1a.f90 | 27 ++
gcc/testsuite/gfortran.dg/goacc/pr93464-2.f90 | 26 +
gcc/testsuite/gfortran.dg/goacc/pr93464.f90| 12 ++
5 files changed, 80 insertions(+), 9 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 4d56a771349..fad76a4791f 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -1597,7 +1597,8 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p, bool openacc)
tree size = create_tmp_var (gfc_array_index_type);
tree elemsz = TYPE_SIZE_UNIT (gfc_get_element_type (type));
elemsz = fold_convert (gfc_array_index_type, elemsz);
- if (GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER
+ if (GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_ALLOCATABLE
+ || GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER
|| GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER_CONT)
{
stmtblock_t cond_block;
@@ -3208,7 +3209,8 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
/* We have to check for n->sym->attr.dimension because
of scalar coarrays. */
- if (n->sym->attr.pointer && n->sym->attr.dimension)
+ if ((n->sym->attr.pointer || n->sym->attr.allocatable)
+
Re: [PATCH] OpenMP, libgomp: Add new runtime routine omp_get_mapped_ptr.
Hi Jakub,
This is an update to the patch from Tue Mar 8:
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591343.html
I just added "get_mapped_ptr" to the "omp_runtime_apis" array in omp-low.cc and
replaced "omp_get_num_devices" by "gomp_get_num_devices" in target.c.
Marcel
-
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
OpenMP, libgomp: Add new runtime routine omp_get_mapped_ptr.
gcc/ChangeLog:
* omp-low.cc (omp_runtime_api_call): Added get_mapped_ptr to
omp_runtime_apis array.
libgomp/ChangeLog:
* libgomp.map: Added omp_get_mapped_ptr.
* libgomp.texi: Tagged omp_get_mapped_ptr as supported.
* omp.h.in: Added omp_get_mapped_ptr.
* omp_lib.f90.in: Added interface for omp_get_mapped_ptr.
* omp_lib.h.in: Likewise.
* target.c (omp_get_mapped_ptr): Added implementation of
omp_get_mapped_ptr.
* testsuite/libgomp.c-c++-common/get-mapped-ptr-1.c: New test.
* testsuite/libgomp.c-c++-common/get-mapped-ptr-2.c: New test.
* testsuite/libgomp.c-c++-common/get-mapped-ptr-3.c: New test.
* testsuite/libgomp.c-c++-common/get-mapped-ptr-4.c: New test.
* testsuite/libgomp.fortran/get-mapped-ptr-1.f90: New test.
* testsuite/libgomp.fortran/get-mapped-ptr-2.f90: New test.
* testsuite/libgomp.fortran/get-mapped-ptr-3.f90: New test.
* testsuite/libgomp.fortran/get-mapped-ptr-4.f90: New test.
diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index 77176ef..02a0f72 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -3962,6 +3962,7 @@ omp_runtime_api_call (const_tree fndecl)
"target_is_present",
"target_memcpy",
"target_memcpy_rect",
+ "get_mapped_ptr",
NULL,
/* Now omp_* calls that are available as omp_* and omp_*_; however, the
DECL_NAME is always omp_* without tailing underscore. */
diff --git a/libgomp/libgomp.map b/libgomp/libgomp.map
index 2ac5809..608a54c 100644
--- a/libgomp/libgomp.map
+++ b/libgomp/libgomp.map
@@ -226,6 +226,11 @@ OMP_5.1 {
omp_get_teams_thread_limit_;
} OMP_5.0.2;
+OMP_5.1.1 {
+ global:
+ omp_get_mapped_ptr;
+} OMP_5.1;
+
GOMP_1.0 {
global:
GOMP_atomic_end;
diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index 161a423..c163b56 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -314,7 +314,7 @@ The OpenMP 4.5 specification is fully supported.
@item @code{omp_target_is_accessible} runtime routine @tab N @tab
@item @code{omp_target_memcpy_async} and @code{omp_target_memcpy_rect_async}
runtime routines @tab N @tab
-@item @code{omp_get_mapped_ptr} runtime routine @tab N @tab
+@item @code{omp_get_mapped_ptr} runtime routine @tab Y @tab
@item @code{omp_calloc}, @code{omp_realloc}, @code{omp_aligned_alloc} and
@code{omp_aligned_calloc} runtime routines @tab Y @tab
@item @code{omp_alloctrait_key_t} enum: @code{omp_atv_serialized} added,
diff --git a/libgomp/omp.h.in b/libgomp/omp.h.in
index 89c5d65..18d0152 100644
--- a/libgomp/omp.h.in
+++ b/libgomp/omp.h.in
@@ -282,6 +282,7 @@ extern int omp_target_memcpy_rect (void *, const void *,
__SIZE_TYPE__, int,
extern int omp_target_associate_ptr (const void *, const void *, __SIZE_TYPE__,
__SIZE_TYPE__, int) __GOMP_NOTHROW;
extern int omp_target_disassociate_ptr (const void *, int) __GOMP_NOTHROW;
+extern void *omp_get_mapped_ptr (const void *, int) __GOMP_NOTHROW;
extern void omp_set_affinity_format (const char *) __GOMP_NOTHROW;
extern __SIZE_TYPE__ omp_get_affinity_format (char *, __SIZE_TYPE__)
diff --git a/libgomp/omp_lib.f90.in b/libgomp/omp_lib.f90.in
index daf40dc..506f15c 100644
--- a/libgomp/omp_lib.f90.in
+++ b/libgomp/omp_lib.f90.in
@@ -835,6 +835,15 @@
end function omp_target_disassociate_ptr
end interface
+interface
+ function omp_get_mapped_ptr (ptr, device_num) bind(c)
+use, intrinsic :: iso_c_binding, only : c_ptr, c_int
+type(c_ptr) :: omp_get_mapped_ptr
+type(c_ptr), value :: ptr
+integer(c_int), value :: device_num
+ end function omp_get_mapped_ptr
+end interface
+
#if _OPENMP >= 201811
!GCC$ ATTRIBUTES DEPRECATED :: omp_get_nested, omp_set_nested
#endif
diff --git a/libgomp/omp_lib.h.in b/libgomp/omp_lib.h.in
index ff857a4..0f48510 100644
--- a/libgomp/omp_lib.h.in
+++ b/libgomp/omp_lib.h.in
@@ -416,3 +416,12 @@
integer(c_int), value :: device_num
end function omp_target_disassociate_ptr
end interface
+
+ interface
+function omp_get_mapped_ptr (ptr, device_num) bind(c)
+ use, intrinsic :: iso_c_binding, only : c_ptr, c_int
+ type(c_ptr) :: omp_get_mapped_ptr
+ t
Re: GCC GSoC 2022: Call for project ideas and mentors
I assume it’s too late for new project ideas. I’m adding Milan Curcic in cc to confirm. Damian On Wed, Mar 9, 2022 at 18:09 Jerry D via Fortran wrote: > Perhaps someone could work on completing and merging the shared memory > (native) fortran coarrays branch. > > Regards, > > Jerry > > On 3/9/22 6:39 AM, Martin Jambor wrote: > > Hello, > > > > I am pleased that I can announce that GCC has been accepted as a > > mentoring organization of Google Summer of Code 2022. > > > > Contributors(*) will be applying from April 4th to April 19th but have > > already seen some announcing their intention to apply and asking for > > guidance when selecting a project and preparing their applications. > > Please continue helping them figure stuff out about GCC like you always > > do. > > > > If anyone still wants to add a project to our idea list (and sign up to > > be a potential mentor), now is the time to do it. > > > > I'm looking forward to another year of interesting projects, > > > > Martin > > > > > > > > (*) Contributors no longer have to be students - they should > > however be "new or beginner" contributors to our project with the > > exception that participants of GSoC 2020 or GSoC 2021 can apply. > > > > More on changes this year is in my original call for projects: > > https://gcc.gnu.org/pipermail/gcc/2022-January/238006.html > > > >
Re: [PATCH] OpenMP, libgomp: Add new runtime routine omp_get_mapped_ptr.
On Thu, Mar 10, 2022 at 05:01:35PM +0100, Marcel Vollweiler wrote: > --- a/gcc/omp-low.cc > +++ b/gcc/omp-low.cc > @@ -3962,6 +3962,7 @@ omp_runtime_api_call (const_tree fndecl) >"target_is_present", >"target_memcpy", >"target_memcpy_rect", > + "get_mapped_ptr", >NULL, >/* Now omp_* calls that are available as omp_* and omp_*_; however, the >DECL_NAME is always omp_* without tailing underscore. */ The entries in each NULL separated subsection are supposed to be sorted alphabetically. Other than that LGTM, but stage1 is still far... Jakub
Re: GCC GSoC 2022: Call for project ideas and mentors
Hi, On Thu, Mar 10 2022, Damian Rouson wrote: > I assume it’s too late for new project ideas. I’m adding Milan Curcic in > cc to confirm. it is not too late but people are already looking at the idea page, so the longer you wait, the more potential contributers you'll miss. Martin > > Damian > > On Wed, Mar 9, 2022 at 18:09 Jerry D via Fortran > wrote: > >> Perhaps someone could work on completing and merging the shared memory >> (native) fortran coarrays branch. >> >> Regards, >> >> Jerry >> >> On 3/9/22 6:39 AM, Martin Jambor wrote: >> > Hello, >> > >> > I am pleased that I can announce that GCC has been accepted as a >> > mentoring organization of Google Summer of Code 2022. >> > >> > Contributors(*) will be applying from April 4th to April 19th but have >> > already seen some announcing their intention to apply and asking for >> > guidance when selecting a project and preparing their applications. >> > Please continue helping them figure stuff out about GCC like you always >> > do. >> > >> > If anyone still wants to add a project to our idea list (and sign up to >> > be a potential mentor), now is the time to do it. >> > >> > I'm looking forward to another year of interesting projects, >> > >> > Martin >> > >> > >> > >> > (*) Contributors no longer have to be students - they should >> > however be "new or beginner" contributors to our project with the >> > exception that participants of GSoC 2020 or GSoC 2021 can apply. >> > >> > More on changes this year is in my original call for projects: >> > https://gcc.gnu.org/pipermail/gcc/2022-January/238006.html >> > >> >>
