| Message ID | CAP3s5k-Ru6a9EHJe-CpK0ZiuZ84fEALn+6rQ8GsU5e_Bez-Vew@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | OpenMP 5.1: omp_display_env | expand |
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
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
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) {
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
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 */
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
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
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
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