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

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  <jakub@redhat.com>

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



	Jakub

Hi Jakub,


> On 9 Nov 2018, at 06:37, Jakub Jelinek <jakub@redhat.com> 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

On Fri, Nov 09, 2018 at 07:33:44AM -0800, Iain Sandoe wrote:
> > On 9 Nov 2018, at 06:37, Jakub Jelinek <jakub@redhat.com> 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

Hi Jakub,

> On 9 Nov 2018, at 07:40, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Fri, Nov 09, 2018 at 07:33:44AM -0800, Iain Sandoe wrote:
>>> On 9 Nov 2018, at 06:37, Jakub Jelinek <jakub@redhat.com> 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:00000%0>xxxxxx.local               <!                xxxxxx.local! 14527_0x7fffe64ed3c0_0x00007fffe64ed3c0_-000001 0-7               
L:00000%0>xxxxxx.local               <!                xxxxxx.local! 14527_0x7fffe64ed3c0_0x00007fffe64ed3c0_-000001 0-7               
%000000001
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

=====

Iain

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:00000%0>xxxxxx.local               <!                xxxxxx.local! 14527_0x7fffe64ed3c0_0x00007fffe64ed3c0_-000001 0-7               
> L:00000%0>xxxxxx.local               <!                xxxxxx.local! 14527_0x7fffe64ed3c0_0x00007fffe64ed3c0_-000001 0-7               
> %000000001
> 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  <jakub@redhat.com>

	* 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 <unistd.h>
 #endif
+#ifdef HAVE_INTTYPES_H
+# include <inttypes.h>  /* For PRIx64.  */
+#endif
 #ifdef HAVE_UNAME
 #include <sys/utsname.h>
 #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