On Thu, Nov 12, 2020 at 10:44:35PM +0000, Kwok Cheung Yeung wrote:
> + /* OMP_NESTED is deprecated in OpenMP 5.0. */
> + if (parse_boolean ("OMP_NESTED", &nested))
> + gomp_global_icv.max_active_levels_var =
> + nested ? gomp_supported_active_levels : 1;
Formatting - = should be on the next line, indented 2 columns further from
gomp_global_icv.
> int
> omp_get_nested (void)
> {
> struct gomp_task_icv *icv = gomp_icv (false);
> - return icv->nest_var;
> + return icv->max_active_levels_var > 1
> + && icv->max_active_levels_var > omp_get_active_level ();
Formatting, should be:
return (icv->max_active_levels_var > 1
&& icv->max_active_levels_var > omp_get_active_level ());
> @@ -118,19 +122,21 @@ omp_get_thread_limit (void)
> void
> omp_set_max_active_levels (int max_levels)
> {
> + struct gomp_task_icv *icv = gomp_icv (false);
Should be gomp_icv (true), because it modifies the ICVs rather than
just querying them. And perhaps move it inside of the if (max_levels >= 0)
if.
> if (max_levels >= 0)
> {
> if (max_levels <= gomp_supported_active_levels)
> - gomp_max_active_levels_var = max_levels;
> + icv->max_active_levels_var = max_levels;
> else
> - gomp_max_active_levels_var = gomp_supported_active_levels;
> + icv->max_active_levels_var = gomp_supported_active_levels;
> }
> }
> --- a/libgomp/libgomp.h
> +++ b/libgomp/libgomp.h
> @@ -428,7 +428,7 @@ struct gomp_task_icv
> int default_device_var;
> unsigned int thread_limit_var;
> bool dyn_var;
> - bool nest_var;
> + unsigned long max_active_levels_var;
> char bind_var;
> /* Internal ICV. */
> struct target_mem_desc *target_data;
This is in __thread vars, so we can't waste space in useless paddings or
overlong fields.
On 64-bit arches, thread_limit_var ends on 64-bit boundary and target_data
starts at that boundary, so with bool; unsigned long; char in between
that means 24 bytes in between them including 14 bytes of padding.
There is no point to make max_active_levels_var unsigned long,
gomp_supported_active_levels is INT_MAX, so one can't have anything higher
than that anyway. So, either
bool dyn_var;
char bind_var;
int max_active_levels_var;
which will be 8 bytes together on both 64-bit and 32-bit arches, or
perhaps we could go even further and lower the max active levels to USHRT_MAX
and use unsigned short, 65535 nesting levels is many thousands more than
what a reasonable program should try, after all, only active levels (aka
where there are 2 or more threads) count, so already 32 active levels
when each level has 2 threads means 4 billion threads, and 64 active levels
means 18 quintillion threads, so even 64 active levels are impossible
in 64-bit address spaces even if each thread occupied just 1 byte rather
than several pages.
So, let's change gomp_supported_active_levels to say 255 and use
bool dyn_var;
unsigned char max_active_levels_var;
char bind_var;
Jakub
