Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD

[Piotr Kubaj via Fortran]

On 22-05-13 10:59:59, Kewen.Lin wrote:
> on 2022/5/13 04:16, Segher Boessenkool wrote:
> > Hi Piotr,
> > 
> > On Tue, May 03, 2022 at 12:21:12PM +0200, pku...@freebsd.org wrote:
> >> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header.
> > 
> > Declared, not defined.  These are required to be real functions (on all
> > platforms that have these functions), not macros or inlines or whatever.
> > 
> 
> Piotr's reply "FreeBSD doesn't have this function in libm, it's
> implemented in /usr/include/fenv.h." from [1] made me feel like
> it's a definition instead of declaration.  So I thought the check
> should use AC_LINK_IFELSE instead, since one fenv.h which doesn't
> have the definition can still pass the proposed AC_COMPILE_IFELSE
> check.
> 
> I just did a further search, the powerpc fenv.h [2] does include
> the definition of feenableexcept.  By comparison, the x86 fenv.h [3]
> doesn't.  But I'm not sure if it's the same as what Piotr's
> environments have.  Hope it's similar. :-)
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593193.html
> [2] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/powerpc/fenv.h
> [3] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/x86/fenv.h

Yes, it's a definition and thanks for confirming that. As for why it's not in 
libm, I asked a developer about that:
03:04 <@adalava> It shouldn't be difficulted but I moved to other thing after 
months looking at FPE in kernel, bugs in context switch and msun test cases 
failing :-P

As far as I know, there are currently no plans to move it to libm on powerpc. 
riscv, arm and arm64 are in the same boat.

I will follow with a next patch that will check for feenableexcept() in fenv.h 
if libm check is unsuccessful.

Thanks,
Piotr Kubaj.

> 
> BR,
> Kewen
> 


signature.asc
Description: PGP signature

Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD

[Segher Boessenkool]

On Fri, May 13, 2022 at 12:34:05PM +0200, Piotr Kubaj wrote:
> On 22-05-13 10:59:59, Kewen.Lin wrote:
> > on 2022/5/13 04:16, Segher Boessenkool wrote:
> > > On Tue, May 03, 2022 at 12:21:12PM +0200, pku...@freebsd.org wrote:
> > >> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header.
> > > 
> > > Declared, not defined.  These are required to be real functions (on all
> > > platforms that have these functions), not macros or inlines or whatever.
> > > 
> > 
> > Piotr's reply "FreeBSD doesn't have this function in libm, it's
> > implemented in /usr/include/fenv.h." from [1] made me feel like
> > it's a definition instead of declaration.  So I thought the check
> > should use AC_LINK_IFELSE instead, since one fenv.h which doesn't
> > have the definition can still pass the proposed AC_COMPILE_IFELSE
> > check.
> > 
> > I just did a further search, the powerpc fenv.h [2] does include
> > the definition of feenableexcept.  By comparison, the x86 fenv.h [3]
> > doesn't.  But I'm not sure if it's the same as what Piotr's
> > environments have.  Hope it's similar. :-)
> > 
> > [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593193.html
> > [2] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/powerpc/fenv.h
> > [3] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/x86/fenv.h
> 
> Yes, it's a definition and thanks for confirming that. As for why it's not in 
> libm, I asked a developer about that:
> 03:04 <@adalava> It shouldn't be difficulted but I moved to other thing after 
> months looking at FPE in kernel, bugs in context switch and msun test cases 
> failing :-P
> 
> As far as I know, there are currently no plans to move it to libm on powerpc. 
> riscv, arm and arm64 are in the same boat.
> 
> I will follow with a next patch that will check for feenableexcept() in 
> fenv.h if libm check is unsuccessful.

FreeBSD's own documentation
() says it is
a function, and it suggests it is in the standard library even.  This
would be as expected, the same holds for the similar C standard
functions.

This  does not provide an external definition, only a static
inline (this is a good thing for a header file of course).

You can take the address of a function.  You also can interpose a real
function normally, but that may not be relevant for FreeBSD, no idea.
You can do neither with a static inline.

So this should probably be fixed?

Thanks for explaining the situation better!


Segher

[Patch] OpenMP/Fortran: Use firstprivat not alloc for ptr attach for arrays

[Tobias Burnus]

Based on sollve_vv's 
tests/4.5/target_teams_distribute/test_target_teams_distribute_nowait.F90

As discussed, for simple pointers – like here with nondescriptor array,
instead of alloc:a + pointer assign, a firstprivate + pointer assign makes
more sense.

It also avoids the race exposed by the sollve_vv testcase in some 
constellations.
(The testcase, both as attached and currently in sollve_vv [→ Issue #532], is
invalid if run with 'nowait' as there is a race related to the array - as it
only (un)mapped as (disjunct) array sections via interleaved, concurrently
running target constructs with nowait clause.)

There might be more places where this should/could be done – and in principle,
firstprivate could also be useful for an array descriptor (but not its data);
this could also be explored. (Including whether it should then not be privatized
with shared memory.)

OK?

Tobias

PS: OpenACC is excluded as it does its own firstprivate handling.
-
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/Fortran: Use firstprivat not alloc for ptr attach for arrays

For a non-descriptor array,  map(A(n:m)) was mapped as
  map(tofrom:A[n-1] [len: ...]) map(alloc:A [pointer assign, bias: ...])
with this patch, it is changed to
  map(tofrom:A[n-1] [len: ...]) map(firstprivate:A [pointer assign, bias: ...])

The latter avoids an alloc - and also avoids the race condition with
nowait in the enclosed testcase. (Note: predantically, the testcase is
invalid since OpenMP 5.1, violating the map clause restriction at [354:10-13].

gcc/fortran/ChangeLog:

	* trans-openmp.cc (gfc_trans_omp_clauses): When mapping nondescriptor
	array sections, use GOMP_MAP_FIRSTPRIVATE_POINTER instead of
	GOMP_MAP_POINTER for the pointer attachment.

libgomp/ChangeLog:

	* testsuite/libgomp.fortran/target-nowait-array-section.f90: New test.

 gcc/fortran/trans-openmp.cc| 12 +++--
 .../target-nowait-array-section.f90| 56 ++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index baa45f78a0e..eb5870c3bc5 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -3312,9 +3312,15 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
 		  /* An array element or array section which is not part of a
 		 derived type, etc.  */
 		  bool element = n->expr->ref->u.ar.type == AR_ELEMENT;
-		  gfc_trans_omp_array_section (block, n, decl, element,
-	   GOMP_MAP_POINTER, node, node2,
-	   node3, node4);
+		  tree type = TREE_TYPE (decl);
+		  gomp_map_kind k = GOMP_MAP_POINTER;
+		  if (!openacc
+		  && !GFC_DESCRIPTOR_TYPE_P (type)
+		  && !(POINTER_TYPE_P (type)
+			   && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (type
+		k = GOMP_MAP_FIRSTPRIVATE_POINTER;
+		  gfc_trans_omp_array_section (block, n, decl, element, k,
+	   node, node2, node3, node4);
 		}
 	  else if (n->expr
 		   && n->expr->expr_type == EXPR_VARIABLE
diff --git a/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90 b/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90
new file mode 100644
index 000..7560cff746b
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90
@@ -0,0 +1,56 @@
+! Runs the the target region asynchrolously and checks for it
+!
+! Note that  map(alloc: work(:, i)) + nowait  should be save
+! given that a nondescriptor array is used. However, it still
+! violates a map clause restriction, added in OpenMP 5.1 [354:10-13].
+
+PROGRAM test_target_teams_distribute_nowait
+  USE ISO_Fortran_env, only: INT64
+  implicit none
+INTEGER, parameter :: N = 1024, N_TASKS = 16
+INTEGER :: i, j, k, my_ticket
+INTEGER :: order(n_tasks)
+INTEGER(INT64) :: work(n, n_tasks)
+INTEGER :: ticket
+logical :: async
+
+ticket = 0
+
+!$omp target enter data map(to: ticket, order)
+
+!$omp parallel do num_threads(n_tasks)
+DO i = 1, n_tasks
+   !$omp target map(alloc: work(:, i), ticket) private(my_ticket) nowait
+   !!$omp target teams distribute map(alloc: work(:, i), ticket) private(my_ticket) nowait
+   DO j = 1, n
+  ! Waste cyles
+!  work(j, i) = 0
+!  DO k = 1, n*(n_tasks - i)
+! work(j, i) = work(j, i) + i*j*k
+!  END DO
+  my_ticket = 0
+  !$omp atomic capture
+  ticket = ticket + 1
+  my_ticket = ticket
+  !$omp end atomic
+  !$omp atomic write
+  order(i) = my_ticket
+   END DO
+   !$omp end target !teams distribute
+END DO
+!$omp end parallel do
+
+!$omp target exit data map(from:ticket, order)
+
+IF (ticket .ne. n_tasks*n) s

Re: [Patch] OpenMP/Fortran: Use firstprivat not alloc for ptr attach for arrays

[Jakub Jelinek via Fortran]

On Fri, May 13, 2022 at 07:21:02PM +0200, Tobias Burnus wrote:
> gcc/fortran/ChangeLog:
> 
>   * trans-openmp.cc (gfc_trans_omp_clauses): When mapping nondescriptor
>   array sections, use GOMP_MAP_FIRSTPRIVATE_POINTER instead of
>   GOMP_MAP_POINTER for the pointer attachment.
> 
> libgomp/ChangeLog:
> 
>   * testsuite/libgomp.fortran/target-nowait-array-section.f90: New test.

Not 100% sure if we want to add such a testcase into the testsuite given
that it is not valid OpenMP, but perhaps it is ok as we are testing a QoI.

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90
> @@ -0,0 +1,56 @@
> +! Runs the the target region asynchrolously and checks for it
> +!
> +! Note that  map(alloc: work(:, i)) + nowait  should be save

s/save/safe/

Otherwise LGTM.

Jakub

[Patch] OpenMP: Add omp_all_memory support to Fortran

[Tobias Burnus]

This adds omp_all_memory handling to Fortran, following C/C++ and shamelessly 
coping
the C testcases and adapting them to Fortran.

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: Add omp_all_memory support to Fortran

Fortran part to the C/C++/backend implementation
r13-337-g7f78783dbedca0183d193e475262ca3c489fd365

gcc/fortran/ChangeLog:

	* dump-parse-tree.cc (show_omp_namelist): Handle omp_all_memory.
	* openmp.cc (gfc_match_omp_variable_list, gfc_match_omp_depend_sink,
	gfc_match_omp_clauses, resolve_omp_clauses): Likewise.
	* trans-openmp.cc (gfc_trans_omp_clauses, gfc_trans_omp_depobj):
	Likewise.
	* resolve.cc (resolve_symbol): Reject it as symbol.

libgomp/ChangeLog:

	* libgomp.texi (OpenMP 5.1): Set omp_all_memory to 'Y'.
	* testsuite/libgomp.fortran/depend-5.f90: New test.
	* testsuite/libgomp.fortran/depend-6.f90: New test.
	* testsuite/libgomp.fortran/depend-7.f90: New test.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/all-memory-1.f90: New test.
	* gfortran.dg/gomp/all-memory-2.f90: New test.
	* gfortran.dg/gomp/all-memory-3.f90: New test.

 gcc/fortran/dump-parse-tree.cc  |   2 +-
 gcc/fortran/openmp.cc   |  79 ---
 gcc/fortran/resolve.cc  |   7 ++
 gcc/fortran/trans-openmp.cc |  10 +-
 gcc/testsuite/gfortran.dg/gomp/all-memory-1.f90 |  51 ++
 gcc/testsuite/gfortran.dg/gomp/all-memory-2.f90 |  52 ++
 gcc/testsuite/gfortran.dg/gomp/all-memory-3.f90 |  24 +
 libgomp/libgomp.texi|   4 +-
 libgomp/testsuite/libgomp.fortran/depend-5.f90  | 121 +++
 libgomp/testsuite/libgomp.fortran/depend-6.f90  | 126 
 libgomp/testsuite/libgomp.fortran/depend-7.f90  | 113 +
 11 files changed, 567 insertions(+), 22 deletions(-)

diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc
index 3635460bffd..a32992035a2 100644
--- a/gcc/fortran/dump-parse-tree.cc
+++ b/gcc/fortran/dump-parse-tree.cc
@@ -1423,7 +1423,7 @@ show_omp_namelist (int list_type, gfc_omp_namelist *n)
 	  case OMP_LINEAR_UVAL: fputs ("uval(", dumpfile); break;
 	  default: break;
 	  }
-  fprintf (dumpfile, "%s", n->sym->name);
+  fprintf (dumpfile, "%s", n->sym ? n->sym->name : "omp_all_memory");
   if (list_type == OMP_LIST_LINEAR && n->u.linear_op != OMP_LINEAR_DEFAULT)
 	fputc (')', dumpfile);
   if (n->expr)
diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index 4d3fcc8bb78..4df1f761c5a 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -296,14 +296,17 @@ gfc_find_omp_udr (gfc_namespace *ns, const char *name, gfc_typespec *ts)
 }
 
 
-/* Match a variable/common block list and construct a namelist from it.  */
+/* Match a variable/common block list and construct a namelist from it;
+   if has_all_memory != NULL, *has_all_memory is set and omp_all_memory
+   yields a list->sym NULL entry. */
 
 static match
 gfc_match_omp_variable_list (const char *str, gfc_omp_namelist **list,
 			 bool allow_common, bool *end_colon = NULL,
 			 gfc_omp_namelist ***headp = NULL,
 			 bool allow_sections = false,
-			 bool allow_derived = false)
+			 bool allow_derived = false,
+			 bool *has_all_memory = NULL)
 {
   gfc_omp_namelist *head, *tail, *p;
   locus old_loc, cur_loc;
@@ -315,7 +318,8 @@ gfc_match_omp_variable_list (const char *str, gfc_omp_namelist **list,
   head = tail = NULL;
 
   old_loc = gfc_current_locus;
-
+  if (has_all_memory)
+*has_all_memory = false;
   m = gfc_match (str);
   if (m != MATCH_YES)
 return m;
@@ -323,7 +327,35 @@ gfc_match_omp_variable_list (const char *str, gfc_omp_namelist **list,
   for (;;)
 {
   cur_loc = gfc_current_locus;
-  m = gfc_match_symbol (&sym, 1);
+
+  m = gfc_match_name (n);
+  if (m == MATCH_YES && strcmp (n, "omp_all_memory") == 0)
+	{
+	  if (!has_all_memory)
+	{
+	  gfc_error ("% at %C not permitted in this "
+			 "clause");
+	  goto cleanup;
+	}
+	  *has_all_memory = true;
+	  p = gfc_get_omp_namelist ();
+	  if (head == NULL)
+	head = tail = p;
+	  else
+	{
+	  tail->next = p;
+	  tail = tail->next;
+	}
+	  tail->where = cur_loc;
+	  goto next_item;
+	}
+  if (m == MATCH_YES)
+	{
+	  gfc_symtree *st;
+	  if ((m = gfc_get_ha_sym_tree (n, &st) ? MATCH_ERROR : MATCH_YES)
+	  == MATCH_YES)
+	sym = st->n.sym;
+	}
   switch (m)
 	{
 	case MATCH_YES:
@@ -578,6 +610,12 @@ gfc_match_omp_depend_sink (gfc_omp_namelist **list)
 	  tail->sym = sym;
 	  tail->expr = NULL;
 	  tail->where = cur_loc;
+	  if (UNLIKELY (strcmp (sym->name, "omp_all_memory") == 0))
+	{
+	  gfc_error ("% used with DEPEND kind 

Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD

[Piotr Kubaj via Fortran]

I'm abandoning this patch. It was fixed in FreeBSD instead to have 
feenableexcept() in libm in 
https://cgit.freebsd.org/src/commit/?id=448c505c33cc334193590f3844406d6a74f26e2a

Thanks for your insight!

On 22-05-13 10:59:59, Kewen.Lin wrote:
> on 2022/5/13 04:16, Segher Boessenkool wrote:
> > Hi Piotr,
> > 
> > On Tue, May 03, 2022 at 12:21:12PM +0200, pku...@freebsd.org wrote:
> >> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header.
> > 
> > Declared, not defined.  These are required to be real functions (on all
> > platforms that have these functions), not macros or inlines or whatever.
> > 
> 
> Piotr's reply "FreeBSD doesn't have this function in libm, it's
> implemented in /usr/include/fenv.h." from [1] made me feel like
> it's a definition instead of declaration.  So I thought the check
> should use AC_LINK_IFELSE instead, since one fenv.h which doesn't
> have the definition can still pass the proposed AC_COMPILE_IFELSE
> check.
> 
> I just did a further search, the powerpc fenv.h [2] does include
> the definition of feenableexcept.  By comparison, the x86 fenv.h [3]
> doesn't.  But I'm not sure if it's the same as what Piotr's
> environments have.  Hope it's similar. :-)
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593193.html
> [2] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/powerpc/fenv.h
> [3] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/x86/fenv.h
> 
> BR,
> Kewen
> 


signature.asc
Description: PGP signature