From: Tobias Burnus <tobias@codesourcery.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>,
fortran <fortran@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>
Subject: [Patch] Fortran/OpenMP: Fix strictly structured blocks parsing
Date: Wed, 24 Aug 2022 19:47:50 +0200 [thread overview]
Message-ID: <df662940-94b2-1b68-9884-b75cde55d385@codesourcery.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]
This patch is about error diagnostic + an ICE for invalid code.
Before the patch, gfortran/f951 showed:
...
Error: 'ancestor' device modifier not preceded by 'requires' directive with 'reverse_offload' clause
18 | end block
| 1
Error: Expecting END PROGRAM statement at (1)
gfortran: internal compiler error: Segmentation fault signal terminated program f951
* The first error is for a user error and fine.
* A follow-up error is expected (due to the now dangling '!$omp end target'),
but the error location and wording is misleading
* And the ICE is plainly wrong.
With the patch, there is no ICE and the the second error reads:
16 | !$omp end target
Error: Unexpected !$OMP END TARGET statement at (1)
The problem was due to nesting '!$omp target' (= same directive),
where the outer one was a strictly structured block - and parsing
the inner '!$omp target' failed with MATCH_ERROR, which ignored that
line - while '!$omp end target' remained.
(So far, so good, but then the parsing-code did run into a bug.)
For the blocks, the following applies. OpenMP permits either
* strictly structured blocks (with optional END_ST == 'end target')
!$omp target
block
...
end block
!$omp end target ! << this line is optional
* loosely structured block
!$omp target
... ! may not start with 'block' (and hence cannot end with 'end block')
!$omp end target ! << required
The parsing issue is in the following code,
which first takes care of the 'strictly': 'end block' + optional 'end target'
and then of the 'loosely structured' case with just: 'end target':
else if (block_construct && st == ST_END_BLOCK)
...
st = next_statement ();
if (st == omp_end_st)
accept_statement (st);
...
else if (st != omp_end_st)
{
unexpected_statement (st);
st = next_statement ();
}
The fix is to change the second if condition to:
else if (st != omp_end_st || (block_construct && st == omp_end_st))
or rather to the following equivalent code:
else if (st != omp_end_st || block_construct)
OK for mainline and GCC 12?*
Tobias
*strictly structured blocks were added in r12-4592.
-----------------
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
[-- Attachment #2: fix-strictly-struct-block.diff --]
[-- Type: text/x-patch, Size: 1770 bytes --]
Fortran/OpenMP: Fix strictly structured blocks parsing
gcc/fortran/ChangeLog:
* parse.cc (parse_omp_structured_block): When parsin strictly
structured blocks, issue an error if the end-directive comes
before the 'end block'.
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/strictly-structured-block-4.f90: New test.
gcc/fortran/parse.cc | 2 +-
.../gomp/strictly-structured-block-4.f90 | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 0b4c596996c..80492c952aa 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -5709,7 +5709,7 @@ parse_omp_structured_block (gfc_statement omp_st, bool workshare_stmts_only)
}
return st;
}
- else if (st != omp_end_st)
+ else if (st != omp_end_st || block_construct)
{
unexpected_statement (st);
st = next_statement ();
diff --git a/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-4.f90 b/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-4.f90
new file mode 100644
index 00000000000..66cf0a3925e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-4.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+implicit none
+integer ::x,z
+x = 42
+print '(*(z16:" "))', loc(x)
+!$omp target map(x, z)
+block
+ integer :: y
+ x = 123
+ y = 99
+ !$omp target device(ancestor:1) map(always,tofrom:x) map(y) ! { dg-error "'ancestor' device modifier not preceded by 'requires' directive with 'reverse_offload' clause" }
+ print '(*(z16:" "))', loc(x), loc(y)
+ print * ,x, y
+ x = -x
+ y = -y
+ !$omp end target ! { dg-error "Unexpected ..OMP END TARGET statement" }
+ z = y
+end block
+ print * ,x !, z
+end
+
WARNING: multiple messages have this Message-ID
From: Tobias Burnus <tobias@codesourcery.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>,
fortran <fortran@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>
Subject: [Patch] Fortran/OpenMP: Fix strictly structured blocks parsing
Date: Wed, 24 Aug 2022 19:47:50 +0200 [thread overview]
Message-ID: <df662940-94b2-1b68-9884-b75cde55d385@codesourcery.com> (raw)
Message-ID: <20220824174750.ga6P-cgqvMYml90ov7pVcptN3-BywNES4pPyVPrJ4w4@z> (raw)
[-- Attachment #1.1: Type: text/plain, Size: 2576 bytes --]
This patch is about error diagnostic + an ICE for invalid code.
Before the patch, gfortran/f951 showed:
...
Error: 'ancestor' device modifier not preceded by 'requires' directive with 'reverse_offload' clause
18 | end block
| 1
Error: Expecting END PROGRAM statement at (1)
gfortran: internal compiler error: Segmentation fault signal terminated program f951
* The first error is for a user error and fine.
* A follow-up error is expected (due to the now dangling '!$omp end target'),
but the error location and wording is misleading
* And the ICE is plainly wrong.
With the patch, there is no ICE and the the second error reads:
16 | !$omp end target
Error: Unexpected !$OMP END TARGET statement at (1)
The problem was due to nesting '!$omp target' (= same directive),
where the outer one was a strictly structured block - and parsing
the inner '!$omp target' failed with MATCH_ERROR, which ignored that
line - while '!$omp end target' remained.
(So far, so good, but then the parsing-code did run into a bug.)
For the blocks, the following applies. OpenMP permits either
* strictly structured blocks (with optional END_ST == 'end target')
!$omp target
block
...
end block
!$omp end target ! << this line is optional
* loosely structured block
!$omp target
... ! may not start with 'block' (and hence cannot end with 'end block')
!$omp end target ! << required
The parsing issue is in the following code,
which first takes care of the 'strictly': 'end block' + optional 'end target'
and then of the 'loosely structured' case with just: 'end target':
else if (block_construct && st == ST_END_BLOCK)
...
st = next_statement ();
if (st == omp_end_st)
accept_statement (st);
...
else if (st != omp_end_st)
{
unexpected_statement (st);
st = next_statement ();
}
The fix is to change the second if condition to:
else if (st != omp_end_st || (block_construct && st == omp_end_st))
or rather to the following equivalent code:
else if (st != omp_end_st || block_construct)
OK for mainline and GCC 12?*
Tobias
*strictly structured blocks were added in r12-4592.
-----------------
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
[-- Attachment #2: fix-strictly-struct-block.diff --]
[-- Type: text/x-patch, Size: 1770 bytes --]
Fortran/OpenMP: Fix strictly structured blocks parsing
gcc/fortran/ChangeLog:
* parse.cc (parse_omp_structured_block): When parsin strictly
structured blocks, issue an error if the end-directive comes
before the 'end block'.
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/strictly-structured-block-4.f90: New test.
gcc/fortran/parse.cc | 2 +-
.../gomp/strictly-structured-block-4.f90 | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 0b4c596996c..80492c952aa 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -5709,7 +5709,7 @@ parse_omp_structured_block (gfc_statement omp_st, bool workshare_stmts_only)
}
return st;
}
- else if (st != omp_end_st)
+ else if (st != omp_end_st || block_construct)
{
unexpected_statement (st);
st = next_statement ();
diff --git a/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-4.f90 b/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-4.f90
new file mode 100644
index 00000000000..66cf0a3925e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/strictly-structured-block-4.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+implicit none
+integer ::x,z
+x = 42
+print '(*(z16:" "))', loc(x)
+!$omp target map(x, z)
+block
+ integer :: y
+ x = 123
+ y = 99
+ !$omp target device(ancestor:1) map(always,tofrom:x) map(y) ! { dg-error "'ancestor' device modifier not preceded by 'requires' directive with 'reverse_offload' clause" }
+ print '(*(z16:" "))', loc(x), loc(y)
+ print * ,x, y
+ x = -x
+ y = -y
+ !$omp end target ! { dg-error "Unexpected ..OMP END TARGET statement" }
+ z = y
+end block
+ print * ,x !, z
+end
+
next reply other threads:[~2022-08-24 17:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-24 17:47 Tobias Burnus [this message]
2022-08-24 17:47 ` Tobias Burnus
2022-08-24 18:50 ` Harald Anlauf
2022-08-24 18:50 ` Harald Anlauf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=df662940-94b2-1b68-9884-b75cde55d385@codesourcery.com \
--to=tobias@codesourcery.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).