Re: [PATCH] OpenMP, Fortran: Bugfix for omp_set_num_teams.

[Marcel Vollweiler]

Hi Jakub,


! { dg-do run }
! { dg-additional-options "-fdefault-integer-8" }

program set_num_teams_8
   use omp_lib
   omp_set_num_teams (42)
   if (omp_get_num_teams () .ne. 42) stop 1
end program


I modified your suggested test case a bit:

program set_num_teams_8
  use omp_lib
  use, intrinsic :: iso_fortran_env
  integer(int64) :: x
  x = 42
  call omp_set_num_teams (x)
  if (omp_get_max_teams () .ne. 42) stop 1
end program

I tested it with/without the fix and the test passed/failed as expected.

Hope, that's ok?

Marcel
-
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: Bugfix for omp_set_num_teams.

This patch fixes a small bug in the omp_set_num_teams implementation.

libgomp/ChangeLog:

* fortran.c (omp_set_num_teams_8_): Fix bug.
* testsuite/libgomp.fortran/icv-8.f90: New test.

diff --git a/libgomp/fortran.c b/libgomp/fortran.c
index 8c1cfd1..d984ce5 100644
--- a/libgomp/fortran.c
+++ b/libgomp/fortran.c
@@ -491,7 +491,7 @@ omp_set_num_teams_ (const int32_t *num_teams)
 void
 omp_set_num_teams_8_ (const int64_t *num_teams)
 {
-  omp_set_max_active_levels (TO_INT (*num_teams));
+  omp_set_num_teams (TO_INT (*num_teams));
 }
 
 int32_t
diff --git a/libgomp/testsuite/libgomp.fortran/icv-8.f90 
b/libgomp/testsuite/libgomp.fortran/icv-8.f90
new file mode 100644
index 000..9478c15
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/icv-8.f90
@@ -0,0 +1,10 @@
+! This tests 'set_num_teams_8' function.
+
+program set_num_teams_8
+  use omp_lib
+  use, intrinsic :: iso_fortran_env
+  integer(int64) :: x
+  x = 42
+  call omp_set_num_teams (x)
+  if (omp_get_max_teams () .ne. 42) stop 1
+end program

Re: [PATCH] OpenMP, Fortran: Bugfix for omp_set_num_teams.

[Jakub Jelinek via Fortran]

On Wed, Mar 16, 2022 at 02:06:16PM +0100, Marcel Vollweiler wrote:
> libgomp/ChangeLog:
> 
>   * fortran.c (omp_set_num_teams_8_): Fix bug.
>   * testsuite/libgomp.fortran/icv-8.f90: New test.

Ok, with a minor nit.  Please use
Call omp_set_num_teams instead of omp_set_max_active_levels.
instead of
Fix bug.
in the ChangeLog.

> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
> index 8c1cfd1..d984ce5 100644
> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c
> @@ -491,7 +491,7 @@ omp_set_num_teams_ (const int32_t *num_teams)
>  void
>  omp_set_num_teams_8_ (const int64_t *num_teams)
>  {
> -  omp_set_max_active_levels (TO_INT (*num_teams));
> +  omp_set_num_teams (TO_INT (*num_teams));
>  }
>  
>  int32_t
> diff --git a/libgomp/testsuite/libgomp.fortran/icv-8.f90 
> b/libgomp/testsuite/libgomp.fortran/icv-8.f90
> new file mode 100644
> index 000..9478c15
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/icv-8.f90
> @@ -0,0 +1,10 @@
> +! This tests 'set_num_teams_8' function.
> +
> +program set_num_teams_8
> +  use omp_lib
> +  use, intrinsic :: iso_fortran_env
> +  integer(int64) :: x
> +  x = 42
> +  call omp_set_num_teams (x)
> +  if (omp_get_max_teams () .ne. 42) stop 1
> +end program


Jakub

Re: [PATCH] fortran: Separate associate character lengths earlier [PR104570]

[Harald Anlauf via Fortran]

Hi Mikael,

Am 14.03.22 um 19:28 schrieb Mikael Morin:

Hello,

this workarounds the regression I introduced with the fix for pr104228.
The problem comes from the evaluation of character length for the
associate target (y) in the testcase.
The expression is non-scalar which is not supported at that point, as no
scalarization setup is made there.

My initial direction to fix this was trying to modify the evaluation of
expressions so that the rank was ignored in the context of character
length evaluation.
That’s the patch I attached to the PR.
The patch I’m proposing here tries instead to avoid the need to evaluate
character length through sharing of gfc_charlen between symbols and
expressions.
I slightly prefer the latter direction, though I’m not sure it’s more
robust. > At least it passes regression testing, which proves some basic level 
of
goodness.
OK for master?


I tried a few minor variations of the testcase, and they
also seemed to work.  The patch looks simple enough, so
OK from my side.


Even if PR104570 is a 12 regression only, the PR104228 fix is targeted
at all the open branches so this one as well.
OK for 11/10/9?


Yes, after giving it a short time on master to "burn in".

Thanks for the patch!

Harald