Re: gfortran.dg/PR82376.f90: Avoid matching a file-path.
On Thu, 12 Aug 2021 00:09:21 +0200
Hans-Peter Nilsson via Fortran wrote:
> I had a file-path to sources with the substring "new" in it,
> and (only) this test regressed compared to results from
> another build without "new" in the name.
>
> The test does
> ! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> i.e. the contents of the tree-dump-file .original needs to match
> the undelimited string "new" exactly four times. Very brittle.
>
> In the dump-file, there are three lines with calls to new:
> D.908 = new ((integer(kind=4) *) data);
> integer(kind=4) * new (integer(kind=4) & data)
>static integer(kind=4) * new (integer(kind=4) &);
>
> But, there's also a line, which for me and cris-elf looked like:
> _gfortran_runtime_error_at (&"At line 46 of file
> /X/xyzzynewfrob/gcc/testsuite/gfortran.dg/PR82376.f90"[1]{lb: 1 sz: 1},
> &"Pointer actual argument \'new\' is not associated"[1]{lb: 1 sz: 1});
> The fourth match is obviously intended to match this line, but only
> with *one* match, whereas the path can as above yield another hit.
>
> With Tcl, the regexp for matching the " " *and* the "'"
> *and* the "\" gets a bit unsightly, so I suggest just
> matching the "new" calls, which according to the comment in
> the test is the key point. You can't have a file-path with
> spaces and parentheses in a gcc build. I'm also making use
> of {} rather than "" needing one level of quoting; the "\("
> is needed because the matched string is a regexp.
>
> Ok to commit?
A wordmatch would be \mnew\M but i agree that counting calls by
{\mnew (} is fine too.
I'd call it obvious, so i dare to approve it.
OK.
thanks!
>
> testsuite:
> * gfortran.dg/PR82376.f90: Robustify match.
> ---
> gcc/testsuite/gfortran.dg/PR82376.f90 | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gfortran.dg/PR82376.f90
> b/gcc/testsuite/gfortran.dg/PR82376.f90
> index 07143ab7e82e..b99779ce9d8a 100644
> --- a/gcc/testsuite/gfortran.dg/PR82376.f90
> +++ b/gcc/testsuite/gfortran.dg/PR82376.f90
> @@ -2,7 +2,8 @@
> ! { dg-options "-fdump-tree-original -fcheck=pointer" }
> !
> ! Test the fix for PR82376. The pointer check was doubling up the call
> -! to new. The fix reduces the count of 'new' from 5 to 4.
> +! to new. The fix reduces the count of 'new' from 5 to 4, or to 3, when
> +! counting only calls.
> !
> ! Contributed by José Rui Faustino de Sousa
> !
> @@ -56,4 +57,4 @@ contains
>end subroutine set
>
> end program main_p
> -! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> +! { dg-final { scan-tree-dump-times { new \(} 3 "original" } }
[Patch] OpenMP 5.1: Add proc-bind 'primary' support
The attached patch adds another (very) low-hanging fruit of OpenMP 5.1
to GCC, given that we already have one OpenMP 5.1 feature and another
one also related to 'master'/'masked' construct might be added soon.
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
OpenMP 5.1: Add proc-bind 'primary' support
In OpenMP 5.1 "master thread" was changed to "primary thread" and
the proc_bind clause and the OMP_PROC_BIND environment variable
now take 'primary' as argument as alias for 'master', while the
latter is deprecated.
This commit accepts 'primary' and adds the named constant
omp_proc_bind_primary and changes 'master thread' in the
documentation; however, given that not even OpenMP 5.0 is
fully supported, omp_display_env and the dumps currently
still output 'master' and there is no deprecation warning
when using the 'master' in the proc_bind clause.
gcc/c/ChangeLog:
* c-parser.c (c_parser_omp_clause_proc_bind): Accept
'primary' as alias for 'master'.
gcc/cp/ChangeLog:
* parser.c (cp_parser_omp_clause_proc_bind): Accept
'primary' as alias for 'master'.
gcc/fortran/ChangeLog:
* dump-parse-tree.c (show_omp_clauses): Add TODO comment to
change 'master' to 'primary' in proc_bind for OpenMP 5.1.
* intrinsic.texi (OMP_LIB): Mention OpenMP 5.1; add
omp_proc_bind_primary.
* openmp.c (gfc_match_omp_clauses): Accept
'primary' as alias for 'master'.
gcc/ChangeLog:
* tree-pretty-print.c (dump_omp_clause): Add TODO comment to
change 'master' to 'primary' in proc_bind for OpenMP 5.1.
libgomp/ChangeLog:
* env.c (parse_bind_var): Accept 'primary' as alias for
'master'.
(omp_display_env): Add TODO comment to
change 'master' to 'primary' in proc_bind for OpenMP 5.1.
* libgomp.texi: Change 'master thread' to 'primary thread'
in line with OpenMP 5.1.
(omp_get_proc_bind): Add omp_proc_bind_primary and note that
omp_proc_bind_master is an alias of it.
(OMP_PROC_BIND): Mention 'PRIMARY'.
* omp.h.in (__GOMP_DEPRECATED_5_1): Define.
(omp_proc_bind_primary): Add.
(omp_proc_bind_master): Deprecate for OpenMP 5.1.
* omp_lib.f90.in (omp_proc_bind_primary): Add.
(omp_proc_bind_master): Deprecate for OpenMP 5.1.
* omp_lib.h.in (omp_proc_bind_primary): Add.
* testsuite/libgomp.c/affinity-1.c: Check that
'primary' works and is identical to 'master'.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/pr61486-2.c: Duplicate one proc_bind(master)
testcase and test proc_bind(primary) instead.
* gfortran.dg/gomp/affinity-1.f90: Likewise.
gcc/c/c-parser.c | 7 --
gcc/cp/parser.c | 7 --
gcc/fortran/dump-parse-tree.c | 1 +
gcc/fortran/intrinsic.texi| 6 +++--
gcc/fortran/openmp.c | 5 -
gcc/testsuite/c-c++-common/gomp/pr61486-2.c | 13 +++
gcc/testsuite/gfortran.dg/gomp/affinity-1.f90 | 9
gcc/tree-pretty-print.c | 1 +
libgomp/env.c | 13 ++-
libgomp/libgomp.texi | 32 ++-
libgomp/omp.h.in | 10 -
libgomp/omp_lib.f90.in| 6 +
libgomp/omp_lib.h.in | 1 +
libgomp/testsuite/libgomp.c/affinity-1.c | 14
14 files changed, 92 insertions(+), 33 deletions(-)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index d24bfdb6719..53f8617ddaa 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -15959,7 +15959,8 @@ c_parser_omp_clause_dist_schedule (c_parser *parser, tree list)
proc_bind ( proc-bind-kind )
proc-bind-kind:
- master | close | spread */
+ primary | master | close | spread
+ where OpenMP 5.1 added 'primary' and deprecated the alias 'master'. */
static tree
c_parser_omp_clause_proc_bind (c_parser *parser, tree list)
@@ -15975,7 +15976,9 @@ c_parser_omp_clause_proc_bind (c_parser *parser, tree list)
if (c_parser_next_token_is (parser, CPP_NAME))
{
const char *p = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
- if (strcmp ("master", p) == 0)
+ if (strcmp ("primary", p) == 0)
+ kind = OMP_CLAUSE_PROC_BIND_MASTER;
+ else if (strcmp ("master", p) == 0)
kind = OMP_CLAUSE_PROC_BIND_MASTER;
else if (strcmp ("close", p) == 0)
kind = OMP_CLAUSE_PROC_BIND_CLOSE;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 74de52992bc..0a5ed95d37f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -39020,7 +39020,8 @@ cp_parser_omp_clause_dist_schedule (cp_parser *parser, tree list,
proc_bind ( proc-bind-kind )
proc-bind-kind:
- master | close | spread */
+ primary | master | close | spread
+ where OpenMP 5.1 added 'primary' and d
Re: [Patch] OpenMP 5.1: Add proc-bind 'primary' support
On Thu, Aug 12, 2021 at 12:52:17PM +0200, Tobias Burnus wrote:
> gcc/c/ChangeLog:
>
> * c-parser.c (c_parser_omp_clause_proc_bind): Accept
> 'primary' as alias for 'master'.
>
> gcc/cp/ChangeLog:
>
> * parser.c (cp_parser_omp_clause_proc_bind): Accept
> 'primary' as alias for 'master'.
>
> gcc/fortran/ChangeLog:
>
> * dump-parse-tree.c (show_omp_clauses): Add TODO comment to
> change 'master' to 'primary' in proc_bind for OpenMP 5.1.
> * intrinsic.texi (OMP_LIB): Mention OpenMP 5.1; add
> omp_proc_bind_primary.
> * openmp.c (gfc_match_omp_clauses): Accept
> 'primary' as alias for 'master'.
>
> gcc/ChangeLog:
>
> * tree-pretty-print.c (dump_omp_clause): Add TODO comment to
> change 'master' to 'primary' in proc_bind for OpenMP 5.1.
>
> libgomp/ChangeLog:
>
> * env.c (parse_bind_var): Accept 'primary' as alias for
> 'master'.
> (omp_display_env): Add TODO comment to
> change 'master' to 'primary' in proc_bind for OpenMP 5.1.
> * libgomp.texi: Change 'master thread' to 'primary thread'
> in line with OpenMP 5.1.
> (omp_get_proc_bind): Add omp_proc_bind_primary and note that
> omp_proc_bind_master is an alias of it.
> (OMP_PROC_BIND): Mention 'PRIMARY'.
> * omp.h.in (__GOMP_DEPRECATED_5_1): Define.
> (omp_proc_bind_primary): Add.
> (omp_proc_bind_master): Deprecate for OpenMP 5.1.
> * omp_lib.f90.in (omp_proc_bind_primary): Add.
> (omp_proc_bind_master): Deprecate for OpenMP 5.1.
> * omp_lib.h.in (omp_proc_bind_primary): Add.
> * testsuite/libgomp.c/affinity-1.c: Check that
> 'primary' works and is identical to 'master'.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/gomp/pr61486-2.c: Duplicate one proc_bind(master)
> testcase and test proc_bind(primary) instead.
> * gfortran.dg/gomp/affinity-1.f90: Likewise.
LGTM, some nits below.
> @@ -15975,7 +15976,9 @@ c_parser_omp_clause_proc_bind (c_parser *parser, tree
> list)
>if (c_parser_next_token_is (parser, CPP_NAME))
> {
>const char *p = IDENTIFIER_POINTER (c_parser_peek_token
> (parser)->value);
> - if (strcmp ("master", p) == 0)
> + if (strcmp ("primary", p) == 0)
> + kind = OMP_CLAUSE_PROC_BIND_MASTER;
> + else if (strcmp ("master", p) == 0)
Maybe in tree-core.h do:
- OMP_CLAUSE_PROC_BIND_MASTER = 2,
+ OMP_CLAUSE_PROC_BIND_PRIMARY = 2,
+ OMP_CLAUSE_PROC_BIND_MASTER = OMP_CLAUSE_PROC_BIND_PRIMARY,
and use OMP_CLAUSE_PROC_BIND_PRIMARY for the "primary" cases?
I'd keep tree-pretty-print.c as is though for now (so print master).
And in omp-expand we actually just count on those enumerators
matching the omp.h omp_proc_bind_* ones.
> @@ -39037,7 +39038,9 @@ cp_parser_omp_clause_proc_bind (cp_parser *parser,
> tree list,
>tree id = cp_lexer_peek_token (parser->lexer)->u.value;
>const char *p = IDENTIFIER_POINTER (id);
>
> - if (strcmp ("master", p) == 0)
> + if (strcmp ("primary", p) == 0)
> + kind = OMP_CLAUSE_PROC_BIND_MASTER;
> + else if (strcmp ("master", p) == 0)
Ditto.
> - if (gfc_match ("proc_bind ( master )") == MATCH_YES)
> + /* Primary is new and master is deprecated in OpenMP 5.1. */
> + if (gfc_match ("proc_bind ( primary )") == MATCH_YES)
> + c->proc_bind = OMP_PROC_BIND_MASTER;
> + else if (gfc_match ("proc_bind ( master )") == MATCH_YES)
Maybe here too with gfortran.h change?
> --- a/libgomp/omp_lib.f90.in
> +++ b/libgomp/omp_lib.f90.in
> @@ -48,6 +48,8 @@
> parameter :: omp_proc_bind_false = 0
> integer (omp_proc_bind_kind), &
> parameter :: omp_proc_bind_true = 1
> +integer (omp_proc_bind_kind), &
> + parameter :: omp_proc_bind_primary = 2
> integer (omp_proc_bind_kind), &
> parameter :: omp_proc_bind_master = 2
> integer (omp_proc_bind_kind), &
> @@ -670,6 +672,10 @@
>
> #if _OPENMP >= 201811
> !GCC$ ATTRIBUTES DEPRECATED :: omp_get_nested, omp_set_nested
> +#endif
> +
> +#if _OPENMP >= 202011
> +!GCC$ ATTRIBUTES DEPRECATED :: omp_proc_bind_master
> #endif
I must say I have no idea how this will work, but for omp_*_nested
it is like that already. I think the file is *.f90, not *.F90 and
so it isn't preprocessed (and is it compiled with -fopenmp at all)?
But let's deal with it incrementally.
Jakub
Re: gfortran.dg/PR82376.f90: Avoid matching a file-path.
> From: Bernhard Reutner-Fischer
> Date: Thu, 12 Aug 2021 09:03:50 +0200
> On Thu, 12 Aug 2021 00:09:21 +0200
> Hans-Peter Nilsson via Fortran wrote:
>
> > I had a file-path to sources with the substring "new" in it,
> > and (only) this test regressed compared to results from
> > another build without "new" in the name.
> >
> > The test does
> > ! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> > i.e. the contents of the tree-dump-file .original needs to match
> > the undelimited string "new" exactly four times. Very brittle.
> >
> > In the dump-file, there are three lines with calls to new:
> > D.908 = new ((integer(kind=4) *) data);
> > integer(kind=4) * new (integer(kind=4) & data)
> >static integer(kind=4) * new (integer(kind=4) &);
> >
> > But, there's also a line, which for me and cris-elf looked like:
> > _gfortran_runtime_error_at (&"At line 46 of file
> > /X/xyzzynewfrob/gcc/testsuite/gfortran.dg/PR82376.f90"[1]{lb: 1 sz: 1},
> > &"Pointer actual argument \'new\' is not associated"[1]{lb: 1 sz: 1});
> > The fourth match is obviously intended to match this line, but only
> > with *one* match, whereas the path can as above yield another hit.
> >
> > With Tcl, the regexp for matching the " " *and* the "'"
> > *and* the "\" gets a bit unsightly, so I suggest just
> > matching the "new" calls, which according to the comment in
> > the test is the key point. You can't have a file-path with
> > spaces and parentheses in a gcc build. I'm also making use
> > of {} rather than "" needing one level of quoting; the "\("
> > is needed because the matched string is a regexp.
> >
> > Ok to commit?
>
> A wordmatch would be \mnew\M but i agree that counting calls by
> {\mnew (} is fine too.
Not really; I guess I should have mentioned that I briefly
considered word-delimeters, but it'd match a subdirectory
named "new"; not to be unexpected in gcc builds. Matching
something that can't be in a file-path (of a gcc build) is
the only way to be sure (that I can think of).
> I'd call it obvious, so i dare to approve it.
> OK.
> thanks!
Thanks, but not coming from a testsuite or fortran
maintainer I'm not sure I can actually rely on that.
OTOH, damn the torpedoes. Committed.
> >
> > testsuite:
> > * gfortran.dg/PR82376.f90: Robustify match.
> > ---
> > gcc/testsuite/gfortran.dg/PR82376.f90 | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/testsuite/gfortran.dg/PR82376.f90
> > b/gcc/testsuite/gfortran.dg/PR82376.f90
> > index 07143ab7e82e..b99779ce9d8a 100644
> > --- a/gcc/testsuite/gfortran.dg/PR82376.f90
> > +++ b/gcc/testsuite/gfortran.dg/PR82376.f90
> > @@ -2,7 +2,8 @@
> > ! { dg-options "-fdump-tree-original -fcheck=pointer" }
> > !
> > ! Test the fix for PR82376. The pointer check was doubling up the call
> > -! to new. The fix reduces the count of 'new' from 5 to 4.
> > +! to new. The fix reduces the count of 'new' from 5 to 4, or to 3, when
> > +! counting only calls.
> > !
> > ! Contributed by José Rui Faustino de Sousa
> > !
> > @@ -56,4 +57,4 @@ contains
> >end subroutine set
> >
> > end program main_p
> > -! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> > +! { dg-final { scan-tree-dump-times { new \(} 3 "original" } }
>
Re: gfortran.dg/PR82376.f90: Avoid matching a file-path.
On 12.08.21 14:13, Hans-Peter Nilsson via Fortran wrote: I'd call it obvious, so i dare to approve it. OK. thanks! Thanks, but not coming from a testsuite or fortran maintainer I'm not sure I can actually rely on that. OTOH, damn the torpedoes. Committed. If it helps: A post-commit LGTM from my side. I think it can also be regarded as obvious.* Tobias *Albeit that reminds me of a professor in mathematics who told us that he tends to very carefully check those places where the (course, bachelor, master, PhD, ...) student writes: 'one then trivially obtains ...' - 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] OpenMP 5.1: Add proc-bind 'primary' support
I will commit the attached patch in a while – unless last-minute
comments or other issues show up.
On 12.08.21 13:31, Jakub Jelinek wrote:
LGTM, some nits below.
Maybe in tree-core.h do:
- OMP_CLAUSE_PROC_BIND_MASTER = 2,
+ OMP_CLAUSE_PROC_BIND_PRIMARY = 2,
+ OMP_CLAUSE_PROC_BIND_MASTER = OMP_CLAUSE_PROC_BIND_PRIMARY,
and use OMP_CLAUSE_PROC_BIND_PRIMARY for the "primary" cases?
I'd keep tree-pretty-print.c as is though for now (so print master).
And in omp-expand we actually just count on those enumerators
matching the omp.h omp_proc_bind_* ones.
Did so (same enum value for primary and master) but ...
- if (gfc_match ("proc_bind ( master )") == MATCH_YES)
+ /* Primary is new and master is deprecated in OpenMP 5.1. */
+ if (gfc_match ("proc_bind ( primary )") == MATCH_YES)
+c->proc_bind = OMP_PROC_BIND_MASTER;
+ else if (gfc_match ("proc_bind ( master )") == MATCH_YES)
Maybe here too with gfortran.h change?
... used for the gfortran-internal enum a disjunct enum value.
--- a/libgomp/omp_lib.f90.in
...
+#if _OPENMP >= 202011
+!GCC$ ATTRIBUTES DEPRECATED :: omp_proc_bind_master
#endif
... I think the file is *.f90, not *.F90 and so it isn't preprocessed ...
Turned out that the file is compiled with '-cpp -fopenmp -fsyntax-only'.
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
OpenMP 5.1: Add proc-bind 'primary' support
In OpenMP 5.1 "master thread" was changed to "primary thread" and
the proc_bind clause and the OMP_PROC_BIND environment variable
now take 'primary' as argument as alias for 'master', while the
latter is deprecated.
This commit accepts 'primary' and adds the named constant
omp_proc_bind_primary and changes 'master thread' in the
documentation; however, given that not even OpenMP 5.0 is
fully supported, omp_display_env and the dumps currently
still output 'master' and there is no deprecation warning
when using the 'master' in the proc_bind clause.
gcc/c/ChangeLog:
* c-parser.c (c_parser_omp_clause_proc_bind): Accept
'primary' as alias for 'master'.
gcc/cp/ChangeLog:
* parser.c (cp_parser_omp_clause_proc_bind): Accept
'primary' as alias for 'master'.
gcc/fortran/ChangeLog:
* gfortran.h (gfc_omp_proc_bind_kind): Add OMP_PROC_BIND_PRIMARY.
* dump-parse-tree.c (show_omp_clauses): Add TODO comment to
change 'master' to 'primary' in proc_bind for OpenMP 5.1.
* intrinsic.texi (OMP_LIB): Mention OpenMP 5.1; add
omp_proc_bind_primary.
* openmp.c (gfc_match_omp_clauses): Accept
'primary' as alias for 'master'.
gcc/ChangeLog:
* tree-core.h (omp_clause_proc_bind_kind): Add
OMP_CLAUSE_PROC_BIND_PRIMARY.
* tree-pretty-print.c (dump_omp_clause): Add TODO comment to
change 'master' to 'primary' in proc_bind for OpenMP 5.1.
libgomp/ChangeLog:
* env.c (parse_bind_var): Accept 'primary' as alias for
'master'.
(omp_display_env): Add TODO comment to
change 'master' to 'primary' in proc_bind for OpenMP 5.1.
* libgomp.texi: Change 'master thread' to 'primary thread'
in line with OpenMP 5.1.
(omp_get_proc_bind): Add omp_proc_bind_primary and note that
omp_proc_bind_master is an alias of it.
(OMP_PROC_BIND): Mention 'PRIMARY'.
* omp.h.in (__GOMP_DEPRECATED_5_1): Define.
(omp_proc_bind_primary): Add.
(omp_proc_bind_master): Deprecate for OpenMP 5.1.
* omp_lib.f90.in (omp_proc_bind_primary): Add.
(omp_proc_bind_master): Deprecate for OpenMP 5.1.
* omp_lib.h.in (omp_proc_bind_primary): Add.
* testsuite/libgomp.c/affinity-1.c: Check that
'primary' works and is identical to 'master'.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/pr61486-2.c: Duplicate one proc_bind(master)
testcase and test proc_bind(primary) instead.
* gfortran.dg/gomp/affinity-1.f90: Likewise.
gcc/c/c-parser.c | 7 --
gcc/cp/parser.c | 7 --
gcc/fortran/dump-parse-tree.c | 1 +
gcc/fortran/gfortran.h| 1 +
gcc/fortran/intrinsic.texi| 6 +++--
gcc/fortran/openmp.c | 5 -
gcc/fortran/trans-openmp.c| 3 +++
gcc/testsuite/c-c++-common/gomp/pr61486-2.c | 13 +++
gcc/testsuite/gfortran.dg/gomp/affinity-1.f90 | 9
gcc/tree-core.h | 1 +
gcc/tree-pretty-print.c | 2 ++
libgomp/env.c | 13 ++-
libgomp/libgomp.texi | 32 ++-
libgomp/omp.h.in | 10 -
libgomp/omp_lib.f90.in| 6 +
libgomp/omp_lib.h.in | 2 ++
libgomp/testsuite/libgomp.c/affinity-1.c | 14
17 files changed, 99 insertions(+)
