public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* [OG12][PATCH] OpenMP: Fix ICE with OMP metadirectives
@ 2022-09-21 21:18 Paul-Antoine Arras
  2022-09-22  9:01 ` Tobias Burnus
  0 siblings, 1 reply; 4+ messages in thread
From: Paul-Antoine Arras @ 2022-09-21 21:18 UTC (permalink / raw)
  To: gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]

Hello,

Here is a patch that fixes an ICE in gfortran triggered by an invalid 
end statement at the end of an OMP metadirective:

```
     !$OMP metadirective ...
	...
     !$OMP end ...
```

Does this fix look correct?

Thanks,
-- 
Paul-Antoine Arras

[-- Attachment #2: 0001-OpenMP-Fix-ICE-with-OMP-metadirectives.patch --]
[-- Type: text/plain, Size: 3682 bytes --]

From 73ecbc2672a5352a08260f7a9d0de6d2c29ea2b6 Mon Sep 17 00:00:00 2001
From: Paul-Antoine Arras <pa@codesourcery.com>
Date: Wed, 21 Sep 2022 15:52:56 +0000
Subject: [PATCH] OpenMP: Fix ICE with OMP metadirectives

Problem: ending an OpenMP metadirective block with an OMP end statement
results in an internal compiler error.
Solution: reject invalid end statements and issue a proper diagnostic.

Also add a new test to check this behaviour.

gcc/fortran/ChangeLog:

	* parse.cc (parse_omp_metadirective_body): Reject OMP end statements
	at the end of an OMP metadirective.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/metadirective-9.f90: New test.
---
 gcc/fortran/ChangeLog.omp                     |  5 ++++
 gcc/fortran/parse.cc                          | 14 +++++++++
 gcc/testsuite/ChangeLog.omp                   |  4 +++
 .../gfortran.dg/gomp/metadirective-9.f90      | 29 +++++++++++++++++++
 4 files changed, 52 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90

diff --git gcc/fortran/ChangeLog.omp gcc/fortran/ChangeLog.omp
index 8c89cd5bd43..7b253608bf8 100644
--- gcc/fortran/ChangeLog.omp
+++ gcc/fortran/ChangeLog.omp
@@ -1,3 +1,8 @@
+2022-09-21  Paul-Antoine Arras  <pa@codesourcery.com>
+
+        * parse.cc (parse_omp_metadirective_body): Reject OMP end statements
+        at the end of an OMP metadirective.
+
 2022-09-09  Tobias Burnus  <tobias@codesourcery.com>
 
 	Backport from mainline:
diff --git gcc/fortran/parse.cc gcc/fortran/parse.cc
index b35d76a4f6b..1f1fa0eba0e 100644
--- gcc/fortran/parse.cc
+++ gcc/fortran/parse.cc
@@ -5863,6 +5863,20 @@ parse_omp_metadirective_body (gfc_statement omp_st)
 	  break;
 	}
 
+      if (gfc_state_stack->state == COMP_OMP_METADIRECTIVE
+	  && startswith (gfc_ascii_statement (st), "!$OMP END "))
+	{
+	  for (gfc_state_data *p = gfc_state_stack; p; p = p->previous)
+	    if (p->state == COMP_OMP_STRUCTURED_BLOCK)
+	      goto finish;
+	  gfc_error (
+	    "Unexpected %s statement in an OMP METADIRECTIVE block at %C",
+	    gfc_ascii_statement (st));
+	  reject_statement ();
+	  st = next_statement ();
+	}
+    finish:
+
       gfc_in_metadirective_body = old_in_metadirective_body;
 
       if (gfc_state_stack->head)
diff --git gcc/testsuite/ChangeLog.omp gcc/testsuite/ChangeLog.omp
index e0c8c138620..f075354af4d 100644
--- gcc/testsuite/ChangeLog.omp
+++ gcc/testsuite/ChangeLog.omp
@@ -1,3 +1,7 @@
+2022-09-21  Paul-Antoine Arras  <pa@codesourcery.com>
+
+        * gfortran.dg/gomp/metadirective-9.f90: New test.
+
 2022-09-09  Paul-Antoine Arras  <pa@codesourcery.com>
 
 	Backport from mainline:
diff --git gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90 gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90
new file mode 100644
index 00000000000..4db37dd0ef9
--- /dev/null
+++ gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90
@@ -0,0 +1,29 @@
+! { dg-do compile }
+
+program OpenMP_Metadirective_WrongEnd_Test
+
+  integer :: &
+    iV, jV, kV
+  integer, dimension ( 3 ) :: &
+    lV, uV
+  logical :: &
+    UseDevice
+
+    !$OMP metadirective &
+    !$OMP   when ( user = { condition ( UseDevice ) } &
+    !$OMP     : target teams distribute parallel do simd collapse ( 3 ) &
+    !$OMP         private ( iaVS ) ) &
+    !$OMP   default ( parallel do simd collapse ( 3 ) private ( iaVS ) )
+    do kV = lV ( 3 ), uV ( 3 )
+      do jV = lV ( 2 ), uV ( 2 )
+        do iV = lV ( 1 ), uV ( 1 )
+
+
+        end do
+      end do
+    end do
+    !$OMP end target teams distribute parallel do simd ! { dg-error "Unexpected !.OMP END TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD statement in an OMP METADIRECTIVE block at .1." }
+
+
+end program
+
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [OG12][PATCH] OpenMP: Fix ICE with OMP metadirectives
  2022-09-21 21:18 [OG12][PATCH] OpenMP: Fix ICE with OMP metadirectives Paul-Antoine Arras
@ 2022-09-22  9:01 ` Tobias Burnus
  2022-09-28 13:47   ` Paul-Antoine Arras
  0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2022-09-22  9:01 UTC (permalink / raw)
  To: Paul-Antoine Arras, gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 7548 bytes --]

Hello Paul-Antoine, hi all,

On 21.09.22 23:18, Paul-Antoine Arras wrote:


Here is a patch that fixes an ICE in gfortran triggered by an invalid end statement at the end of an OMP metadirective:


Remark for other reads of this email: This only applies to OG12 as mainline
does not have the following patches:
---------------------------
Patch set from December: https://gcc.gnu.org/pipermail/gcc-patches/2021-December/thread.html#586599
Reviews in May (multiple, e.g. 1/7 is at https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595762.html )
Fortran follow-up patches: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590368.html
---------------------------


I played a bit with the patch. I think it looks okayish.

It seems to handle the code in question correctly. I worried about some bits,
which turned out to be unfounded. However, I found some related issues that
look similar (but are unaffected of the patch).

I do not quickly see whether your patch should handle them as well or whether
that's a completely separate code location which requires a completely separate
patch. – If the latter, the patch LGTM, otherwise, it would be great if it could
handle the other issues as well.


First, can you include in your patch also:

--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -2520 +2520 @@ gfc_ascii_statement (gfc_statement st)
-      p = "!OMP END METADIRECTIVE";
+      p = "!$OMP END METADIRECTIVE";


The following first two examples are about "omp begin/end metadirective" - while
"your" ICE was about the non-delimited "omp metadirective".

The following program gives an ICE - and I believe it is valid code.
When replacing 'nothing' by 'parallel', it is instead rejected
("Unexpected !OMP END METADIRECTIVE statement".)

! ice-on-valid-code, rejects-valid -- this is bad!
subroutine test2
 logical :: UseDevice
 !$OMP begin metadirective &
 !$OMP   when ( user = { condition ( UseDevice ) } &
 !$OMP     : nothing ) &
 !$OMP   default ( parallel )
   block
      call bar()
   end block
   !block
   !   call foo()
   !end block
 !$omp end metadirective
end

I wonder whether it is also related to strictly nested blocks,
but, in any case, (strictly/loosely structures) blocks do not
apply here ('begin/end metadirective' has association 'delimited'
not 'block'). – Thus, I tried also with two 'block' to check this
is also accepted.



Likewise, the following code is mishandled in an odd way – but only
if all when/default use the same delimited directive:

! diagnostic, accepts-invalid -- not ideal but neither ICE nor rejecting valid code
 !$OMP begin metadirective &
 !$OMP   when ( user = { condition ( UseDevice ) } &
 !$OMP     : parallel ) &
 !$OMP   default ( parallel )
    call bar()
 !!$omp end parallel  ! (1)
 !!$omp end metadirective  ! (2)
end

Uncommenting (2): it is accepted (and it should be)
Uncommenting (1): This is accepted - but shouldn't. There is an
"end metadirective" missing – that is required.
Uncommenting (1) and (2): The line (1) accepted but then (2) is rejected

Note: This only happens if all directives in when/default are the same
such that the 'end parallel' works for all of them.


I also tried the non-delimited '!$omp metadirective' (i.e. no begin...end),
but that seems to work fine. I still wonder whether it should be added as
another testcase (three tests, could be in the same files), just to make sure.

The following handles "end parallel" if there is only "parallel" in when/default;
however, I think all variants of the following are valid
(but bad style - for a (non-loop-associated) block-associated directive,
using begin/end makes more sense than dumping an explicit end directive.)


! OK - add as three test cases (?)
program test
 logical :: UseDevice
 !$OMP metadirective &
 !$OMP   when ( user = { condition ( UseDevice ) } &
 !$OMP     : parallel ) &
 !$OMP   default ( parallel )
 block
    ! ...
 end block
 !$omp end parallel  ! Accepted, but only all cases have 'parallel'
end program

The "end parallel" is optional here as there is a strictly structured block
(the "block ... end block"); without "end parallel" or without the
"block" / "end block" (→ loosely structured block, then "end parallel" is required),
it also work. (Hence, three testcases.)

 * * *


To the patch - one important comment to "ChangeLog(.omp)" and otherwise only
some personal comments.


Subject: [PATCH] OpenMP: Fix ICE with OMP metadirectives
...
Also add a new test to check this behaviour.

(Personally, I would remove the last line: I always would expect a testcase
if feasible and 4 lines later there is also "New test.". But it also does
not harm. – Especially when just browsing through the comments ("git log"),
I am happy if the log is short. However, when studying a commit in more detail,
I am happy about understanding what/why a patch did something. )

(Personally, I also would also add 'Fortran' to the subject line if only related
Fortran; first, it makes it more likely to get reviewed by Fortran maintainers
and it also groups a patch a bit. But as OpenMP already groups it, this is only
a minor personal taste. - In tendency, it helps if the patch discussion email thread
and the commit match. I recently did miss to find a patch because of "Push" vs.
"push" because git log invoked "less" such that it searches case sensitively by default ...
Thus, it is better not to change the subject for this patch!)




gcc/fortran/ChangeLog:

        * parse.cc (parse_omp_metadirective_body): Reject OMP end statements
        at the end of an OMP metadirective.

gcc/testsuite/ChangeLog:

        * gfortran.dg/gomp/metadirective-9.f90: New test.

...

--- gcc/testsuite/ChangeLog.omp
+++ gcc/testsuite/ChangeLog.omp
@@ -1,3 +1,7 @@
+2022-09-21  Paul-Antoine Arras  <pa@codesourcery.com><mailto:pa@codesourcery.com>
+
+        * gfortran.dg/gomp/metadirective-9.f90: New test.

There should be a 'tab' not spaces before '*'. That also applies to the
commit log but you or mklog.py already did that.


+++ gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90
@@ -0,0 +1,29 @@
+! { dg-do compile }
+
+program OpenMP_Metadirective_WrongEnd_Test

...

+    !$OMP         private ( iaVS ) ) &
+    !$OMP   default ( parallel do simd collapse ( 3 ) private ( iaVS ) )

General Fortran comment:

For testcases, it is perfectly fine - especially if they are compile only.
However, when writing production code, it is strongly recommended to add an
"implicit none" (after any 'use'/'import' lines, if any. Here it would be at
"..." directly after the "program" line.)

The "implicit none" helps to avoid some odd issues when writing code, which
are hard to trace. In this testcase, adding the implicit line will cause that is fails to
compile as iaVS is not declared and it is therefore implicitly declared as integer.

Here, it is perfectly fine - but a typical problem is a misspelling (e.g. dev_num vs. devnum)
and suddenly the wrong variable is used ... (in this example, even an implicitly declared REAL
number while surely a device number should be an integer).

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [OG12][PATCH] OpenMP: Fix ICE with OMP metadirectives
  2022-09-22  9:01 ` Tobias Burnus
@ 2022-09-28 13:47   ` Paul-Antoine Arras
  2022-09-28 14:26     ` Tobias Burnus
  0 siblings, 1 reply; 4+ messages in thread
From: Paul-Antoine Arras @ 2022-09-28 13:47 UTC (permalink / raw)
  To: Tobias Burnus, gcc-patches, fortran

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

Hi Tobias,

Thanks for your review.

Here is a revised patch resulting from your comments.

The only issue that I could not easily fix is the following code 
triggering an ICE:

```
    !$OMP begin metadirective &
    !$OMP   when ( user = { condition ( UseDevice ) } &
    !$OMP     : nothing ) &
    !$OMP   default ( parallel )
    block
       call foo()
    end block
    call bar()	<--- ICE
    !$omp end metadirective
```

I filed a PR on Bugzilla: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107067

Is it OK for you?
-- 
PA

[-- Attachment #2: 0001-OpenMP-Fix-ICE-with-OMP-metadirectives.patch --]
[-- Type: text/plain, Size: 9279 bytes --]

From 03476214bda71c6581b7978cf9fd5b701896a052 Mon Sep 17 00:00:00 2001
From: Paul-Antoine Arras <pa@codesourcery.com>
Date: Wed, 21 Sep 2022 15:52:56 +0000
Subject: [PATCH] OpenMP: Fix ICE with OMP metadirectives

Problem: ending an OpenMP metadirective block with an OMP end statement
results in an internal compiler error.
Solution: reject invalid end statements and issue a proper diagnostic.

This revision also fixes a couple of minor metadirective issues and adds
related test cases.

gcc/fortran/ChangeLog:

	* parse.cc (gfc_ascii_statement): Missing $ in !$OMP END METADIRECTIVE.
	(parse_omp_structured_block): Fix handling of OMP end metadirective.
	(parse_omp_metadirective_body): Reject OMP end statements
	at the end of an OMP metadirective.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/metadirective-1.f90: Match !$OMP END METADIRECTIVE.
	* gfortran.dg/gomp/metadirective-10.f90: New test.
	* gfortran.dg/gomp/metadirective-11.f90: New xfail test.
	* gfortran.dg/gomp/metadirective-9.f90: New test.
---
 gcc/fortran/ChangeLog.omp                     |  7 ++++
 gcc/fortran/parse.cc                          | 32 ++++++++++-----
 gcc/testsuite/ChangeLog.omp                   |  7 ++++
 .../gfortran.dg/gomp/metadirective-1.f90      |  2 +-
 .../gfortran.dg/gomp/metadirective-10.f90     | 40 +++++++++++++++++++
 .../gfortran.dg/gomp/metadirective-11.f90     | 33 +++++++++++++++
 .../gfortran.dg/gomp/metadirective-9.f90      | 30 ++++++++++++++
 7 files changed, 141 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/gomp/metadirective-10.f90
 create mode 100644 gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
 create mode 100644 gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90

diff --git gcc/fortran/ChangeLog.omp gcc/fortran/ChangeLog.omp
index 8c89cd5bd43..14f897e8fec 100644
--- gcc/fortran/ChangeLog.omp
+++ gcc/fortran/ChangeLog.omp
@@ -1,3 +1,10 @@
+2022-09-21  Paul-Antoine Arras  <pa@codesourcery.com>
+
+	* parse.cc (gfc_ascii_statement): Missing $ in !$OMP END METADIRECTIVE.
+	(parse_omp_structured_block): Fix handling of OMP end metadirective.
+	(parse_omp_metadirective_body): Reject OMP end statements
+	at the end of an OMP metadirective.
+
 2022-09-09  Tobias Burnus  <tobias@codesourcery.com>
 
 	Backport from mainline:
diff --git gcc/fortran/parse.cc gcc/fortran/parse.cc
index b35d76a4f6b..fc88111a7ad 100644
--- gcc/fortran/parse.cc
+++ gcc/fortran/parse.cc
@@ -2517,7 +2517,7 @@ gfc_ascii_statement (gfc_statement st)
       p = "!$OMP END MASTER TASKLOOP SIMD";
       break;
     case ST_OMP_END_METADIRECTIVE:
-      p = "!OMP END METADIRECTIVE";
+      p = "!$OMP END METADIRECTIVE";
       break;
     case ST_OMP_END_ORDERED:
       p = "!$OMP END ORDERED";
@@ -5643,9 +5643,15 @@ parse_omp_structured_block (gfc_statement omp_st, bool workshare_stmts_only)
   np->block = NULL;
 
   omp_end_st = gfc_omp_end_stmt (omp_st, false, true);
-  if (omp_st == ST_NONE)
+  if (omp_end_st == ST_NONE)
     gcc_unreachable ();
 
+  /* If handling a metadirective variant, treat 'omp end metadirective'
+     as the expected end statement for the current construct.  */
+  if (gfc_state_stack->previous != NULL
+      && gfc_state_stack->previous->state == COMP_OMP_BEGIN_METADIRECTIVE)
+    omp_end_st = ST_OMP_END_METADIRECTIVE;
+
   bool block_construct = false;
   gfc_namespace *my_ns = NULL;
   gfc_namespace *my_parent = NULL;
@@ -5744,13 +5750,6 @@ parse_omp_structured_block (gfc_statement omp_st, bool workshare_stmts_only)
       else
 	st = parse_executable (st);
 
-      /* If handling a metadirective variant, treat 'omp end metadirective'
-	 as the expected end statement for the current construct.  */
-      if (st == ST_OMP_END_METADIRECTIVE
-	  && gfc_state_stack->previous != NULL
-	  && gfc_state_stack->previous->state == COMP_OMP_BEGIN_METADIRECTIVE)
-	st = omp_end_st;
-
       if (st == ST_NONE)
 	unexpected_eof ();
       else if (st == ST_OMP_SECTION
@@ -5863,6 +5862,21 @@ parse_omp_metadirective_body (gfc_statement omp_st)
 	  break;
 	}
 
+      if (gfc_state_stack->state == COMP_OMP_METADIRECTIVE
+	  && startswith (gfc_ascii_statement (st), "!$OMP END "))
+	{
+	  for (gfc_state_data *p = gfc_state_stack; p; p = p->previous)
+	    if (p->state == COMP_OMP_STRUCTURED_BLOCK
+		|| p->state == COMP_OMP_BEGIN_METADIRECTIVE)
+	      goto finish;
+	  gfc_error (
+	    "Unexpected %s statement in an OMP METADIRECTIVE block at %C",
+	    gfc_ascii_statement (st));
+	  reject_statement ();
+	  st = next_statement ();
+	}
+    finish:
+
       gfc_in_metadirective_body = old_in_metadirective_body;
 
       if (gfc_state_stack->head)
diff --git gcc/testsuite/ChangeLog.omp gcc/testsuite/ChangeLog.omp
index e0c8c138620..f0d1b1a388b 100644
--- gcc/testsuite/ChangeLog.omp
+++ gcc/testsuite/ChangeLog.omp
@@ -1,3 +1,10 @@
+2022-09-21  Paul-Antoine Arras  <pa@codesourcery.com>
+
+	* gfortran.dg/gomp/metadirective-1.f90: Match !$OMP END METADIRECTIVE.
+	* gfortran.dg/gomp/metadirective-10.f90: New test.
+	* gfortran.dg/gomp/metadirective-11.f90: New test.
+	* gfortran.dg/gomp/metadirective-9.f90: New test.
+
 2022-09-09  Paul-Antoine Arras  <pa@codesourcery.com>
 
 	Backport from mainline:
diff --git gcc/testsuite/gfortran.dg/gomp/metadirective-1.f90 gcc/testsuite/gfortran.dg/gomp/metadirective-1.f90
index aa439fc855e..ca62aecad89 100644
--- gcc/testsuite/gfortran.dg/gomp/metadirective-1.f90
+++ gcc/testsuite/gfortran.dg/gomp/metadirective-1.f90
@@ -37,5 +37,5 @@ program main
     do i = 1, N
       c(i) = a(i) * b(i)
     end do
-  !$omp end metadirective ! { dg-error "Unexpected !OMP END METADIRECTIVE statement at .1." }
+  !$omp end metadirective ! { dg-error "Unexpected !.OMP END METADIRECTIVE statement at .1." }
 end program
diff --git gcc/testsuite/gfortran.dg/gomp/metadirective-10.f90 gcc/testsuite/gfortran.dg/gomp/metadirective-10.f90
new file mode 100644
index 00000000000..5dad5d29eb6
--- /dev/null
+++ gcc/testsuite/gfortran.dg/gomp/metadirective-10.f90
@@ -0,0 +1,40 @@
+! { dg-do compile }
+
+program metadirectives
+   implicit none
+   logical :: UseDevice
+
+   !$OMP metadirective &
+   !$OMP   when ( user = { condition ( UseDevice ) } &
+   !$OMP     : parallel ) &
+   !$OMP   default ( parallel )
+   block
+      call bar()
+   end block
+
+   !$OMP metadirective &
+   !$OMP   when ( user = { condition ( UseDevice ) } &
+   !$OMP     : parallel ) &
+   !$OMP   default ( parallel )
+   call bar()
+   !$omp end parallel  ! Accepted, because all cases have 'parallel'
+   
+   !$OMP begin metadirective &
+   !$OMP   when ( user = { condition ( UseDevice ) } &
+   !$OMP     : nothing ) &
+   !$OMP   default ( parallel )
+   call bar()
+   block
+      call foo()
+   end block
+   !$OMP end metadirective
+
+   !$OMP begin metadirective &
+   !$OMP   when ( user = { condition ( UseDevice ) } &
+   !$OMP     : parallel ) &
+   !$OMP   default ( parallel )
+   call bar()
+   !$omp end parallel  ! { dg-error "Unexpected !.OMP END PARALLEL statement at .1." }
+end program ! { dg-error "Unexpected END statement at .1." }
+
+! { dg-error "Unexpected end of file" "" { target *-*-* } 0 }
diff --git gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90 gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
new file mode 100644
index 00000000000..e7de70e6259
--- /dev/null
+++ gcc/testsuite/gfortran.dg/gomp/metadirective-11.f90
@@ -0,0 +1,33 @@
+! { dg-do compile }
+! { dg-ice "Statements following a block in a metadirective" }
+! PR fortran/107067
+
+program metadirectives
+   implicit none
+   logical :: UseDevice
+
+   !$OMP begin metadirective &
+   !$OMP   when ( user = { condition ( UseDevice ) } &
+   !$OMP     : nothing ) &
+   !$OMP   default ( parallel )
+   block
+      call foo()
+   end block
+   call bar()   ! FIXME/XFAIL ICE in parse_omp_metadirective_body()
+   !$omp end metadirective
+
+
+   !$OMP begin metadirective &
+   !$OMP   when ( user = { condition ( UseDevice ) } &
+   !$OMP     : nothing ) &
+   !$OMP   default ( parallel )
+   block
+      call bar()
+   end block
+   block        ! FIXME/XFAIL ICE in parse_omp_metadirective_body()
+      call foo()
+   end block
+   !$omp end metadirective
+end program
+
+
diff --git gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90 gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90
new file mode 100644
index 00000000000..e6ab3fc0a65
--- /dev/null
+++ gcc/testsuite/gfortran.dg/gomp/metadirective-9.f90
@@ -0,0 +1,30 @@
+! { dg-do compile }
+
+program OpenMP_Metadirective_WrongEnd_Test
+  implicit none
+
+  integer :: &
+  iaVS, iV, jV, kV
+  integer, dimension ( 3 ) :: &
+    lV, uV
+  logical :: &
+    UseDevice
+
+    !$OMP metadirective &
+    !$OMP   when ( user = { condition ( UseDevice ) } &
+    !$OMP     : target teams distribute parallel do simd collapse ( 3 ) &
+    !$OMP         private ( iaVS ) ) &
+    !$OMP   default ( parallel do simd collapse ( 3 ) private ( iaVS ) )
+    do kV = lV ( 3 ), uV ( 3 )
+      do jV = lV ( 2 ), uV ( 2 )
+        do iV = lV ( 1 ), uV ( 1 )
+
+
+        end do
+      end do
+    end do
+    !$OMP end target teams distribute parallel do simd ! { dg-error "Unexpected !.OMP END TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD statement in an OMP METADIRECTIVE block at .1." }
+
+
+end program
+
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [OG12][PATCH] OpenMP: Fix ICE with OMP metadirectives
  2022-09-28 13:47   ` Paul-Antoine Arras
@ 2022-09-28 14:26     ` Tobias Burnus
  0 siblings, 0 replies; 4+ messages in thread
From: Tobias Burnus @ 2022-09-28 14:26 UTC (permalink / raw)
  To: Paul-Antoine Arras, gcc-patches, fortran

Hi Paul-Antoine, hi all,

On 28.09.22 15:47, Paul-Antoine Arras wrote:
> Here is a revised patch resulting from your comments.
> The only issue that I could not easily fix is the following code
> triggering an ICE: [...]
> I filed a PR on Bugzilla:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107067
> Is it OK for you?

Thanks for the update – LGTM.

(We may want to revise some general block handling, but that's something
for either upstreaming – or even after the upstreaming hen fixing the PR.)

Two minor remarks: (a) Don't forget to update the date in the
ChangeLog.omp. (b) On can shorten bug URL by using
https://gcc.gnu.org/PR107067 which is, well, shorter. (Similarly,
https://gcc.gnu.org/g:<githash> or https://gcc.gnu.org/r... for the svn
or 'git gcc-descr [--full]' revision number also works.)

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-09-28 14:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 21:18 [OG12][PATCH] OpenMP: Fix ICE with OMP metadirectives Paul-Antoine Arras
2022-09-22  9:01 ` Tobias Burnus
2022-09-28 13:47   ` Paul-Antoine Arras
2022-09-28 14:26     ` Tobias Burnus

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).