On Mon, Nov 09, 2020 at 05:43:20PM +0000, Kwok Cheung Yeung wrote:
> commit b4feb16f3c84b8f82163a4cbba6a31d55fbb8e5b
> Author: Kwok Cheung Yeung <k...@codesourcery.com>
> Date: Mon Nov 9 09:34:39 2020 -0800
>
> openmp: Retire nest-var ICV for OpenMP 5.0
>
> This removes the nest-var ICV, expressing nesting in terms of the
> max-active-levels-var ICV instead.
>
> 2020-11-09 Kwok Cheung Yeung <k...@codesourcery.com>
>
> libgomp/
> * env.c (gomp_global_icv): Remove nest_var field.
> (gomp_max_active_levels_var): Initialize to 1.
> (parse_boolean): Return true on success.
> (handle_omp_display_env): Express OMP_NESTED in terms of
> gomp_max_active_levels_var.
> (initialize_env): Set gomp_max_active_levels_var from
> OMP_MAX_ACTIVE_LEVELS, OMP_NESTED, OMP_NUM_THREADS and
> OMP_PROC_BIND.
> * icv.c (omp_set_nested): Express in terms of
> gomp_max_active_levels_var.
> (omp_get_nested): Likewise.
> * libgomp.h (struct gomp_task_icv): Remove nest_var field.
> * libgomp.texi (omp_get_nested): Update documentation.
> (omp_set_nested): Likewise.
> (OMP_MAX_ACTIVE_LEVELS): Likewise.
> (OMP_NESTED): Likewise.
> (OMP_NUM_THREADS): Likewise.
> (OMP_PROC_BIND): Likewise.
> * parallel.c (gomp_resolve_num_threads): Replace reference
> to nest_var with gomp_max_active_levels_var.
> * testsuite/libgomp.c/target-5.c: Remove additional options.
> (main): Remove references to omp_get_nested and omp_set_nested.
One thing is that max-active-levels-var in 5.0 is per-device,
but in 5.1 per-data environment. The question is if we should implement
the problematic 5.0 way or the 5.1 one. E.g.:
#include <omp.h>
#include <stdio.h>
int
main ()
{
#pragma omp parallel
{
omp_set_nested (1);
#pragma omp parallel num_threads(2)
printf ("Hello, world!\n");
}
}
which used to be valid in 4.5 (where nest-var used to be per-data
environment) is in 5.0 racy (and in 5.1 will not be racy again).
Though, as these are deprecated APIs, perhaps we can just do the 5.0 way for
now.
> --- a/libgomp/libgomp.texi
> +++ b/libgomp/libgomp.texi
> @@ -489,8 +489,11 @@ represent their language-specific counterparts.
>
> Nested parallel regions may be initialized at startup by the
> @env{OMP_NESTED} environment variable or at runtime using
> -@code{omp_set_nested}. If undefined, nested parallel regions are
> -disabled by default.
> +@code{omp_set_nested}. Setting the maximum number of nested
> +regions to above one using the @env{OMP_MAX_ACTIVE_LEVELS}
> +environment variable or @code{omp_set_max_active_levels} will
> +also enable nesting. If undefined, nested parallel regions
> +are disabled by default.
This doesn't really describe what env.c does. If undefined, then
if OMP_NESTED is defined, it will be folloed, and if neither is
defined, the code sets the default based on
"OMP_NUM_THREADS or OMP_PROC_BIND is set to a
comma-separated list of more than one value"
as the spec says and only is disabled otherwise.
> @@ -1502,10 +1511,11 @@ disabled by default.
> @item @emph{Description}:
> Specifies the initial value for the maximum number of nested parallel
> regions. The value of this variable shall be a positive integer.
> -If undefined, the number of active levels is unlimited.
> +If undefined, the number of active levels is one, which effectively
> +disables nested regions.
Similarly.
>
> @item @emph{See also}:
> -@ref{omp_set_max_active_levels}
> +@ref{omp_set_max_active_levels}, @ref{OMP_NESTED}
>
> @item @emph{Reference}:
> @uref{https://www.openmp.org, OpenMP specification v4.5}, Section 4.9
> @@ -1541,11 +1551,13 @@ integer, and zero is allowed. If undefined, the
> default priority is
> @item @emph{Description}:
> Enable or disable nested parallel regions, i.e., whether team members
> are allowed to create new teams. The value of this environment variable
> -shall be @code{TRUE} or @code{FALSE}. If undefined, nested parallel
> -regions are disabled by default.
> +shall be @code{TRUE} or @code{FALSE}. If set to @code{TRUE}, the number
> +of maximum active nested regions supported will by default be set to the
> +maximum supported, otherwise it will be set to one. If undefined, nested
> +parallel regions are disabled by default.
Again.
> --- a/libgomp/testsuite/libgomp.c/target-5.c
> +++ b/libgomp/testsuite/libgomp.c/target-5.c
> @@ -1,5 +1,3 @@
> -/* { dg-additional-options "-Wno-deprecated-declarations" } */
> -
> #include <omp.h>
> #include <stdlib.h>
>
> @@ -7,17 +5,14 @@ int
> main ()
> {
> int d_o = omp_get_dynamic ();
> - int n_o = omp_get_nested ();
> omp_sched_t s_o;
> int c_o;
> omp_get_schedule (&s_o, &c_o);
> int m_o = omp_get_max_threads ();
> omp_set_dynamic (1);
> - omp_set_nested (1);
> omp_set_schedule (omp_sched_static, 2);
> omp_set_num_threads (4);
> int d = omp_get_dynamic ();
> - int n = omp_get_nested ();
> omp_sched_t s;
> int c;
> omp_get_schedule (&s, &c);
> @@ -30,13 +25,11 @@ main ()
> int c_c;
> omp_get_schedule (&s_c, &c_c);
> if (d_o != omp_get_dynamic ()
> - || n_o != omp_get_nested ()
> || s_o != s_c
> || c_o != c_c
> || m_o != omp_get_max_threads ())
> abort ();
> omp_set_dynamic (0);
> - omp_set_nested (0);
> omp_set_schedule (omp_sched_dynamic, 4);
> omp_set_num_threads (2);
> if (!omp_is_initial_device ())
> @@ -48,7 +41,6 @@ main ()
> int c_c;
> omp_get_schedule (&s_c, &c_c);
> if (d != omp_get_dynamic ()
> - || n != omp_get_nested ()
> || s != s_c
> || c != c_c
> || m != omp_get_max_threads ())
> @@ -60,13 +52,11 @@ main ()
> int c_c;
> omp_get_schedule (&s_c, &c_c);
> if (d_o != omp_get_dynamic ()
> - || n_o != omp_get_nested ()
> || s_o != s_c
> || c_o != c_c
> || m_o != omp_get_max_threads ())
> abort ();
> omp_set_dynamic (0);
> - omp_set_nested (0);
> omp_set_schedule (omp_sched_dynamic, 4);
> omp_set_num_threads (2);
> if (!omp_is_initial_device ())
> @@ -76,7 +66,6 @@ main ()
> abort ();
> omp_get_schedule (&s_c, &c_c);
> if (d != omp_get_dynamic ()
> - || n != omp_get_nested ()
> || s != s_c
> || c != c_c
> || m != omp_get_max_threads ())
Why does this testcase need updates?
It doesn't seem to use omp_[sg]et_max_active_levels and so I don't see
why it couldn't use omp_[sg]et_nested.
Jakub
