| Message ID | 26b4617e-7452-3b67-c29d-2d9f0c39d3e8@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | [libgomp,nvptx] Disable OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support | expand |
On Wed, Dec 12, 2018 at 09:29:36AM +0100, Tom de Vries wrote: > > This is the libgomp/ except libgomp/testsuite/ part of the gomp-5_0-branch > > merge to trunk I've just committed. > > > > 2018-11-08 Jakub Jelinek <jakub@redhat.com> > > > > * affinity.c (gomp_display_affinity_place): New function. > > * affinity-fmt.c: New file. > > > * fortran.c: Include stdio.h and string.h. > > (omp_pause_resource, omp_pause_resource_all): Add ialias_redirect. > > (omp_get_schedule_, omp_get_schedule_8_): Mask off GFS_MONOTONIC bit. > > (omp_set_affinity_format_, omp_get_affinity_format_, > > omp_display_affinity_, omp_capture_affinity_, omp_pause_resource_, > > omp_pause_resource_all_): New functions. > > OK for trunk? > > Thanks, > - Tom > [libgomp, nvptx] Disable OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support > > Disable compilation of support for OMP_DISPLAY_AFFINITY and OMP_AFFINITY_FORMAT > for nvptx. > > This fixes some libgomp testsuite failures for x86_64 with nvptx accelerator. > > Build on x86_64 with nvptx accelerator, tested libgomp. > > 2018-12-12 Tom de Vries <tdevries@suse.de> > > * affinity.c (gomp_display_affinity_place): Guard with > #ifndef LIBGOMP_OFFLOADED_ONLY. > * fortran.c (omp_set_affinity_format_, omp_get_affinity_format_) > (omp_display_affinity_, omp_capture_affinity_): Same. > * libgomp/config/nvptx/affinity-fmt.c: New, empty file overriding > libgomp/affinity-fmt.c. The runtime APIs should still be available inside of OpenMP regions, your patch makes them unavailable. What exactly doesn't work? hostname handling, getpid, thread_id, affinity places? I'd prefer for now to just stub what doesn't work in the LIBGOMP_OFFLOADED_ONLY case (print "node", 0 as pid, 0 as thread id, something for affinity (0 or 0-N where N is the number of warps it can support or whatever). Eventually we need to find a way how to transfer some ICVs and other data from the host to the offloading library, either process shared that aren't changing, which can include the hostname, getpid, or some others that would need to be passed for every target region. Jakub
On 12-12-18 10:36, Jakub Jelinek wrote: > On Wed, Dec 12, 2018 at 09:29:36AM +0100, Tom de Vries wrote: >>> This is the libgomp/ except libgomp/testsuite/ part of the gomp-5_0-branch >>> merge to trunk I've just committed. >>> >>> 2018-11-08 Jakub Jelinek <jakub@redhat.com> >>> >>> * affinity.c (gomp_display_affinity_place): New function. >>> * affinity-fmt.c: New file. >> >>> * fortran.c: Include stdio.h and string.h. >>> (omp_pause_resource, omp_pause_resource_all): Add ialias_redirect. >>> (omp_get_schedule_, omp_get_schedule_8_): Mask off GFS_MONOTONIC bit. >>> (omp_set_affinity_format_, omp_get_affinity_format_, >>> omp_display_affinity_, omp_capture_affinity_, omp_pause_resource_, >>> omp_pause_resource_all_): New functions. >> >> OK for trunk? >> >> Thanks, >> - Tom > >> [libgomp, nvptx] Disable OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support >> >> Disable compilation of support for OMP_DISPLAY_AFFINITY and OMP_AFFINITY_FORMAT >> for nvptx. >> >> This fixes some libgomp testsuite failures for x86_64 with nvptx accelerator. >> >> Build on x86_64 with nvptx accelerator, tested libgomp. >> >> 2018-12-12 Tom de Vries <tdevries@suse.de> >> >> * affinity.c (gomp_display_affinity_place): Guard with >> #ifndef LIBGOMP_OFFLOADED_ONLY. >> * fortran.c (omp_set_affinity_format_, omp_get_affinity_format_) >> (omp_display_affinity_, omp_capture_affinity_): Same. >> * libgomp/config/nvptx/affinity-fmt.c: New, empty file overriding >> libgomp/affinity-fmt.c. > > The runtime APIs should still be available inside of OpenMP regions, your > patch makes them unavailable. Ah, I see. > What exactly doesn't work? > hostname handling, getpid, thread_id, affinity places? The concrete error I'm getting is unresolved symbol getpid. This is caused by the AC_COMPILE_IFELSE configure test for HAVE_GETPID passing for nvptx, while the master newlib for nvptx does not contain getpid. > I'd prefer for now to just stub what doesn't work in the LIBGOMP_OFFLOADED_ONLY > case (print "node", 0 as pid, 0 as thread id, something for affinity > (0 or 0-N where N is the number of warps it can support or whatever). > OK, I'll give that a try. Thanks, - Tom > Eventually we need to find a way how to transfer some ICVs and other data > from the host to the offloading library, either process shared that aren't > changing, which can include the hostname, getpid, or some others that would > need to be passed for every target region.
Hi! On Wed, 12 Dec 2018 10:36:17 +0100, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Dec 12, 2018 at 09:29:36AM +0100, Tom de Vries wrote: > > > This is the libgomp/ except libgomp/testsuite/ part of the gomp-5_0-branch > > > merge to trunk I've just committed. > > [libgomp, nvptx] Disable OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support Thanks, Tom, for looking into these issues. I'd also noticed these, but not yet been able to allocate time to resolve them. I'll test your patches, too. > Eventually we need to find a way how to transfer some ICVs and other data > from the host to the offloading library, either process shared that aren't > changing, which can include the hostname, getpid, or some others that would > need to be passed for every target region. That would probably also include any state that the respective language runtime libraries maintain? For example, C's global rounding mode as set my "fesetround". ..., and I now wonder how to propagate that from the host libc to the target libcs, for example host: glibc vs. nvptx offloading: newlib -- have to do a host-side "fegetround" before each offloaded code region, or at least when there's been an intermediate host-side "fesetround" call (so have to track these?), and likewise for any other such state-changing functions? Also, the "options" array and call of "_gfortran_set_options" that the Fortran front end synthesizes into each "main" function? I have not yet looked into these things in detail, however. Grüße Thomas
On 12-12-18 11:48, Tom de Vries wrote: > On 12-12-18 10:36, Jakub Jelinek wrote: >> On Wed, Dec 12, 2018 at 09:29:36AM +0100, Tom de Vries wrote: >>>> This is the libgomp/ except libgomp/testsuite/ part of the gomp-5_0-branch >>>> merge to trunk I've just committed. >>>> >>>> 2018-11-08 Jakub Jelinek <jakub@redhat.com> >>>> >>>> * affinity.c (gomp_display_affinity_place): New function. >>>> * affinity-fmt.c: New file. >>> >>>> * fortran.c: Include stdio.h and string.h. >>>> (omp_pause_resource, omp_pause_resource_all): Add ialias_redirect. >>>> (omp_get_schedule_, omp_get_schedule_8_): Mask off GFS_MONOTONIC bit. >>>> (omp_set_affinity_format_, omp_get_affinity_format_, >>>> omp_display_affinity_, omp_capture_affinity_, omp_pause_resource_, >>>> omp_pause_resource_all_): New functions. >>> >>> OK for trunk? >>> >>> Thanks, >>> - Tom >> >>> [libgomp, nvptx] Disable OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support >>> >>> Disable compilation of support for OMP_DISPLAY_AFFINITY and OMP_AFFINITY_FORMAT >>> for nvptx. >>> >>> This fixes some libgomp testsuite failures for x86_64 with nvptx accelerator. >>> >>> Build on x86_64 with nvptx accelerator, tested libgomp. >>> >>> 2018-12-12 Tom de Vries <tdevries@suse.de> >>> >>> * affinity.c (gomp_display_affinity_place): Guard with >>> #ifndef LIBGOMP_OFFLOADED_ONLY. >>> * fortran.c (omp_set_affinity_format_, omp_get_affinity_format_) >>> (omp_display_affinity_, omp_capture_affinity_): Same. >>> * libgomp/config/nvptx/affinity-fmt.c: New, empty file overriding >>> libgomp/affinity-fmt.c. >> >> The runtime APIs should still be available inside of OpenMP regions, your >> patch makes them unavailable. > > Ah, I see. > >> What exactly doesn't work? >> hostname handling, getpid, thread_id, affinity places? > > The concrete error I'm getting is unresolved symbol getpid. This is > caused by the AC_COMPILE_IFELSE configure test for HAVE_GETPID passing > for nvptx, while the master newlib for nvptx does not contain getpid. > >> I'd prefer for now to just stub what doesn't work in the LIBGOMP_OFFLOADED_ONLY >> case (print "node", 0 as pid, 0 as thread id, something for affinity >> (0 or 0-N where N is the number of warps it can support or whatever). >> > > OK, I'll give that a try. > This RFC patch implements that approach for getpid and gethostname (I wonder though whether it's not possible to turn the corresponding configure tests into link tests, which could also fix this for nvptx). Another problem we're running into is missing istty, pulled in by fwrite. The nvptx newlib port does support write, but I'm unsure in what form I should handle this. Add configure tests for fwrite and write? Or special case the files in the config/nvptx dir? Any advice here? Thanks, - Tom [RFC][libgomp, nvptx] Fix target-5.c compilation Libgomp test-case libgomp.c/target-5.c is failing to compile with nvptx accelerator due to missing: - getpid - gethostname - isatty (pulled in by fwrite) in the nvptx newlib. This demonstrator patch fixes that by minimally guarding all related locations with ifndef LIBGOMP_OFFLOADED_ONLY, which allows the test-case (and others) to pass. --- libgomp/affinity-fmt.c | 12 +++++++++++- libgomp/fortran.c | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/libgomp/affinity-fmt.c b/libgomp/affinity-fmt.c index 9e5c5f754eb..72b511eec73 100644 --- a/libgomp/affinity-fmt.c +++ b/libgomp/affinity-fmt.c @@ -184,6 +184,7 @@ static void gomp_display_hostname (char *buffer, size_t size, size_t *ret, bool right, size_t sz) { +#ifndef LIBGOMP_OFFLOADED_ONLY #ifdef HAVE_GETHOSTNAME { char buf[256]; @@ -227,6 +228,7 @@ gomp_display_hostname (char *buffer, size_t size, size_t *ret, return; } } +#endif #endif gomp_display_string_len (buffer, size, ret, right, sz, "node", 4); } @@ -351,7 +353,7 @@ gomp_display_affinity (char *buffer, size_t size, gomp_display_hostname (buffer, size, &ret, right, sz); break; case 'P': -#ifdef HAVE_GETPID +#if defined(HAVE_GETPID) && !defined(LIBGOMP_OFFLOADED_ONLY) val = getpid (); #else val = 0; @@ -456,13 +458,17 @@ omp_display_affinity (const char *format) if (ret < sizeof buf) { buf[ret] = '\n'; +#ifndef LIBGOMP_OFFLOADED_ONLY fwrite (buf, 1, ret + 1, stderr); +#endif return; } b = gomp_malloc (ret + 1); ialias_call (omp_capture_affinity) (b, ret + 1, format); b[ret] = '\n'; +#ifndef LIBGOMP_OFFLOADED_ONLY fwrite (b, 1, ret + 1, stderr); +#endif free (b); } @@ -477,13 +483,17 @@ gomp_display_affinity_thread (gomp_thread_handle handle, if (ret < sizeof buf) { buf[ret] = '\n'; +#ifndef LIBGOMP_OFFLOADED_ONLY fwrite (buf, 1, ret + 1, stderr); +#endif return; } b = gomp_malloc (ret + 1); gomp_display_affinity (b, ret + 1, gomp_affinity_format_var, handle, ts, place); b[ret] = '\n'; +#ifndef LIBGOMP_OFFLOADED_ONLY fwrite (b, 1, ret + 1, stderr); +#endif free (b); } diff --git a/libgomp/fortran.c b/libgomp/fortran.c index 0157baec648..21ea9482e09 100644 --- a/libgomp/fortran.c +++ b/libgomp/fortran.c @@ -626,7 +626,9 @@ omp_display_affinity_ (const char *format, size_t format_len) if (ret < sizeof buf) { buf[ret] = '\n'; +#ifndef LIBGOMP_OFFLOADED_ONLY fwrite (buf, 1, ret + 1, stderr); +#endif } else { @@ -635,7 +637,9 @@ omp_display_affinity_ (const char *format, size_t format_len) format_len ? fmt : gomp_affinity_format_var, gomp_thread_self (), &thr->ts, thr->place); b[ret] = '\n'; +#ifndef LIBGOMP_OFFLOADED_ONLY fwrite (b, 1, ret + 1, stderr); +#endif free (b); } if (fmt && fmt != fmt_buf)
On Wed, Dec 12, 2018 at 02:02:20PM +0100, Tom de Vries wrote: > This RFC patch implements that approach for getpid and gethostname (I > wonder though whether it's not possible to turn the corresponding > configure tests into link tests, which could also fix this for nvptx). > > Another problem we're running into is missing istty, pulled in by > fwrite. The nvptx newlib port does support write, but I'm unsure in what > form I should handle this. Add configure tests for fwrite and write? Or > special case the files in the config/nvptx dir? Any advice here? > > Thanks, > - Tom > [RFC][libgomp, nvptx] Fix target-5.c compilation > > Libgomp test-case libgomp.c/target-5.c is failing to compile with nvptx > accelerator due to missing: > - getpid > - gethostname > - isatty (pulled in by fwrite) > in the nvptx newlib. > > This demonstrator patch fixes that by minimally guarding all related locations > with ifndef LIBGOMP_OFFLOADED_ONLY, which allows the test-case (and others) to > pass. I guess my preference would be something similar to what libgomp/config/mingw32/affinity-fmt.c does now, so override the file, define/undefine something in there and include the common affinity-fmt.c. Perhaps for the fwrite stuff define introduce a function that does that (say void gomp_print_string (const char *str, size_t len) { fwrite (str, 1, len, stderr); }, because it is in more than one file, make it hidden in libgomp.h. config/nvptx/error.c then has what you could also use in config/nvptx/affinity-fmt.c. And for hostname/getpid maybe just #undef those with a comment? Jakub
[libgomp, nvptx] Disable OMP_{DISPLAY_AFFINITY,AFFINITY_FORMAT} support
Disable compilation of support for OMP_DISPLAY_AFFINITY and OMP_AFFINITY_FORMAT
for nvptx.
This fixes some libgomp testsuite failures for x86_64 with nvptx accelerator.
Build on x86_64 with nvptx accelerator, tested libgomp.
2018-12-12 Tom de Vries <tdevries@suse.de>
* affinity.c (gomp_display_affinity_place): Guard with
#ifndef LIBGOMP_OFFLOADED_ONLY.
* fortran.c (omp_set_affinity_format_, omp_get_affinity_format_)
(omp_display_affinity_, omp_capture_affinity_): Same.
* libgomp/config/nvptx/affinity-fmt.c: New, empty file overriding
libgomp/affinity-fmt.c.
---
libgomp/affinity.c | 2 ++
libgomp/config/nvptx/affinity-fmt.c | 0
libgomp/fortran.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/libgomp/affinity.c b/libgomp/affinity.c
index efe5c4260e3..dbfb8e7a75d 100644
--- a/libgomp/affinity.c
+++ b/libgomp/affinity.c
@@ -140,6 +140,7 @@ gomp_get_place_proc_ids_8 (int place_num, int64_t *ids)
(void) ids;
}
+#ifndef LIBGOMP_OFFLOADED_ONLY
void
gomp_display_affinity_place (char *buffer, size_t size, size_t *ret,
int place)
@@ -151,6 +152,7 @@ gomp_display_affinity_place (char *buffer, size_t size, size_t *ret,
strcpy (buf, "0");
gomp_display_string (buffer, size, ret, buf, strlen (buf));
}
+#endif
ialias(omp_get_place_num_procs)
ialias(omp_get_place_proc_ids)
diff --git a/libgomp/config/nvptx/affinity-fmt.c b/libgomp/config/nvptx/affinity-fmt.c
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/libgomp/fortran.c b/libgomp/fortran.c
index 0157baec648..a577a2e515b 100644
--- a/libgomp/fortran.c
+++ b/libgomp/fortran.c
@@ -584,6 +584,7 @@ omp_get_max_task_priority_ (void)
return omp_get_max_task_priority ();
}
+#ifndef LIBGOMP_OFFLOADED_ONLY
void
omp_set_affinity_format_ (const char *format, size_t format_len)
{
@@ -664,6 +665,7 @@ omp_capture_affinity_ (char *buffer, const char *format,
memset (buffer + ret, ' ', buffer_len - ret);
return ret;
}
+#endif
int32_t
omp_pause_resource_ (const int32_t *kind, const int32_t *device_num)