[committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Jakub Jelinek]

Hi!

The OpenMP 5.0 specification, https://www.openmp.org/specifications/ ,
has been just released a few minutes ago and to celebrate that, I've merged
gomp-5_0-branch into trunk after bootstrapping/regtesting it on x86_64-linux and
i686-linux.

Because the amount of changes in OpenMP 5.0 is much bigger than in any of the 
earlier
releases of the standard, unfortunately the whole spec isn't implemented at 
this point,
not even for C/C++.  So, let me start by listing features that are implemented.
Unless otherwise stated, the implementation is for now for C/C++ only, Fortran 
to follow
after C/C++ is fully done.

New OpenMP 5.0 features in this patchset:

- task reductions, including task modifier on parallel/worksharing construct 
reduction
- != conditions in OpenMP loops
- C++1[147] range for loops in worksharing loop, taskloop and distribute
  (and combined/composite constructs)
- allow private or lastprivate clauses for iterator variable(s) on simd 
construct or
  combined/composite constructs including simd
- iterators in depend clause
- support for lvalue expressions in depend clauses (note, some expressions in 
depend
  clauses that got allowed very recently are still unsupported)
- mutexinoutset dependence kind (right now this is implemented in the runtime 
library
  as less efficient inout, but can be improved later solely on in the runtime 
library)
- depobj construct, depobj dependence kind and omp_depend_t
- depend clause on taskwait construct
- host teams construct (the library implementation still needs work, so that it 
is
  actually beneficial on NUMA setups, see below)
- cancel if clause modifier
- if and nontemporal clauses on simd (both are parsed only right now, unless I 
figure
  out how to propagate it through IL to the vectorizer quickly, if will either 
force
  no simd at all, or will cause duplication of the loop; nontemporal either will
  use nontemporal stores, or for GCC 9 will do nothing)
- defaultmap clause extensions
- hint and memory order clauses on atomic (hint is parsed and ignored by the
  implementation for now, memory order fully implemented)
- memory order clauses on flush
- support for new combined #pragma omp parallel master
  and #pragma omp {,parallel }master taskloop{, simd} constructs
- affinity display support, 
omp_{[sg]et_affinity_format,{display,capture}_affinity},
  OMP_DISPLAY_AFFINITY and OMP_AFFINITY_FORMAT env vars (this is implemented 
also
  for Fortran)
- omp_pause_resource{,_all} support, omp_pause_resource_t type (for now the
  runtime library is able to free resources on the host only; this is 
implemented
  also for Fortran)
- worksharing loop schedules now default to nonmonotonic with the exception of
  static schedules, nonmonotonic allowed on static, runtime and auto,
  omp_sched_monotonic modifier and OMP_SCHEDULE env var parsing changes (note,
  the runtime library is told if monotonic or nonmonotonic schedule is used
  in a backwards compatible way, but the runtime library ATM doesn't take
  advantage of nonmonotonic schedules, everything is still effectively 
monotonic)
- allow only use_device_ptr clause(s) on target data construct
- change data sharing for readonly variables without mutable members, they are
  no longer predetermined shared (this actually changed in earlier OpenMP 
standard
  releases, but was considered a mistake; for 5.0 it was decided it isn't going 
to
  be reverted; this makes a difference mainly when using default(none))
- allow atomic constructs in simd regions (note, for now this causes the 
vectorizer
  to fail to vectorize such regions)
- allow comma in between (name) and hint clause on critical construct
- device routine prototype changes (void * to const void * arguments)
- omp_lock_hint* to omp_sync_hint* changes
- partial requires construct support, only atomic_default_mem_order fully 
implemented

Features I'll still try to implement for GCC 9:

- make sure all expressions in the OpenMP grammar within clauses are
  assignment-expression, with the exception of array section expressions
- verify taskloop construct cancellation works
- stop diagnosing threadprivate directive after first use to allow definitions 
of
  variables with constructors followed by threadprivate, rather than the 
currently
  required workaround of extern declaration of it, followed by threadprivate 
directive
  followed by definition

New OpenMP 5.0 features that won't be available in GCC 9, are planned for GCC 10
or later versions as time permits:

- requires directive other than atomic_default_mem_order (parsing is 
implemented, but
  the rest is not)
- inscan reduction clause modifier and scan directive
- lastprivate clause with conditional modifier
- OMP_TARGET_OFFLOAD env var and target-offload-var ICV
- max-active-levels-var / nested-var ICV, omp_[sg]et_nested and OMP_NESTED 
changes
- omp_get_supported_active_levels API addition
- array shaping support, array sections with non-unit strides in to/from/depend 
clauses
- metadirecti

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Thomas Schwinge]

Hi Jakub!

On Thu, 8 Nov 2018 18:16:11 +0100, Jakub Jelinek  wrote:
> The OpenMP 5.0 specification, https://www.openmp.org/specifications/ ,
> has been just released a few minutes ago and to celebrate that, I've merged
> gomp-5_0-branch into trunk after bootstrapping/regtesting it on x86_64-linux 
> and
> i686-linux.

In addition to not having tested this with nvptx offloading (where Tom
and you now restored the regressed test cases, thanks!), I can tell that
you also didn't test this with Intel MIC (emulated) offloading.  ;-)

> Because the amount of changes in OpenMP 5.0 is much bigger than in any of the 
> earlier
> releases of the standard, [...]

Oh yes, that's massive!  I immediately thought "poor Jakub" ;-) when I
read a summary of all the new stuff in there.


After  'Intel MIC (emulated) offloading:
"relocation [...] can not be used when making a shared object; recompile
with -fPIC"', yours is now another commit that further broke Intel MIC
(emulated) offloading, but in the past month apparently nobody but me has
run into this (or didn't bother to report it), and I thus again wonder
whether anyone but me is still testing Intel MIC (emulated) offloading?

Anyway, that was easy enough to fix; in r267145 committed to trunk:

commit fbd4f724c13b078755a96a257eabc18ddb83a9cd
Author: tschwinge 
Date:   Fri Dec 14 20:41:46 2018 +

Repair liboffloadmic after "(Partial) OpenMP 5.0 support for GCC 9"

..., which now failed to build, as follows:

In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
[...]/source-gcc/liboffloadmic/runtime/offload.h:220:12: error: 
conflicting declaration of C function 'int omp_target_is_present(void*, int)'
  220 | extern int omp_target_is_present(
  |^
In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload.h:45,
 from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
./../libgomp/omp.h:166:12: note: previous declaration 'int 
omp_target_is_present(const void*, int)'
  166 | extern int omp_target_is_present (const void *, int) 
__GOMP_NOTHROW;
  |^
In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
[...]/source-gcc/liboffloadmic/runtime/offload.h:236:12: error: 
conflicting declaration of C function 'int omp_target_memcpy(void*, void*, 
size_t, size_t, size_t, int, int)'
  236 | extern int omp_target_memcpy(
  |^
In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload.h:45,
 from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
./../libgomp/omp.h:167:12: note: previous declaration 'int 
omp_target_memcpy(void*, const void*, long unsigned int, long unsigned int, 
long unsigned int, int, int)'
  167 | extern int omp_target_memcpy (void *, const void *, 
__SIZE_TYPE__,
  |^
In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
[...]/source-gcc/liboffloadmic/runtime/offload.h:262:12: error: 
conflicting declaration of C function 'int omp_target_memcpy_rect(void*, void*, 
size_t, int, const size_t*, const size_t*, const size_t*, const size_t*, const 
size_t*, int, int)'
  262 | extern int omp_target_memcpy_rect(
  |^~
In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload.h:45,
 from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
./../libgomp/omp.h:170:12: note: previous declaration 'int 
omp_target_memcpy_rect(void*, const void*, long unsigned int, int, const long 
unsigned int*, const long unsigned int*, const long unsigned int*, const long 
unsigned int*, const long unsigned int*, int, int)'
  170 | extern int omp_target_memcpy_rect (void *, const void *, 
__SIZE_TYPE__, int,
  |^~
In file included from 
[...]/source-gcc/liboffloadmic/runtime/offload_common.h:43,
 from 
[...]/source-gcc/liboffloadmic/runtime/dv_util.cpp:31:
[...]/source-gcc/liboffloadmic/runtime/offload.h:285:12: error: 
conflicting declaration of C function 'int omp_target_associate_ptr(

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Jakub Jelinek]

On Fri, Dec 14, 2018 at 09:55:51PM +0100, Thomas Schwinge wrote:
> Anyway, that was easy enough to fix; in r267145 committed to trunk:

> liboffloadmic/
> * runtime/offload.h (omp_target_is_present, omp_target_memcpy)
> (omp_target_memcpy_rect, omp_target_associate_ptr)
> (omp_target_disassociate_ptr): Adjust to libgomp changes.

Thanks; indeed, I wasn't testing the OpenMP 5.0 patchset with offloading
mainly because there weren't too many offloading related changes so far
(most of them waiting for GCC 10).

Jakub

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Rainer Orth]

Hi Jakub,

> The OpenMP 5.0 specification, https://www.openmp.org/specifications/ ,
> has been just released a few minutes ago and to celebrate that, I've merged
> gomp-5_0-branch into trunk after bootstrapping/regtesting it on x86_64-linux 
> and
> i686-linux.

this patch series broke the Solaris build:

/vol/gcc/src/hg/trunk/local/libgomp/affinity.c: In function 
'gomp_display_affinity_place':
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:145:3: error: unknown type name 
'cpu_set_t'
  145 |   cpu_set_t *cpusetp;
  |   ^
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:148:5: error: implicit 
declaration of function 'sprintf' [-Werror=implicit-function-declaration]
  148 | sprintf (buf, "0-%lu", gomp_available_cpus - 1);
  | ^~~

/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:148:5: error: implicit 
declaration of function 'sprintf' [-Werror=implicit-function-declaration]
  148 | sprintf (buf, "0-%lu", gomp_available_cpus - 1);
  | ^~~
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:148:5: error: incompatible 
implicit declaration of built-in function 'sprintf' [-Werror]
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:29:1: note: include '' 
or provide a declaration of 'sprintf'
   28 | #include "libgomp.h"
  +++ |+#include 
   29 |
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:150:5: error: implicit 
declaration of function 'strcpy' [-Werror=implicit-function-declaration]
  150 | strcpy (buf, "0");
  | ^~
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:150:5: error: incompatible 
implicit declaration of built-in function 'strcpy' [-Werror]
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:29:1: note: include '' 
or provide a declaration of 'strcpy'
   28 | #include "libgomp.h"
  +++ |+#include 
   29 |
/vol/gcc/src/hg/trunk/local/libgomp/affinity.c:151:48: error: implicit 
declaration of function 'strlen' [-Werror=implicit-function-declaration]
  151 |   gomp_display_string (buffer, size, ret, buf, strlen (buf));
  |^~

/vol/gcc/src/hg/trunk/local/libgomp/teams.c: In function 'GOMP_teams_reg':
/vol/gcc/src/hg/trunk/local/libgomp/teams.c:44:19: error: 'INT_MAX' undeclared 
(first use in this function)
   44 |  = thread_limit > INT_MAX ? UINT_MAX : thread_limit;
  |   ^~~
/vol/gcc/src/hg/trunk/local/libgomp/teams.c:29:1: note: 'INT_MAX' is defined in 
header ''; did you forget to '#include '?
   28 | #include "libgomp.h"
  +++ |+#include 
   29 |
/vol/gcc/src/hg/trunk/local/libgomp/teams.c:44:19: note: each undeclared 
identifier is reported only once for each function it appears in
   44 |  = thread_limit > INT_MAX ? UINT_MAX : thread_limit;
  |   ^~~
/vol/gcc/src/hg/trunk/local/libgomp/teams.c:44:29: error: 'UINT_MAX' undeclared 
(first use in this function)
   44 |  = thread_limit > INT_MAX ? UINT_MAX : thread_limit;
  | ^~~~

The patch below fixes this and allows the build to continue on
i386-pc-solaris2.1[01] and sparc-sun-solaris2.11.  Also built on
x86_64-pc-linux-gnu.

I guess this is obvious?

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


2018-11-08  Rainer Orth  

* affinity.c: Include , .
(gomp_display_affinity_place): Remove cpusetp.
* teams.c: Include .

# HG changeset patch
# Parent  84ce7857bee0cad12f0066cd39dcdf95644c1a6d
Fix Solaris build with OpenMP 5.0

diff --git a/libgomp/affinity.c b/libgomp/affinity.c
--- a/libgomp/affinity.c
+++ b/libgomp/affinity.c
@@ -26,6 +26,8 @@
 /* This is a generic stub implementation of a CPU affinity setting.  */
 
 #include "libgomp.h"
+#include 
+#include 
 
 void
 gomp_init_affinity (void)
@@ -142,7 +144,6 @@ void
 gomp_display_affinity_place (char *buffer, size_t size, size_t *ret,
 			 int place)
 {
-  cpu_set_t *cpusetp;
   char buf[sizeof (long) * 3 + 4];
   if (gomp_available_cpus > 1)
 sprintf (buf, "0-%lu", gomp_available_cpus - 1);
diff --git a/libgomp/teams.c b/libgomp/teams.c
--- a/libgomp/teams.c
+++ b/libgomp/teams.c
@@ -26,6 +26,7 @@
 /* This file handles the host TEAMS construct.  */
 
 #include "libgomp.h"
+#include 
 
 static unsigned gomp_num_teams = 1, gomp_team_num = 0;
 

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Jakub Jelinek]

On Thu, Nov 08, 2018 at 09:39:12PM +0100, Rainer Orth wrote:
> I guess this is obvious?
> 
>   Rainer
> 
> -- 
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University
> 
> 
> 2018-11-08  Rainer Orth  
> 
>   * affinity.c: Include , .
>   (gomp_display_affinity_place): Remove cpusetp.
>   * teams.c: Include .

Ok, thanks.

Jakub

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Dominique d'Humières]

Hi Jakub,

Bootstrapping r265942 on darwin failed with

In file included from 
/Applications/Xcode-6.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/stdio.h:490,
 from ../../../work/libgomp/affinity-fmt.c:28:
../../../work/libgomp/affinity-fmt.c: In function 'gomp_display_affinity':
../../../work/libgomp/affinity-fmt.c:369:17: error: format '%x' expects 
argument of type 'unsigned int', but argument 5 has type 'long unsigned int' 
-Werror=format=]
  369 |   sprintf (buf, "0x%x", (uintptr_t) handle);
  | ^~  ~~
  | |
  | long unsigned int
../../../work/libgomp/affinity-fmt.c:369:21: note: format string is defined here
  369 |   sprintf (buf, "0x%x", (uintptr_t) handle);
  |~^
  | |
  | unsigned int
  |%lx
cc1: all warnings being treated as errors

I have managed to bootstrap with the following hack:

--- ../_clean/libgomp/affinity-fmt.c2018-11-08 19:03:37.0 +0100
+++ libgomp/affinity-fmt.c  2018-11-09 01:00:16.0 +0100
@@ -362,11 +362,11 @@ gomp_display_affinity (char *buffer, siz
  char buf[3 * (sizeof (handle) + sizeof (int)) + 4];
 
  if (sizeof (handle) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) handle);
+   sprintf (buf, "0x%lx", (long) (uintptr_t) handle);
  else if (sizeof (handle) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) handle);
+   sprintf (buf, "0x%llx", (long long) (uintptr_t) handle);
  else
-   sprintf (buf, "0x%x", (int) handle);
+   sprintf (buf, "0x%x", (int) (uintptr_t) handle);
  gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
  break;
}

which is certainly wrong, but allowed me to bootstrap.

TIA

Dominique

PS I can file a PR if necessary.

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Jakub Jelinek]

On Fri, Nov 09, 2018 at 11:34:48AM +0100, Dominique d'Humières wrote:
> Bootstrapping r265942 on darwin failed with
> 
> In file included from 
> /Applications/Xcode-6.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/stdio.h:490,
>  from ../../../work/libgomp/affinity-fmt.c:28:
> ../../../work/libgomp/affinity-fmt.c: In function 'gomp_display_affinity':
> ../../../work/libgomp/affinity-fmt.c:369:17: error: format '%x' expects 
> argument of type 'unsigned int', but argument 5 has type 'long unsigned int' 
> -Werror=format=]
>   369 |   sprintf (buf, "0x%x", (uintptr_t) handle);
>   | ^~  ~~

Weird, the above prints a line which just isn't there, line 369 is:
sprintf (buf, "0x%x", (int) handle);
not
sprintf (buf, "0x%x", (uintptr_t) handle);

What is pthread_t on your platform?  An integral type (which one), pointer
or something different (e.g. an aggregate)?

I wonder if I shouldn't use __builtin_choose_expr to wrap it up and select
at compile time the only variant that applies.

Something like:

--- libgomp/affinity-fmt.c  2018-10-08 13:27:41.021061844 +0200
+++ libgomp/affinity-fmt.c  2018-11-09 12:01:17.048151087 +0100
@@ -356,37 +356,39 @@ gomp_display_affinity (char *buffer, siz
  goto do_int;
case 'i':
 #if defined(LIBGOMP_USE_PTHREADS) && defined(__GNUC__)
- /* Handle integral pthread_t.  */
- if (__builtin_classify_type (handle) == 1)
-   {
- char buf[3 * (sizeof (handle) + sizeof (int)) + 4];
-
- if (sizeof (handle) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) handle);
- else if (sizeof (handle) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) handle);
- else
-   sprintf (buf, "0x%x", (int) handle);
- gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
- break;
-   }
- /* And pointer pthread_t.  */
- else if (__builtin_classify_type (handle) == 5)
-   {
- char buf[3 * (sizeof (uintptr_t) + sizeof (int)) + 4];
-
- if (sizeof (uintptr_t) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) (uintptr_t) handle);
- else if (sizeof (uintptr_t) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) (uintptr_t) handle);
- else
-   sprintf (buf, "0x%x", (int) (uintptr_t) handle);
- gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
- break;
-   }
-#endif
+ {
+   char buf[3 * (sizeof (handle) + sizeof (uintptr_t) + sizeof (int))
++ 4];
+   /* Handle integral pthread_t.  */
+   __builtin_choose_expr (__builtin_classify_type (handle) == 1,
+  sizeof (handle) == sizeof (long)
+  ? sprintf (buf, "0x%lx", (long) handle)
+  : sizeof (handle) == sizeof (long long)
+  ? sprintf (buf, "0x%llx",
+ (long long) handle)
+  : sprintf (buf, "0x%x", (int) handle),
+  0);
+   /* And pointer pthread_t.  */
+   __builtin_choose_expr (__builtin_classify_type (handle) == 5,
+  sizeof (uintptr_t) == sizeof (long)
+  ? sprintf (buf, "0x%lx",
+ (long) (uintptr_t) handle)
+  : sizeof (uintptr_t) == sizeof (long long)
+  ? sprintf (buf, "0x%llx",
+ (long long) (uintptr_t) handle)
+  : sprintf (buf, "0x%x",
+ (int) (uintptr_t) handle),
+  0);
+   if (__builtin_classify_type (handle) != 1
+   && __builtin_classify_type (handle) != 5)
+ strcpy (buf, "0");
+   gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
+   break;
+ }
+#else
  val = 0;
  goto do_int;
+#endif
case 'A':
  if (sz == (size_t) -1)
gomp_display_affinity_place (buffer, size, &ret,



Jakub

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[Jakub Jelinek]

On Fri, Nov 09, 2018 at 12:04:54PM +0100, Jakub Jelinek wrote:
> On Fri, Nov 09, 2018 at 11:34:48AM +0100, Dominique d'Humières wrote:
> > Bootstrapping r265942 on darwin failed with
> > 
> > In file included from 
> > /Applications/Xcode-6.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/stdio.h:490,
> >  from ../../../work/libgomp/affinity-fmt.c:28:
> > ../../../work/libgomp/affinity-fmt.c: In function 'gomp_display_affinity':
> > ../../../work/libgomp/affinity-fmt.c:369:17: error: format '%x' expects 
> > argument of type 'unsigned int', but argument 5 has type 'long unsigned 
> > int' -Werror=format=]
> >   369 |   sprintf (buf, "0x%x", (uintptr_t) handle);
> >   | ^~  ~~
> 
> Weird, the above prints a line which just isn't there, line 369 is:
> sprintf (buf, "0x%x", (int) handle);
> not
> sprintf (buf, "0x%x", (uintptr_t) handle);
> 
> What is pthread_t on your platform?  An integral type (which one), pointer
> or something different (e.g. an aggregate)?

The "but argument 5" in there is also weird, sprintf has 3 arguments.
Doesn't current gcc emit warnings like:
/tmp/j.c: In function ‘main’:
/tmp/j.c:8:21: warning: format ‘%x’ expects argument of type ‘unsigned int’, 
but argument 3 has type ‘long unsigned int’ [-Wformat=]
8 |   sprintf (buf, "0x%x", l);
  |~^   ~
  | |   |
  | |   long unsigned int
  | unsigned int
  |%lx
?
So, what exact compiler is compiling this code?

Jakub

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[David Malcolm]

On Fri, 2018-11-09 at 12:11 +0100, Jakub Jelinek wrote:
> On Fri, Nov 09, 2018 at 12:04:54PM +0100, Jakub Jelinek wrote:
> > On Fri, Nov 09, 2018 at 11:34:48AM +0100, Dominique d'Humières
> > wrote:
> > > Bootstrapping r265942 on darwin failed with
> > > 
> > > In file included from /Applications/Xcode-
> > > 6.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SD
> > > Ks/MacOSX10.9.sdk/usr/include/stdio.h:490,
> > >  from ../../../work/libgomp/affinity-fmt.c:28:
> > > ../../../work/libgomp/affinity-fmt.c: In function
> > > 'gomp_display_affinity':
> > > ../../../work/libgomp/affinity-fmt.c:369:17: error: format '%x'
> > > expects argument of type 'unsigned int', but argument 5 has type
> > > 'long unsigned int' -Werror=format=]
> > >   369 |   sprintf (buf, "0x%x", (uintptr_t) handle);
> > >   | ^~  ~~
> > 
> > Weird, the above prints a line which just isn't there, line 369 is:
> > sprintf (buf, "0x%x", (int) handle);
> > not
> > sprintf (buf, "0x%x", (uintptr_t) handle);
> > 
> > What is pthread_t on your platform?  An integral type (which one),
> > pointer
> > or something different (e.g. an aggregate)?
> 
> The "but argument 5" in there is also weird, sprintf has 3 arguments.
> Doesn't current gcc emit warnings like:
> /tmp/j.c: In function ‘main’:
> /tmp/j.c:8:21: warning: format ‘%x’ expects argument of type
> ‘unsigned int’, but argument 3 has type ‘long unsigned int’ [-
> Wformat=]
> 8 |   sprintf (buf, "0x%x", l);
>   |~^   ~
>   | |   |
>   | |   long unsigned int
>   | unsigned int
>   |%lx
> ?
> So, what exact compiler is compiling this code?

I agree that that looks weird.  I wonder if this is showing a bug in
the substring-locations.c or c-format.c code?  (I've changed these in
the last couple of months; maybe I messed up?)  Or is the compiler
picking up the wrong code?

The fact that it fails to use the location *within* the string in the
initial error but then prints a supporting "note" with it suggests to
me that there are macros involved.  Is sprintf a macro on this
configuration?  Maybe that's got something to do with it.

Dave

Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9

[David Edelsohn]

> 2018-11-08  Rainer Orth  
>
> * affinity.c: Include , .
> (gomp_display_affinity_place): Remove cpusetp.
> * teams.c: Include .

This fixed AIX bootstrap failure also.

Thanks, David

[PATCH] Fix up affinity-fmt.c (was Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9)

[Jakub Jelinek]

Hi!

The earlier patch doesn't work, because there were still expressions
where handle could be still cast to integer of different size if it
happened to be a pointer, or an invalid cast of e.g. aggregate to
integer.

The following patch so far only tested on a simplified code should
handle it properly though, gomp_integral should yield an integral type
without a warning (for aggregates etc. 0, but that is what I wanted to
print, don't know what else to print if pthread_t is an aggregate I have no
idea what it contains).  Plus I've added some portability stuff for mingw
%llx vs. %I64x.

Can you please give it a whirl on Darwin and see what the
display-affinity-1.c testcase prints (in libgomp/testsuite/libgomp.log) ?

Thanks.

2018-11-09  Jakub Jelinek  

* affinity-fmt.c: Include inttypes.h if HAVE_INTTYPES_H.
(gomp_display_affinity): Use __builtin_choose_expr to handle
properly handle argument having integral, or pointer or some other
type.  If inttypes.h is available and PRIx64 is defined, use PRIx64
with uint64_t type instead of %llx and unsigned long long.

--- libgomp/affinity-fmt.c.jj   2018-11-08 18:08:01.412987460 +0100
+++ libgomp/affinity-fmt.c  2018-11-09 15:24:52.049169494 +0100
@@ -30,6 +30,9 @@
 #ifdef HAVE_UNISTD_H
 #include 
 #endif
+#ifdef HAVE_INTTYPES_H
+# include   /* For PRIx64.  */
+#endif
 #ifdef HAVE_UNAME
 #include 
 #endif
@@ -356,37 +359,42 @@ gomp_display_affinity (char *buffer, siz
  goto do_int;
case 'i':
 #if defined(LIBGOMP_USE_PTHREADS) && defined(__GNUC__)
- /* Handle integral pthread_t.  */
- if (__builtin_classify_type (handle) == 1)
-   {
- char buf[3 * (sizeof (handle) + sizeof (int)) + 4];
-
- if (sizeof (handle) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) handle);
- else if (sizeof (handle) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) handle);
- else
-   sprintf (buf, "0x%x", (int) handle);
- gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
- break;
-   }
- /* And pointer pthread_t.  */
- else if (__builtin_classify_type (handle) == 5)
-   {
- char buf[3 * (sizeof (uintptr_t) + sizeof (int)) + 4];
-
- if (sizeof (uintptr_t) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) (uintptr_t) handle);
- else if (sizeof (uintptr_t) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) (uintptr_t) handle);
- else
-   sprintf (buf, "0x%x", (int) (uintptr_t) handle);
- gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
- break;
-   }
+ {
+   char buf[3 * (sizeof (handle) + sizeof (uintptr_t) + sizeof (int))
++ 4];
+   /* This macro returns expr unmodified for integral or pointer
+  types and 0 for anything else (e.g. aggregates).  */
+#define gomp_nonaggregate(expr) \
+  __builtin_choose_expr (__builtin_classify_type (expr) == 1   \
+|| __builtin_classify_type (expr) == 5, expr, 0)
+   /* This macro returns expr unmodified for integral types,
+  (uintptr_t) (expr) for pointer types and 0 for anything else
+  (e.g. aggregates).  */
+#define gomp_integral(expr) \
+  __builtin_choose_expr (__builtin_classify_type (expr) == 5,  \
+(uintptr_t) gomp_nonaggregate (expr),  \
+gomp_nonaggregate (expr))
+
+   if (sizeof (gomp_integral (handle)) == sizeof (unsigned long))
+ sprintf (buf, "0x%lx", (unsigned long) gomp_integral (handle));
+#if defined (HAVE_INTTYPES_H) && defined (PRIx64)
+   else if (sizeof (gomp_integral (handle)) == sizeof (uint64_t))
+ sprintf (buf, "0x" PRIx64, (uint64_t) gomp_integral (handle));
+#else
+   else if (sizeof (gomp_integral (handle))
+== sizeof (unsigned long long))
+ sprintf (buf, "0x%llx",
+  (unsigned long long) gomp_integral (handle));
 #endif
+   else
+ sprintf (buf, "0x%x", (unsigned int) gomp_integral (handle));
+   gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
+   break;
+ }
+#else
  val = 0;
  goto do_int;
+#endif
case 'A':
  if (sz == (size_t) -1)
gomp_display_affinity_place (buffer, size, &ret,


Jakub

Re: [PATCH] Fix up affinity-fmt.c (was Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9)

[Iain Sandoe]

Hi Jakub,


> On 9 Nov 2018, at 06:37, Jakub Jelinek  wrote:

> +#if defined (HAVE_INTTYPES_H) && defined (PRIx64)
> + else if (sizeof (gomp_integral (handle)) == sizeof (uint64_t))
> +   sprintf (buf, "0x" PRIx64, (uint64_t) gomp_integral (handle));

s/0x/0x%/?

Iain

Re: [PATCH] Fix up affinity-fmt.c (was Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9)

[Jakub Jelinek]

On Fri, Nov 09, 2018 at 07:33:44AM -0800, Iain Sandoe wrote:
> > On 9 Nov 2018, at 06:37, Jakub Jelinek  wrote:
> 
> > +#if defined (HAVE_INTTYPES_H) && defined (PRIx64)
> > +   else if (sizeof (gomp_integral (handle)) == sizeof (uint64_t))
> > + sprintf (buf, "0x" PRIx64, (uint64_t) gomp_integral (handle));
> 
> s/0x/0x%/?

Oops, consider it adjusted.  Haven't started my bootstrap/regtest of it
yet...

Thanks.

Jakub

Re: [PATCH] Fix up affinity-fmt.c (was Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9)

[Iain Sandoe]

Hi Jakub,

> On 9 Nov 2018, at 07:40, Jakub Jelinek  wrote:
> 
> On Fri, Nov 09, 2018 at 07:33:44AM -0800, Iain Sandoe wrote:
>>> On 9 Nov 2018, at 06:37, Jakub Jelinek  wrote:
>> 
>>> +#if defined (HAVE_INTTYPES_H) && defined (PRIx64)
>>> +   else if (sizeof (gomp_integral (handle)) == sizeof (uint64_t))
>>> + sprintf (buf, "0x" PRIx64, (uint64_t) gomp_integral (handle));
>> 
>> s/0x/0x%/?
> 
> Oops, consider it adjusted.  Haven't started my bootstrap/regtest of it
> yet…

Works For Me (as amended, bootstrap succeeds):

display-affinity-1.c:
libgomp: Affinity not supported on this configuration
L:0%0>xx.local   xx.local   

Re: [PATCH] Fix up affinity-fmt.c (was Re: [committed 0/4] (Partial) OpenMP 5.0 support for GCC 9)

[Jakub Jelinek]

On Fri, Nov 09, 2018 at 10:23:24AM -0800, Iain Sandoe wrote:
> Works For Me (as amended, bootstrap succeeds):
> 
> display-affinity-1.c:
> libgomp: Affinity not supported on this configuration
> L:0%0>xx.local14527_0x7fffe64ed3c0_0x7fffe64ed3c0_-01 0-7   
> L:0%0>xx.local14527_0x7fffe64ed3c0_0x7fffe64ed3c0_-01 0-7   
> %1
> 00!0!   1!4; 0;01;0;1;0-7
> 00!1!   1!4; 0;01;0;1;0-7
> 00!2!   1!4; 0;01;0;1;0-7
> 00!3!   1!4; 0;01;0;1;0-7
> 
> =

Thanks, I've also successfully bootstrapped/regtested it on x86_64-linux and
i686-linux, verified the display-affinity-1.c output on both and committed
to trunk.  Here it is again because of the missing %:

2018-11-09  Jakub Jelinek  

* affinity-fmt.c: Include inttypes.h if HAVE_INTTYPES_H.
(gomp_display_affinity): Use __builtin_choose_expr to handle
properly handle argument having integral, or pointer or some other
type.  If inttypes.h is available and PRIx64 is defined, use PRIx64
with uint64_t type instead of %llx and unsigned long long.

--- libgomp/affinity-fmt.c.jj   2018-11-08 18:08:01.412987460 +0100
+++ libgomp/affinity-fmt.c  2018-11-09 15:24:52.049169494 +0100
@@ -30,6 +30,9 @@
 #ifdef HAVE_UNISTD_H
 #include 
 #endif
+#ifdef HAVE_INTTYPES_H
+# include   /* For PRIx64.  */
+#endif
 #ifdef HAVE_UNAME
 #include 
 #endif
@@ -356,37 +359,42 @@ gomp_display_affinity (char *buffer, siz
  goto do_int;
case 'i':
 #if defined(LIBGOMP_USE_PTHREADS) && defined(__GNUC__)
- /* Handle integral pthread_t.  */
- if (__builtin_classify_type (handle) == 1)
-   {
- char buf[3 * (sizeof (handle) + sizeof (int)) + 4];
-
- if (sizeof (handle) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) handle);
- else if (sizeof (handle) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) handle);
- else
-   sprintf (buf, "0x%x", (int) handle);
- gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
- break;
-   }
- /* And pointer pthread_t.  */
- else if (__builtin_classify_type (handle) == 5)
-   {
- char buf[3 * (sizeof (uintptr_t) + sizeof (int)) + 4];
-
- if (sizeof (uintptr_t) == sizeof (long))
-   sprintf (buf, "0x%lx", (long) (uintptr_t) handle);
- else if (sizeof (uintptr_t) == sizeof (long long))
-   sprintf (buf, "0x%llx", (long long) (uintptr_t) handle);
- else
-   sprintf (buf, "0x%x", (int) (uintptr_t) handle);
- gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
- break;
-   }
+ {
+   char buf[3 * (sizeof (handle) + sizeof (uintptr_t) + sizeof (int))
++ 4];
+   /* This macro returns expr unmodified for integral or pointer
+  types and 0 for anything else (e.g. aggregates).  */
+#define gomp_nonaggregate(expr) \
+  __builtin_choose_expr (__builtin_classify_type (expr) == 1   \
+|| __builtin_classify_type (expr) == 5, expr, 0)
+   /* This macro returns expr unmodified for integral types,
+  (uintptr_t) (expr) for pointer types and 0 for anything else
+  (e.g. aggregates).  */
+#define gomp_integral(expr) \
+  __builtin_choose_expr (__builtin_classify_type (expr) == 5,  \
+(uintptr_t) gomp_nonaggregate (expr),  \
+gomp_nonaggregate (expr))
+
+   if (sizeof (gomp_integral (handle)) == sizeof (unsigned long))
+ sprintf (buf, "0x%lx", (unsigned long) gomp_integral (handle));
+#if defined (HAVE_INTTYPES_H) && defined (PRIx64)
+   else if (sizeof (gomp_integral (handle)) == sizeof (uint64_t))
+ sprintf (buf, "0x%" PRIx64, (uint64_t) gomp_integral (handle));
+#else
+   else if (sizeof (gomp_integral (handle))
+== sizeof (unsigned long long))
+ sprintf (buf, "0x%llx",
+  (unsigned long long) gomp_integral (handle));
 #endif
+   else
+ sprintf (buf, "0x%x", (unsigned int) gomp_integral (handle));
+   gomp_display_num (buffer, size, &ret, zero, right, sz, buf);
+   break;
+ }
+#else
  val = 0;
  goto do_int;
+#endif
case 'A':
  if (sz == (size_t) -1)
gomp_display_affinity_place (buffer, size, &ret,


Jakub