OpenMP 5.1: omp_display_env

OpenMP 5.1: omp_display_env

[Ulrich Drepper]

I know OpenMP 5.1 is not really a focus yet but adding this new interface
should not be problematic.  I stumbled across this part of the spec and the
functionality is really already mostly there in the form of
OMP_DISPLAY_ENV=verbose etc.  This is just a function interface to the same
functionality.

Aside from the busywork to add a new interface (headers, map file) the only
real question was how to deal with the two parameters which are passed
to handle_omp_display_env in the current implementation. The
omp_display_env interface is supposed to show the information of the
initial values and therefore I think the right implementation is to store
the values determined in the constructor in two global, static variables
and reuse them.

The rest should be completely boring and therefore not distracting anyone
from OpenMP 5.0 work.

OK to commit?

diff --git a/libgomp/env.c b/libgomp/env.c
index a24deabcd58..84038eb4683 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -99,6 +99,9 @@ int goacc_default_dims[GOMP_DIM_MAX];

 #ifndef LIBGOMP_OFFLOADED_ONLY

+static int wait_policy;
+static unsigned long stacksize = GOMP_DEFAULT_STACKSIZE;
+
 /* Parse the OMP_SCHEDULE environment variable.  */

 static void
@@ -1210,46 +1213,11 @@ parse_gomp_openacc_dim (void)
     }
 }

-static void
-handle_omp_display_env (unsigned long stacksize, int wait_policy)
+void
+omp_display_env (int verbose)
 {
-  const char *env;
-  bool display = false;
-  bool verbose = false;
   int i;

-  env = getenv ("OMP_DISPLAY_ENV");
-  if (env == NULL)
-    return;
-
-  while (isspace ((unsigned char) *env))
-    ++env;
-  if (strncasecmp (env, "true", 4) == 0)
-    {
-      display = true;
-      env += 4;
-    }
-  else if (strncasecmp (env, "false", 5) == 0)
-    {
-      display = false;
-      env += 5;
-    }
-  else if (strncasecmp (env, "verbose", 7) == 0)
-    {
-      display = true;
-      verbose = true;
-      env += 7;
-    }
-  else
-    env = "X";
-  while (isspace ((unsigned char) *env))
-    ++env;
-  if (*env != '\0')
-    gomp_error ("Invalid value for environment variable OMP_DISPLAY_ENV");
-
-  if (!display)
-    return;
-
   fputs ("\nOPENMP DISPLAY ENVIRONMENT BEGIN\n", stderr);

   fputs ("  _OPENMP = '201511'\n", stderr);
@@ -1409,13 +1377,52 @@ handle_omp_display_env (unsigned long stacksize,
int wait_policy)
   fputs ("OPENMP DISPLAY ENVIRONMENT END\n", stderr);
 }

+static void
+handle_omp_display_env (void)
+{
+  const char *env;
+  bool display = false;
+  bool verbose = false;
+
+  env = getenv ("OMP_DISPLAY_ENV");
+  if (env == NULL)
+    return;
+
+  while (isspace ((unsigned char) *env))
+    ++env;
+  if (strncasecmp (env, "true", 4) == 0)
+    {
+      display = true;
+      env += 4;
+    }
+  else if (strncasecmp (env, "false", 5) == 0)
+    {
+      display = false;
+      env += 5;
+    }
+  else if (strncasecmp (env, "verbose", 7) == 0)
+    {
+      display = true;
+      verbose = true;
+      env += 7;
+    }
+  else
+    env = "X";
+  while (isspace ((unsigned char) *env))
+    ++env;
+  if (*env != '\0')
+    gomp_error ("Invalid value for environment variable OMP_DISPLAY_ENV");
+
+  if (display)
+    omp_display_env (verbose);
+}
+

 static void __attribute__((constructor))
 initialize_env (void)
 {
-  unsigned long thread_limit_var, stacksize = GOMP_DEFAULT_STACKSIZE;
+  unsigned long thread_limit_var;
   unsigned long max_active_levels_var;
-  int wait_policy;

   /* Do a compile time check that mkomp_h.pl did good job.  */
   omp_check_defines ();
@@ -1546,7 +1553,7 @@ initialize_env (void)
  gomp_error ("Stack size change failed: %s", strerror (err));
     }

-  handle_omp_display_env (stacksize, wait_policy);
+  handle_omp_display_env ();

   /* OpenACC.  */

diff --git a/libgomp/fortran.c b/libgomp/fortran.c
index 4ec39c4e61b..2f36e326624 100644
--- a/libgomp/fortran.c
+++ b/libgomp/fortran.c
@@ -736,3 +736,9 @@ omp_get_default_allocator_ ()
 {
   return (intptr_t) omp_get_default_allocator ();
 }
+
+void
+omp_display_env_ (const int32_t *verbose)
+{
+  omp_display_env (*verbose);
+}
diff --git a/libgomp/libgomp.map b/libgomp/libgomp.map
index 8ea27b5565f..fe51d3dbd46 100644
--- a/libgomp/libgomp.map
+++ b/libgomp/libgomp.map
@@ -199,6 +199,12 @@ OMP_5.0.1 {
  omp_fulfill_event_;
 } OMP_5.0;

+OMP_5.1 {
+  global:
+ omp_display_env;
+ omp_display_env_;
+} OMP_5.0.1;
+
 GOMP_1.0 {
   global:
  GOMP_atomic_end;
diff --git a/libgomp/omp.h.in b/libgomp/omp.h.in
index 69f96f09124..c93db968d2e 100644
--- a/libgomp/omp.h.in
+++ b/libgomp/omp.h.in
@@ -293,6 +293,8 @@ extern void omp_free (void *,
       omp_allocator_handle_t __GOMP_DEFAULT_NULL_ALLOCATOR)
   __GOMP_NOTHROW;

+extern void omp_display_env (int) __GOMP_NOTHROW;
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libgomp/omp_lib.f90.in b/libgomp/omp_lib.f90.in
index 851f85f5316..4939bfd9751 100644
--- a/libgomp/omp_lib.f90.in
+++ b/libgomp/omp_lib.f90.in
@@ -653,6 +653,12 @@
           end function
         end interface

+        interface
+          subroutine omp_display_env (verbose)
+            logical,intent(in) :: verbose
+          end subroutine
+        end interface
+
 #if _OPENMP >= 201811
 !GCC$ ATTRIBUTES DEPRECATED :: omp_get_nested, omp_set_nested
 #endif
diff --git a/libgomp/omp_lib.h.in b/libgomp/omp_lib.h.in
index 06d17b5fcdc..9873cea9ac1 100644
--- a/libgomp/omp_lib.h.in
+++ b/libgomp/omp_lib.h.in
@@ -264,3 +264,5 @@
       external omp_set_default_allocator
       external omp_get_default_allocator
       integer (omp_allocator_handle_kind) omp_get_default_allocator
+
+      external omp_display_env

Re: OpenMP 5.1: omp_display_env

[Jakub Jelinek]

On Tue, Jul 27, 2021 at 04:26:22PM +0200, Ulrich Drepper via Gcc-patches wrote:
> I know OpenMP 5.1 is not really a focus yet but adding this new interface
> should not be problematic.  I stumbled across this part of the spec and the
> functionality is really already mostly there in the form of
> OMP_DISPLAY_ENV=verbose etc.  This is just a function interface to the same
> functionality.
> 
> Aside from the busywork to add a new interface (headers, map file) the only
> real question was how to deal with the two parameters which are passed
> to handle_omp_display_env in the current implementation. The
> omp_display_env interface is supposed to show the information of the
> initial values and therefore I think the right implementation is to store
> the values determined in the constructor in two global, static variables
> and reuse them.
> 
> The rest should be completely boring and therefore not distracting anyone
> from OpenMP 5.0 work.

Thanks.
You'll need to write a ChangeLog entry and put it at the end of the commit
message, otherwise pre-commit hooks will reject the commit.

--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -1210,46 +1213,11 @@ parse_gomp_openacc_dim (void)
     }
 }

-static void
-handle_omp_display_env (unsigned long stacksize, int wait_policy)
+void
+omp_display_env (int verbose)

Please add
ialias (omp_display_env)
right after the function definition, we don't want to introduce new
PLT slots unnecessarily and omp_display_env is an exported function.

> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c

And ialias_redirect (omp_display_env) here.

> @@ -736,3 +736,9 @@ omp_get_default_allocator_ ()
>  {
>    return (intptr_t) omp_get_default_allocator ();
>  }
> +
> +void
> +omp_display_env_ (const int32_t *verbose)
> +{
> +  omp_display_env (*verbose);
> +}

For Fortran functions/subroutines that take integer arguments
libgomp typically defines two functions, one with _ suffix
and one with _8_ suffix, the former taking const int32_t *
and the latter const int64_t * and using !! for logicals
and TO_INT macro for integers.
Please grep e.g. for omp_set_dynamic in
fortran.c, libgomp.map and omp_lib.f90.in.
This is needed to make calls to those functions work even with
-fdefault-integer-8

> --- a/libgomp/omp_lib.f90.in
> +++ b/libgomp/omp_lib.f90.in
> @@ -653,6 +653,12 @@
>            end function
>          end interface
> 
> +        interface
> +          subroutine omp_display_env (verbose)
> +            logical,intent(in) :: verbose
> +          end subroutine
> +        end interface

See above.  Plus, please add space between comma and intent(in).

Otherwise LGTM.

	Jakub

Re: OpenMP 5.1: omp_display_env

[Tobias Burnus]

Hi Ulrich, hi all,

this patch breaks offloading. The reason is that most code
in env.c is enclosed in:

#ifndef LIBGOMP_OFFLOADED_ONLY
...
#endif /* LIBGOMP_OFFLOADED_ONLY */

But omp_display_env_ / omp_display_env_8_ in fortran.c is not.
Hence, there is a link error for  omp_display_env (e.g. nvptx)
or gomp_ialias_omp_display_env  (gcn).

Tobias


On 27.07.21 17:05, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Jul 27, 2021 at 04:26:22PM +0200, Ulrich Drepper via Gcc-patches wrote:
>> I know OpenMP 5.1 is not really a focus yet but adding this new interface
>> should not be problematic.  I stumbled across this part of the spec and the
>> functionality is really already mostly there in the form of
>> OMP_DISPLAY_ENV=verbose etc.  This is just a function interface to the same
>> functionality.
>>
>> Aside from the busywork to add a new interface (headers, map file) the only
>> real question was how to deal with the two parameters which are passed
>> to handle_omp_display_env in the current implementation. The
>> omp_display_env interface is supposed to show the information of the
>> initial values and therefore I think the right implementation is to store
>> the values determined in the constructor in two global, static variables
>> and reuse them.
>>
>> The rest should be completely boring and therefore not distracting anyone
>> from OpenMP 5.0 work.
> Thanks.
> You'll need to write a ChangeLog entry and put it at the end of the commit
> message, otherwise pre-commit hooks will reject the commit.
>
> --- a/libgomp/env.c
> +++ b/libgomp/env.c
> @@ -1210,46 +1213,11 @@ parse_gomp_openacc_dim (void)
>       }
>   }
>
> -static void
> -handle_omp_display_env (unsigned long stacksize, int wait_policy)
> +void
> +omp_display_env (int verbose)
>
> Please add
> ialias (omp_display_env)
> right after the function definition, we don't want to introduce new
> PLT slots unnecessarily and omp_display_env is an exported function.
>
>> --- a/libgomp/fortran.c
>> +++ b/libgomp/fortran.c
> And ialias_redirect (omp_display_env) here.
>
>> @@ -736,3 +736,9 @@ omp_get_default_allocator_ ()
>>   {
>>     return (intptr_t) omp_get_default_allocator ();
>>   }
>> +
>> +void
>> +omp_display_env_ (const int32_t *verbose)
>> +{
>> +  omp_display_env (*verbose);
>> +}
> For Fortran functions/subroutines that take integer arguments
> libgomp typically defines two functions, one with _ suffix
> and one with _8_ suffix, the former taking const int32_t *
> and the latter const int64_t * and using !! for logicals
> and TO_INT macro for integers.
> Please grep e.g. for omp_set_dynamic in
> fortran.c, libgomp.map and omp_lib.f90.in.
> This is needed to make calls to those functions work even with
> -fdefault-integer-8
>
>> --- a/libgomp/omp_lib.f90.in
>> +++ b/libgomp/omp_lib.f90.in
>> @@ -653,6 +653,12 @@
>>             end function
>>           end interface
>>
>> +        interface
>> +          subroutine omp_display_env (verbose)
>> +            logical,intent(in) :: verbose
>> +          end subroutine
>> +        end interface
> See above.  Plus, please add space between comma and intent(in).
>
> Otherwise LGTM.
>
>       Jakub
>
-----------------
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

Re: OpenMP 5.1: omp_display_env

[Ulrich Drepper]

Hi,

On Fri, Jul 30, 2021 at 9:50 AM Tobias Burnus <tobias@codesourcery.com>
wrote:

> this patch breaks offloading. The reason is that most code
> in env.c is enclosed in:
>

Indeed, normally I test that configuration but my setup currently has a few
problems.

Although the env vars aren't parsed for those targets it seems to be
appropriate to still provide the complete implementation.  There are other
functions which print something this is likely bogus as well and/or the
output isn't seen.

How about this change?  Compiles for me for NVPTX.  It doesn't change
anything but the location of the function definition in the file and
includes for LIBGOMP_OFFLOADED_ONLY some headers which aren't included in
this file before (but are present).

diff --git a/libgomp/env.c b/libgomp/env.c
index 5220877d533..e7ef294139d 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -30,15 +30,16 @@
 #include "libgomp.h"
 #include "gomp-constants.h"
 #include <limits.h>
+#include <stdio.h>
+#include "thread-stacksize.h"
+#ifdef HAVE_INTTYPES_H
+# include <inttypes.h> /* For PRIu64.  */
+#endif
 #ifndef LIBGOMP_OFFLOADED_ONLY
 #include "libgomp_f.h"
 #include "oacc-int.h"
 #include <ctype.h>
 #include <stdlib.h>
-#include <stdio.h>
-#ifdef HAVE_INTTYPES_H
-# include <inttypes.h> /* For PRIu64.  */
-#endif
 #ifdef STRING_WITH_STRINGS
 # include <string.h>
 # include <strings.h>
@@ -52,7 +53,6 @@
 # endif
 #endif
 #include <errno.h>
-#include "thread-stacksize.h"

 #ifndef HAVE_STRTOULL
 # define strtoull(ptr, eptr, base) strtoul (ptr, eptr, base)
@@ -97,11 +97,178 @@ char *goacc_device_type;
 int goacc_device_num;
 int goacc_default_dims[GOMP_DIM_MAX];

-#ifndef LIBGOMP_OFFLOADED_ONLY
-
 static int wait_policy;
 static unsigned long stacksize = GOMP_DEFAULT_STACKSIZE;

+
+void
+omp_display_env (int verbose)
+{
+  int i;
+
+  fputs ("\nOPENMP DISPLAY ENVIRONMENT BEGIN\n", stderr);
+
+  fputs ("  _OPENMP = '201511'\n", stderr);
+  fprintf (stderr, "  OMP_DYNAMIC = '%s'\n",
+   gomp_global_icv.dyn_var ? "TRUE" : "FALSE");
+  fprintf (stderr, "  OMP_NESTED = '%s'\n",
+   gomp_global_icv.max_active_levels_var > 1 ? "TRUE" : "FALSE");
+
+  fprintf (stderr, "  OMP_NUM_THREADS = '%lu",
gomp_global_icv.nthreads_var);
+  for (i = 1; i < gomp_nthreads_var_list_len; i++)
+    fprintf (stderr, ",%lu", gomp_nthreads_var_list[i]);
+  fputs ("'\n", stderr);
+
+  fprintf (stderr, "  OMP_SCHEDULE = '");
+  if ((gomp_global_icv.run_sched_var & GFS_MONOTONIC))
+    {
+      if (gomp_global_icv.run_sched_var != (GFS_MONOTONIC | GFS_STATIC))
+ fputs ("MONOTONIC:", stderr);
+    }
+  else if (gomp_global_icv.run_sched_var == GFS_STATIC)
+    fputs ("NONMONOTONIC:", stderr);
+  switch (gomp_global_icv.run_sched_var & ~GFS_MONOTONIC)
+    {
+    case GFS_RUNTIME:
+      fputs ("RUNTIME", stderr);
+      if (gomp_global_icv.run_sched_chunk_size != 1)
+ fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
+      break;
+    case GFS_STATIC:
+      fputs ("STATIC", stderr);
+      if (gomp_global_icv.run_sched_chunk_size != 0)
+ fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
+      break;
+    case GFS_DYNAMIC:
+      fputs ("DYNAMIC", stderr);
+      if (gomp_global_icv.run_sched_chunk_size != 1)
+ fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
+      break;
+    case GFS_GUIDED:
+      fputs ("GUIDED", stderr);
+      if (gomp_global_icv.run_sched_chunk_size != 1)
+ fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
+      break;
+    case GFS_AUTO:
+      fputs ("AUTO", stderr);
+      break;
+    }
+  fputs ("'\n", stderr);
+
+  fputs ("  OMP_PROC_BIND = '", stderr);
+  switch (gomp_global_icv.bind_var)
+    {
+    case omp_proc_bind_false:
+      fputs ("FALSE", stderr);
+      break;
+    case omp_proc_bind_true:
+      fputs ("TRUE", stderr);
+      break;
+    case omp_proc_bind_master:
+      fputs ("MASTER", stderr);
+      break;
+    case omp_proc_bind_close:
+      fputs ("CLOSE", stderr);
+      break;
+    case omp_proc_bind_spread:
+      fputs ("SPREAD", stderr);
+      break;
+    }
+  for (i = 1; i < gomp_bind_var_list_len; i++)
+    switch (gomp_bind_var_list[i])
+      {
+      case omp_proc_bind_master:
+ fputs (",MASTER", stderr);
+ break;
+      case omp_proc_bind_close:
+ fputs (",CLOSE", stderr);
+ break;
+      case omp_proc_bind_spread:
+ fputs (",SPREAD", stderr);
+ break;
+      }
+  fputs ("'\n", stderr);
+  fputs ("  OMP_PLACES = '", stderr);
+  for (i = 0; i < gomp_places_list_len; i++)
+    {
+      fputs ("{", stderr);
+      gomp_affinity_print_place (gomp_places_list[i]);
+      fputs (i + 1 == gomp_places_list_len ? "}" : "},", stderr);
+    }
+  fputs ("'\n", stderr);
+
+  fprintf (stderr, "  OMP_STACKSIZE = '%lu'\n", stacksize);
+
+  /* GOMP's default value is actually neither active nor passive.  */
+  fprintf (stderr, "  OMP_WAIT_POLICY = '%s'\n",
+   wait_policy > 0 ? "ACTIVE" : "PASSIVE");
+  fprintf (stderr, "  OMP_THREAD_LIMIT = '%u'\n",
+   gomp_global_icv.thread_limit_var);
+  fprintf (stderr, "  OMP_MAX_ACTIVE_LEVELS = '%u'\n",
+   gomp_global_icv.max_active_levels_var);
+
+  fprintf (stderr, "  OMP_CANCELLATION = '%s'\n",
+   gomp_cancel_var ? "TRUE" : "FALSE");
+  fprintf (stderr, "  OMP_DEFAULT_DEVICE = '%d'\n",
+   gomp_global_icv.default_device_var);
+  fprintf (stderr, "  OMP_MAX_TASK_PRIORITY = '%d'\n",
+   gomp_max_task_priority_var);
+  fprintf (stderr, "  OMP_DISPLAY_AFFINITY = '%s'\n",
+   gomp_display_affinity_var ? "TRUE" : "FALSE");
+  fprintf (stderr, "  OMP_AFFINITY_FORMAT = '%s'\n",
+   gomp_affinity_format_var);
+  fprintf (stderr, "  OMP_ALLOCATOR = '");
+  switch (gomp_def_allocator)
+    {
+#define C(v) case v: fputs (#v, stderr); break;
+    C (omp_default_mem_alloc)
+    C (omp_large_cap_mem_alloc)
+    C (omp_const_mem_alloc)
+    C (omp_high_bw_mem_alloc)
+    C (omp_low_lat_mem_alloc)
+    C (omp_cgroup_mem_alloc)
+    C (omp_pteam_mem_alloc)
+    C (omp_thread_mem_alloc)
+#undef C
+    default: break;
+    }
+  fputs ("'\n", stderr);
+
+  fputs ("  OMP_TARGET_OFFLOAD = '", stderr);
+  switch (gomp_target_offload_var)
+    {
+    case GOMP_TARGET_OFFLOAD_DEFAULT:
+      fputs ("DEFAULT", stderr);
+      break;
+    case GOMP_TARGET_OFFLOAD_MANDATORY:
+      fputs ("MANDATORY", stderr);
+      break;
+    case GOMP_TARGET_OFFLOAD_DISABLED:
+      fputs ("DISABLED", stderr);
+      break;
+    }
+  fputs ("'\n", stderr);
+
+  if (verbose)
+    {
+      fputs ("  GOMP_CPU_AFFINITY = ''\n", stderr);
+      fprintf (stderr, "  GOMP_STACKSIZE = '%lu'\n", stacksize);
+#ifdef HAVE_INTTYPES_H
+      fprintf (stderr, "  GOMP_SPINCOUNT = '%"PRIu64"'\n",
+       (uint64_t) gomp_spin_count_var);
+#else
+      fprintf (stderr, "  GOMP_SPINCOUNT = '%lu'\n",
+       (unsigned long) gomp_spin_count_var);
+#endif
+    }
+
+  fputs ("OPENMP DISPLAY ENVIRONMENT END\n", stderr);
+}
+ialias (omp_display_env)
+
+
+#ifndef LIBGOMP_OFFLOADED_ONLY
+
 /* Parse the OMP_SCHEDULE environment variable.  */

 static void
@@ -1213,171 +1380,6 @@ parse_gomp_openacc_dim (void)
     }
 }

-void
-omp_display_env (int verbose)
-{
-  int i;
-
-  fputs ("\nOPENMP DISPLAY ENVIRONMENT BEGIN\n", stderr);
-
-  fputs ("  _OPENMP = '201511'\n", stderr);
-  fprintf (stderr, "  OMP_DYNAMIC = '%s'\n",
-   gomp_global_icv.dyn_var ? "TRUE" : "FALSE");
-  fprintf (stderr, "  OMP_NESTED = '%s'\n",
-   gomp_global_icv.max_active_levels_var > 1 ? "TRUE" : "FALSE");
-
-  fprintf (stderr, "  OMP_NUM_THREADS = '%lu",
gomp_global_icv.nthreads_var);
-  for (i = 1; i < gomp_nthreads_var_list_len; i++)
-    fprintf (stderr, ",%lu", gomp_nthreads_var_list[i]);
-  fputs ("'\n", stderr);
-
-  fprintf (stderr, "  OMP_SCHEDULE = '");
-  if ((gomp_global_icv.run_sched_var & GFS_MONOTONIC))
-    {
-      if (gomp_global_icv.run_sched_var != (GFS_MONOTONIC | GFS_STATIC))
- fputs ("MONOTONIC:", stderr);
-    }
-  else if (gomp_global_icv.run_sched_var == GFS_STATIC)
-    fputs ("NONMONOTONIC:", stderr);
-  switch (gomp_global_icv.run_sched_var & ~GFS_MONOTONIC)
-    {
-    case GFS_RUNTIME:
-      fputs ("RUNTIME", stderr);
-      if (gomp_global_icv.run_sched_chunk_size != 1)
- fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
-      break;
-    case GFS_STATIC:
-      fputs ("STATIC", stderr);
-      if (gomp_global_icv.run_sched_chunk_size != 0)
- fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
-      break;
-    case GFS_DYNAMIC:
-      fputs ("DYNAMIC", stderr);
-      if (gomp_global_icv.run_sched_chunk_size != 1)
- fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
-      break;
-    case GFS_GUIDED:
-      fputs ("GUIDED", stderr);
-      if (gomp_global_icv.run_sched_chunk_size != 1)
- fprintf (stderr, ",%d", gomp_global_icv.run_sched_chunk_size);
-      break;
-    case GFS_AUTO:
-      fputs ("AUTO", stderr);
-      break;
-    }
-  fputs ("'\n", stderr);
-
-  fputs ("  OMP_PROC_BIND = '", stderr);
-  switch (gomp_global_icv.bind_var)
-    {
-    case omp_proc_bind_false:
-      fputs ("FALSE", stderr);
-      break;
-    case omp_proc_bind_true:
-      fputs ("TRUE", stderr);
-      break;
-    case omp_proc_bind_master:
-      fputs ("MASTER", stderr);
-      break;
-    case omp_proc_bind_close:
-      fputs ("CLOSE", stderr);
-      break;
-    case omp_proc_bind_spread:
-      fputs ("SPREAD", stderr);
-      break;
-    }
-  for (i = 1; i < gomp_bind_var_list_len; i++)
-    switch (gomp_bind_var_list[i])
-      {
-      case omp_proc_bind_master:
- fputs (",MASTER", stderr);
- break;
-      case omp_proc_bind_close:
- fputs (",CLOSE", stderr);
- break;
-      case omp_proc_bind_spread:
- fputs (",SPREAD", stderr);
- break;
-      }
-  fputs ("'\n", stderr);
-  fputs ("  OMP_PLACES = '", stderr);
-  for (i = 0; i < gomp_places_list_len; i++)
-    {
-      fputs ("{", stderr);
-      gomp_affinity_print_place (gomp_places_list[i]);
-      fputs (i + 1 == gomp_places_list_len ? "}" : "},", stderr);
-    }
-  fputs ("'\n", stderr);
-
-  fprintf (stderr, "  OMP_STACKSIZE = '%lu'\n", stacksize);
-
-  /* GOMP's default value is actually neither active nor passive.  */
-  fprintf (stderr, "  OMP_WAIT_POLICY = '%s'\n",
-   wait_policy > 0 ? "ACTIVE" : "PASSIVE");
-  fprintf (stderr, "  OMP_THREAD_LIMIT = '%u'\n",
-   gomp_global_icv.thread_limit_var);
-  fprintf (stderr, "  OMP_MAX_ACTIVE_LEVELS = '%u'\n",
-   gomp_global_icv.max_active_levels_var);
-
-  fprintf (stderr, "  OMP_CANCELLATION = '%s'\n",
-   gomp_cancel_var ? "TRUE" : "FALSE");
-  fprintf (stderr, "  OMP_DEFAULT_DEVICE = '%d'\n",
-   gomp_global_icv.default_device_var);
-  fprintf (stderr, "  OMP_MAX_TASK_PRIORITY = '%d'\n",
-   gomp_max_task_priority_var);
-  fprintf (stderr, "  OMP_DISPLAY_AFFINITY = '%s'\n",
-   gomp_display_affinity_var ? "TRUE" : "FALSE");
-  fprintf (stderr, "  OMP_AFFINITY_FORMAT = '%s'\n",
-   gomp_affinity_format_var);
-  fprintf (stderr, "  OMP_ALLOCATOR = '");
-  switch (gomp_def_allocator)
-    {
-#define C(v) case v: fputs (#v, stderr); break;
-    C (omp_default_mem_alloc)
-    C (omp_large_cap_mem_alloc)
-    C (omp_const_mem_alloc)
-    C (omp_high_bw_mem_alloc)
-    C (omp_low_lat_mem_alloc)
-    C (omp_cgroup_mem_alloc)
-    C (omp_pteam_mem_alloc)
-    C (omp_thread_mem_alloc)
-#undef C
-    default: break;
-    }
-  fputs ("'\n", stderr);
-
-  fputs ("  OMP_TARGET_OFFLOAD = '", stderr);
-  switch (gomp_target_offload_var)
-    {
-    case GOMP_TARGET_OFFLOAD_DEFAULT:
-      fputs ("DEFAULT", stderr);
-      break;
-    case GOMP_TARGET_OFFLOAD_MANDATORY:
-      fputs ("MANDATORY", stderr);
-      break;
-    case GOMP_TARGET_OFFLOAD_DISABLED:
-      fputs ("DISABLED", stderr);
-      break;
-    }
-  fputs ("'\n", stderr);
-
-  if (verbose)
-    {
-      fputs ("  GOMP_CPU_AFFINITY = ''\n", stderr);
-      fprintf (stderr, "  GOMP_STACKSIZE = '%lu'\n", stacksize);
-#ifdef HAVE_INTTYPES_H
-      fprintf (stderr, "  GOMP_SPINCOUNT = '%"PRIu64"'\n",
-       (uint64_t) gomp_spin_count_var);
-#else
-      fprintf (stderr, "  GOMP_SPINCOUNT = '%lu'\n",
-       (unsigned long) gomp_spin_count_var);
-#endif
-    }
-
-  fputs ("OPENMP DISPLAY ENVIRONMENT END\n", stderr);
-}
-ialias (omp_display_env)
-
 static void
 handle_omp_display_env (void)
 {

Re: OpenMP 5.1: omp_display_env

[Jakub Jelinek]

On Fri, Jul 30, 2021 at 10:43:01AM +0200, Ulrich Drepper wrote:
> On Fri, Jul 30, 2021 at 9:50 AM Tobias Burnus <tobias@codesourcery.com>
> wrote:
> 
> > this patch breaks offloading. The reason is that most code
> > in env.c is enclosed in:
> >
> 
> Indeed, normally I test that configuration but my setup currently has a few
> problems.
> 
> Although the env vars aren't parsed for those targets it seems to be
> appropriate to still provide the complete implementation.  There are other
> functions which print something this is likely bogus as well and/or the
> output isn't seen.
> 
> How about this change?  Compiles for me for NVPTX.  It doesn't change
> anything but the location of the function definition in the file and
> includes for LIBGOMP_OFFLOADED_ONLY some headers which aren't included in
> this file before (but are present).

I think for now it would be better to guard the omp_display_env_*
in fortran.c with #ifndef LIBGOMP_OFFLOADED_ONLY
It is true that we have e.g. omp_display_affinity supported in offloaded
regions, but in that case we don't really support affinity in the offloaded
regions and so it prints the limited information (like it does even on
hosts that don't support affinity).
But for omp_display_env we shouldn't just print the variables with
default unmodified values, but need to have a structure with all of that
info copied from host to target during load image time.

	Jakub

Re: OpenMP 5.1: omp_display_env

[Ulrich Drepper]

On Fri, Jul 30, 2021 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote:

> I think for now it would be better to guard the omp_display_env_*
> in fortran.c with #ifndef LIBGOMP_OFFLOADED_ONLY
>

OK, easy enough.  This compiles for me.


diff --git a/libgomp/fortran.c b/libgomp/fortran.c
index 76285d4376b..26ec8ce30d8 100644
--- a/libgomp/fortran.c
+++ b/libgomp/fortran.c
@@ -738,6 +738,7 @@ omp_get_default_allocator_ ()
   return (intptr_t) omp_get_default_allocator ();
 }

+#ifndef LIBGOMP_OFFLOADED_ONLY
 void
 omp_display_env_ (const int32_t *verbose)
 {
@@ -749,3 +750,4 @@ omp_display_env_8_ (const int64_t *verbose)
 {
   omp_display_env (!!*verbose);
 }
+#endif /* LIBGOMP_OFFLOADED_ONLY */

Re: OpenMP 5.1: omp_display_env

[Jakub Jelinek]

On Fri, Jul 30, 2021 at 11:54:00AM +0200, Ulrich Drepper wrote:
> On Fri, Jul 30, 2021 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote:
> 
> > I think for now it would be better to guard the omp_display_env_*
> > in fortran.c with #ifndef LIBGOMP_OFFLOADED_ONLY
> >
> 
> OK, easy enough.  This compiles for me.

Ok (with ChangeLog entry), thanks.

> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
> index 76285d4376b..26ec8ce30d8 100644
> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c
> @@ -738,6 +738,7 @@ omp_get_default_allocator_ ()
>    return (intptr_t) omp_get_default_allocator ();
>  }
> 
> +#ifndef LIBGOMP_OFFLOADED_ONLY
>  void
>  omp_display_env_ (const int32_t *verbose)
>  {
> @@ -749,3 +750,4 @@ omp_display_env_8_ (const int64_t *verbose)
>  {
>    omp_display_env (!!*verbose);
>  }
> +#endif /* LIBGOMP_OFFLOADED_ONLY */

	Jakub

Re: OpenMP 5.1: omp_display_env

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

Hi!

On 2021-07-30T12:02:00+0200, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On Fri, Jul 30, 2021 at 11:54:00AM +0200, Ulrich Drepper wrote:
>> On Fri, Jul 30, 2021 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> > I think for now it would be better to guard the omp_display_env_*
>> > in fortran.c with #ifndef LIBGOMP_OFFLOADED_ONLY
>>
>> OK, easy enough.  This compiles for me.
>
> Ok (with ChangeLog entry), thanks.

Heh, I had just come up with the same patch, and pushed
"[libgomp] Restore offloading 'libgomp/fortran.c'" to master branch in
commit 28665ddc7efa48f9b39615e313a2c4a7a66cdb24, see attached.


Grüße
 Thomas


>> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
>> index 76285d4376b..26ec8ce30d8 100644
>> --- a/libgomp/fortran.c
>> +++ b/libgomp/fortran.c
>> @@ -738,6 +738,7 @@ omp_get_default_allocator_ ()
>>    return (intptr_t) omp_get_default_allocator ();
>>  }
>>
>> +#ifndef LIBGOMP_OFFLOADED_ONLY
>>  void
>>  omp_display_env_ (const int32_t *verbose)
>>  {
>> @@ -749,3 +750,4 @@ omp_display_env_8_ (const int64_t *verbose)
>>  {
>>    omp_display_env (!!*verbose);
>>  }
>> +#endif /* LIBGOMP_OFFLOADED_ONLY */
>
>       Jakub


-----------------
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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libgomp-Restore-offloading-libgomp-fortran.c.patch --]
[-- Type: text/x-diff, Size: 2339 bytes --]

From 28665ddc7efa48f9b39615e313a2c4a7a66cdb24 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Fri, 30 Jul 2021 11:48:54 +0200
Subject: [PATCH] [libgomp] Restore offloading 'libgomp/fortran.c'

GCN:

    ld: error: undefined symbol: gomp_ialias_omp_display_env
    >>> referenced by fortran.c:744 ([...]/source-gcc/libgomp/fortran.c:744)
    >>>               fortran.o:(omp_display_env_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
    >>> referenced by fortran.c:744 ([...]/source-gcc/libgomp/fortran.c:744)
    >>>               fortran.o:(omp_display_env_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
    >>> referenced by fortran.c:750 ([...]/source-gcc/libgomp/fortran.c:750)
    >>>               fortran.o:(omp_display_env_8_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
    >>> referenced by fortran.c:750 ([...]/source-gcc/libgomp/fortran.c:750)
    >>>               fortran.o:(omp_display_env_8_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
    collect2: error: ld returned 1 exit status
    mkoffload: fatal error: build-gcc/gcc/x86_64-pc-linux-gnu-accel-amdgcn-amdhsa-gcc returned 1 exit status

nvptx:

    unresolved symbol omp_display_env
    collect2: error: ld returned 1 exit status
    mkoffload: fatal error: [...]/build-gcc/./gcc/x86_64-pc-linux-gnu-accel-nvptx-none-gcc returned 1 exit status

Fix-up for commit 7123ae2455b5a1a2f19f13fa82c377cfda157f23
"Implement OpenMP 5.1 section 3.15: omp_display_env".

	libgomp/
	* fortran.c (omp_display_env_, omp_display_env_8_): Only
	'#ifndef LIBGOMP_OFFLOADED_ONLY'.

Co-Authored-By: Ulrich Drepper <drepper@redhat.com>
---
 libgomp/fortran.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libgomp/fortran.c b/libgomp/fortran.c
index 76285d4376b..e042702ac91 100644
--- a/libgomp/fortran.c
+++ b/libgomp/fortran.c
@@ -738,6 +738,8 @@ omp_get_default_allocator_ ()
   return (intptr_t) omp_get_default_allocator ();
 }
 
+#ifndef LIBGOMP_OFFLOADED_ONLY
+
 void
 omp_display_env_ (const int32_t *verbose)
 {
@@ -749,3 +751,5 @@ omp_display_env_8_ (const int64_t *verbose)
 {
   omp_display_env (!!*verbose);
 }
+
+#endif /* LIBGOMP_OFFLOADED_ONLY */
-- 
2.30.2

Re: OpenMP 5.1: omp_display_env

[Thomas Schwinge]

Hi!

On 2021-07-30T12:05:56+0200, I wrote:
> On 2021-07-30T12:02:00+0200, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> On Fri, Jul 30, 2021 at 11:54:00AM +0200, Ulrich Drepper wrote:
>>> On Fri, Jul 30, 2021 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote:
>>>
>>> > I think for now it would be better to guard the omp_display_env_*
>>> > in fortran.c with #ifndef LIBGOMP_OFFLOADED_ONLY
>>>
>>> OK, easy enough.  This compiles for me.
>>
>> Ok (with ChangeLog entry), thanks.
>
> Heh, I had just come up with the same patch, and pushed
> "[libgomp] Restore offloading 'libgomp/fortran.c'" to master branch in
> commit 28665ddc7efa48f9b39615e313a2c4a7a66cdb24, see attached.

I'm sorry if I stepped on anyone's toes: Tobias told me that you were
still discussing on IRC the proper way of fixing this, while I went ahead
and pushed what I considered "obvious enough", meaning that it fixes the
regression, whilst not breaking anything else.  (Adding more
functionality can of course be done, incrementally.)


Grüße
 Thomas


> From 28665ddc7efa48f9b39615e313a2c4a7a66cdb24 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Fri, 30 Jul 2021 11:48:54 +0200
> Subject: [PATCH] [libgomp] Restore offloading 'libgomp/fortran.c'
>
> GCN:
>
>     ld: error: undefined symbol: gomp_ialias_omp_display_env
>     >>> referenced by fortran.c:744 ([...]/source-gcc/libgomp/fortran.c:744)
>     >>>               fortran.o:(omp_display_env_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
>     >>> referenced by fortran.c:744 ([...]/source-gcc/libgomp/fortran.c:744)
>     >>>               fortran.o:(omp_display_env_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
>     >>> referenced by fortran.c:750 ([...]/source-gcc/libgomp/fortran.c:750)
>     >>>               fortran.o:(omp_display_env_8_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
>     >>> referenced by fortran.c:750 ([...]/source-gcc/libgomp/fortran.c:750)
>     >>>               fortran.o:(omp_display_env_8_) in archive [...]/build-gcc-offload-amdgcn-amdhsa/amdgcn-amdhsa/libgomp/.libs/libgomp.a
>     collect2: error: ld returned 1 exit status
>     mkoffload: fatal error: build-gcc/gcc/x86_64-pc-linux-gnu-accel-amdgcn-amdhsa-gcc returned 1 exit status
>
> nvptx:
>
>     unresolved symbol omp_display_env
>     collect2: error: ld returned 1 exit status
>     mkoffload: fatal error: [...]/build-gcc/./gcc/x86_64-pc-linux-gnu-accel-nvptx-none-gcc returned 1 exit status
>
> Fix-up for commit 7123ae2455b5a1a2f19f13fa82c377cfda157f23
> "Implement OpenMP 5.1 section 3.15: omp_display_env".
>
>       libgomp/
>       * fortran.c (omp_display_env_, omp_display_env_8_): Only
>       '#ifndef LIBGOMP_OFFLOADED_ONLY'.
>
> Co-Authored-By: Ulrich Drepper <drepper@redhat.com>
> ---
>  libgomp/fortran.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
> index 76285d4376b..e042702ac91 100644
> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c
> @@ -738,6 +738,8 @@ omp_get_default_allocator_ ()
>    return (intptr_t) omp_get_default_allocator ();
>  }
>
> +#ifndef LIBGOMP_OFFLOADED_ONLY
> +
>  void
>  omp_display_env_ (const int32_t *verbose)
>  {
> @@ -749,3 +751,5 @@ omp_display_env_8_ (const int64_t *verbose)
>  {
>    omp_display_env (!!*verbose);
>  }
> +
> +#endif /* LIBGOMP_OFFLOADED_ONLY */
> --
> 2.30.2
-----------------
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