Re: [patch] OpenMP: Fortran - support omp flush's memorder clauses

2020-10-23 Thread Bernhard Reutner-Fischer via Gcc-patches
On 22 October 2020 17:02:36 CEST, Jakub Jelinek via Gcc-patches 
 wrote:
>On Thu, Oct 22, 2020 at 04:52:10PM +0200, Tobias Burnus wrote:
>> +  else
>> +{
>> +  enum memmodel mo = MEMMODEL_LAST;
>> +  switch (code->ext.omp_clauses->memorder)
>> +{
>> +case OMP_MEMORDER_ACQ_REL: mo = MEMMODEL_ACQ_REL; break;
>> +case OMP_MEMORDER_RELEASE: mo = MEMMODEL_RELEASE; break;
>> +case OMP_MEMORDER_ACQUIRE: mo = MEMMODEL_ACQUIRE; break;
>> +case OMP_MEMORDER_LAST: gcc_unreachable (); break;
>
>I'd probably use
>   default: gcc_unreachable (); break;
>here instead.
>Otherwise LGTM, thanks.

I have one trivial remark though. In

+{
+  if (gfc_match ("acq_rel") == MATCH_YES)
+   mo = OMP_MEMORDER_ACQ_REL;
+  else if (gfc_match ("release") == MATCH_YES)
+   mo = OMP_MEMORDER_RELEASE;
+  else if (gfc_match ("acquire") == MATCH_YES)
+   mo = OMP_MEMORDER_ACQUIRE;
+  else
+   {
+ gfc_error ("Expected AQC_REL, RELEASE, or ACQUIRE at %C");

I suggest s/AQC/ACQ/
LGTM too, otherwise ;)


Re: [patch] OpenMP: Fortran - support omp flush's memorder clauses

2020-10-22 Thread Jakub Jelinek via Gcc-patches
On Thu, Oct 22, 2020 at 04:52:10PM +0200, Tobias Burnus wrote:
> +  else
> +{
> +  enum memmodel mo = MEMMODEL_LAST;
> +  switch (code->ext.omp_clauses->memorder)
> + {
> + case OMP_MEMORDER_ACQ_REL: mo = MEMMODEL_ACQ_REL; break;
> + case OMP_MEMORDER_RELEASE: mo = MEMMODEL_RELEASE; break;
> + case OMP_MEMORDER_ACQUIRE: mo = MEMMODEL_ACQUIRE; break;
> + case OMP_MEMORDER_LAST: gcc_unreachable (); break;

I'd probably use
default: gcc_unreachable (); break;
here instead.
Otherwise LGTM, thanks.

Jakub



[patch] OpenMP: Fortran - support omp flush's memorder clauses

2020-10-22 Thread Tobias Burnus

Add the memorder clause of OpenMP 5 which are already supported by C/C++.

(I copied & adapted the two C/C++ testcases.)

OK?

Tobias

PS: An alternative to creating an additional enum would be to
include memmodel.h in gfortran.h - and use it directly. But at
the end, I decided to copy to follow the current code and
add a new enum – and include it only in trans-openmp.c.

-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
OpenMP: Fortran - support omp flush's memorder clauses

gcc/fortran/ChangeLog:

	* gfortran.h (enum gfc_omp_memorder): Add.
	(gfc_omp_clauses): Use it.
	* openmp.c (gfc_match_omp_flush): Match memorder clauses.
	* trans-openmp.c (gfc_trans_omp_flush): Handle them.
	(gfc_trans_omp_directive): Update call.

gcc/testsuite/ChangeLog:

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

 gcc/fortran/gfortran.h |  9 +++
 gcc/fortran/openmp.c   | 29 +
 gcc/fortran/trans-openmp.c | 30 +++---
 gcc/testsuite/gfortran.dg/gomp/flush-1.f90 | 41 ++
 gcc/testsuite/gfortran.dg/gomp/flush-2.f90 | 18 +
 5 files changed, 123 insertions(+), 4 deletions(-)

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index d0cea838444..931b908a16e 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1361,6 +1361,14 @@ enum gfc_omp_requires_kind
    | OMP_REQ_ATOMIC_MEM_ORDER_RELAXED)
 };
 
+enum gfc_omp_memorder
+{
+  OMP_MEMORDER_ACQ_REL,
+  OMP_MEMORDER_RELEASE,
+  OMP_MEMORDER_ACQUIRE,
+  OMP_MEMORDER_LAST
+};
+
 typedef struct gfc_omp_clauses
 {
   struct gfc_expr *if_expr;
@@ -1376,6 +1384,7 @@ typedef struct gfc_omp_clauses
   bool inbranch, notinbranch, defaultmap, nogroup;
   bool sched_simd, sched_monotonic, sched_nonmonotonic;
   bool simd, threads, depend_source, order_concurrent;
+  enum gfc_omp_memorder memorder;
   enum gfc_omp_cancel_kind cancel;
   enum gfc_omp_proc_bind_kind proc_bind;
   struct gfc_expr *safelen_expr;
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 1efce33e519..b143ba7454a 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2766,15 +2766,44 @@ match
 gfc_match_omp_flush (void)
 {
   gfc_omp_namelist *list = NULL;
+  gfc_omp_clauses *c = NULL;
+  gfc_gobble_whitespace ();
+  enum gfc_omp_memorder mo = OMP_MEMORDER_LAST;
+  if (gfc_match_omp_eos () == MATCH_NO && gfc_peek_ascii_char () != '(')
+{
+  if (gfc_match ("acq_rel") == MATCH_YES)
+	mo = OMP_MEMORDER_ACQ_REL;
+  else if (gfc_match ("release") == MATCH_YES)
+	mo = OMP_MEMORDER_RELEASE;
+  else if (gfc_match ("acquire") == MATCH_YES)
+	mo = OMP_MEMORDER_ACQUIRE;
+  else
+	{
+	  gfc_error ("Expected AQC_REL, RELEASE, or ACQUIRE at %C");
+	  return MATCH_ERROR;
+	}
+  c = gfc_get_omp_clauses ();
+  c->memorder = mo;
+}
   gfc_match_omp_variable_list (" (", , true);
+  if (list && mo != OMP_MEMORDER_LAST)
+{
+  gfc_error ("List specified together with memory order clause in FLUSH "
+		 "directive at %C");
+  gfc_free_omp_namelist (list);
+  gfc_free_omp_clauses (c);
+  return MATCH_ERROR;
+}
   if (gfc_match_omp_eos () != MATCH_YES)
 {
   gfc_error ("Unexpected junk after $OMP FLUSH statement at %C");
   gfc_free_omp_namelist (list);
+  gfc_free_omp_clauses (c);
   return MATCH_ERROR;
 }
   new_st.op = EXEC_OMP_FLUSH;
   new_st.ext.omp_namelist = list;
+  new_st.ext.omp_clauses = c;
   return MATCH_YES;
 }
 
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 378088a9d04..fdb573f8072 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -38,6 +38,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "gomp-constants.h"
 #include "omp-general.h"
 #include "omp-low.h"
+#include "memmodel.h"  /* For MEMMODEL_ enums.  */
+
 #undef GCC_DIAG_STYLE
 #define GCC_DIAG_STYLE __gcc_tdiag__
 #include "diagnostic-core.h"
@@ -4785,10 +4787,30 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
 }
 
 static tree
-gfc_trans_omp_flush (void)
+gfc_trans_omp_flush (gfc_code *code)
 {
-  tree decl = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
-  return build_call_expr_loc (input_location, decl, 0);
+  tree call;
+  if (!code->ext.omp_clauses
+  || code->ext.omp_clauses->memorder == OMP_MEMORDER_LAST)
+{
+  call = builtin_decl_explicit (BUILT_IN_SYNC_SYNCHRONIZE);
+  call = build_call_expr_loc (input_location, call, 0);
+}
+  else
+{
+  enum memmodel mo = MEMMODEL_LAST;
+  switch (code->ext.omp_clauses->memorder)
+	{
+	case OMP_MEMORDER_ACQ_REL: mo = MEMMODEL_ACQ_REL; break;
+	case OMP_MEMORDER_RELEASE: mo = MEMMODEL_RELEASE; break;
+	case OMP_MEMORDER_ACQUIRE: mo = MEMMODEL_ACQUIRE; break;
+	case OMP_MEMORDER_LAST: