diff mbox series

(Re: [committed] openmp: Add nothing directive support)

Message ID e5b81d8b-7397-30ca-da27-c98eeb6e3fd4@codesourcery.com
State New
Headers show
Series (Re: [committed] openmp: Add nothing directive support) | expand

Commit Message

Tobias Burnus Aug. 18, 2021, 11:10 a.m. UTC
On 18.08.21 11:18, Jakub Jelinek wrote:

> As has been clarified, it is intentional that nothing directive is accepted
> in substatements of selection and looping statements and after labels and
> is handled as if the directive just isn't there ...

And here is the Fortran version; as ST_NONE is used, it is also just ignored like a comment.

However, there is a pure-procedure check, which triggers. I think that's
fine and a spec issue. (Tracked on the OpenMP side as Issue 2913.)

I think otherwise the patch is boring - as boring as 'omp nothing' itself
(outside its use in metadirectives).

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

Comments

Jakub Jelinek Aug. 18, 2021, 11:18 a.m. UTC | #1
On Wed, Aug 18, 2021 at 01:10:12PM +0200, Tobias Burnus wrote:
> I think otherwise the patch is boring - as boring as 'omp nothing' itself
> (outside its use in metadirectives).

Yeah, I think nobody sane will use it for anything but metadirective
except in compiler testsuites.

> Fortran: Add OpenMP's nothing directive support
> 
> Fortran version of commit 5079b7781a2c506dcdfb241347d74c7891268225
> 
> gcc/fortran/ChangeLog:
> 
> 	* match.h (gfc_match_omp_nothing): New.
> 	* openmp.c (gfc_match_omp_nothing): New.
> 	* parse.c (decode_omp_directive): Match 'nothing' directive.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gfortran.dg/nothing-1.f90: New test.
> 	* gfortran.dg/nothing-2.f90: New test.

LGTM, thanks.

> +  !$omp nothing
> +  if (.false.) &
> +& &    !$omp nothing
> +    i = i + 1
> +
> +  if (.false.) &
> +&   & !$omp nothing
> +    then
> +  end if

I'm actually not sure if the above really is valid (well, treated as OpenMP
directive, rather than just an arbitrary comment), e.g. 5.0 2.2.2 says:
The sentinel can appear in any column but must be preceded only by white space;
which is not the case above.  And I think that is the reason we don't have a
Fortran counterpart to the stand-alone and declarative directive
restrictions C/C++ has that they can't appear in certain contexts.

So, do we actually parse
  if (.true.) &
& & !$omp barrier
as if with barrier construct in it?

	Jakub
Tobias Burnus Aug. 18, 2021, 8:01 p.m. UTC | #2
On 18.08.21 13:18, Jakub Jelinek wrote:

>> gcc/testsuite/ChangeLog:
>>      * gfortran.dg/nothing-1.f90: New test.
>>      * gfortran.dg/nothing-2.f90: New test.

While testing manually with -fopenmp, I did manage to place it for the
testsuite run outside the 'gomp/' subdirectory. – Now fixed → attachment.

For completeness (as already discussed on IRC):

>> +  !$omp nothing
>> +  if (.false.) &
>> +& &    !$omp nothing
>> +    i = i + 1

This indeed does not trigger the sentinel (as also discussed on IRC and as comment in the test).
While
   if (x) &
     !$omp nothing
gives an error but it is not obviously invalid. → That's now OpenMP spec bug #2914.

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
diff mbox series

Patch

Fortran: Add OpenMP's nothing directive support

Fortran version of commit 5079b7781a2c506dcdfb241347d74c7891268225

gcc/fortran/ChangeLog:

	* match.h (gfc_match_omp_nothing): New.
	* openmp.c (gfc_match_omp_nothing): New.
	* parse.c (decode_omp_directive): Match 'nothing' directive.

gcc/testsuite/ChangeLog:

	* gfortran.dg/nothing-1.f90: New test.
	* gfortran.dg/nothing-2.f90: New test.

 gcc/fortran/match.h                     |  1 +
 gcc/fortran/openmp.c                    | 11 +++++++++++
 gcc/fortran/parse.c                     |  3 +++
 gcc/testsuite/gfortran.dg/nothing-1.f90 | 26 ++++++++++++++++++++++++++
 gcc/testsuite/gfortran.dg/nothing-2.f90 |  7 +++++++
 5 files changed, 48 insertions(+)

diff --git a/gcc/fortran/match.h b/gcc/fortran/match.h
index aac16a8d3d0..5127b4b8ea3 100644
--- a/gcc/fortran/match.h
+++ b/gcc/fortran/match.h
@@ -175,6 +175,7 @@  match gfc_match_omp_masked_taskloop_simd (void);
 match gfc_match_omp_master (void);
 match gfc_match_omp_master_taskloop (void);
 match gfc_match_omp_master_taskloop_simd (void);
+match gfc_match_omp_nothing (void);
 match gfc_match_omp_ordered (void);
 match gfc_match_omp_ordered_depend (void);
 match gfc_match_omp_parallel (void);
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 9675b658409..fd219dc9c4d 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -4797,6 +4797,17 @@  gfc_match_omp_ordered (void)
   return match_omp (EXEC_OMP_ORDERED, OMP_ORDERED_CLAUSES);
 }
 
+match
+gfc_match_omp_nothing (void)
+{
+  if (gfc_match_omp_eos () != MATCH_YES)
+    {
+      gfc_error ("Unexpected junk after $OMP NOTHING statement at %C");
+      return MATCH_ERROR;
+    }
+  /* Will use ST_NONE; therefore, no EXEC_OMP_ is needed.  */
+  return MATCH_YES;
+}
 
 match
 gfc_match_omp_ordered_depend (void)
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 24cc9bfb9f1..d004732677c 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -1005,6 +1005,9 @@  decode_omp_directive (void)
 	      ST_OMP_MASTER_TASKLOOP);
       matcho ("master", gfc_match_omp_master, ST_OMP_MASTER);
       break;
+    case 'n':
+      matcho ("nothing", gfc_match_omp_nothing, ST_NONE);
+      break;
     case 'l':
       matcho ("loop", gfc_match_omp_loop, ST_OMP_LOOP);
       break;
diff --git a/gcc/testsuite/gfortran.dg/nothing-1.f90 b/gcc/testsuite/gfortran.dg/nothing-1.f90
new file mode 100644
index 00000000000..2bc3688e2ac
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/nothing-1.f90
@@ -0,0 +1,26 @@ 
+module m
+  implicit none (type, external)
+  !$omp nothing
+
+  type t
+    !$omp nothing
+    integer s
+  end type
+
+contains
+
+integer function foo (i)
+  integer :: i
+
+  !$omp nothing
+  if (.false.) &
+& &    !$omp nothing
+    i = i + 1
+
+  if (.false.) &
+&   & !$omp nothing
+    then
+  end if
+  foo = i
+end
+end module
diff --git a/gcc/testsuite/gfortran.dg/nothing-2.f90 b/gcc/testsuite/gfortran.dg/nothing-2.f90
new file mode 100644
index 00000000000..74a4a5a22b0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/nothing-2.f90
@@ -0,0 +1,7 @@ 
+pure subroutine foo
+  !$omp nothing  ! { dg-error "OpenMP directives other than SIMD or DECLARE TARGET at .1. may not appear in PURE procedures" }
+end subroutine
+
+subroutine bar
+  !$omp nothing foo  ! { dg-error "Unexpected junk after $OMP NOTHING statement" }
+end