public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
@ 2023-07-07  7:41 Mark Geisert
  2023-07-07  9:44 ` Corinna Vinschen
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Geisert @ 2023-07-07  7:41 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Mark Geisert

The current version of <sys/cpuset.h> cannot be compiled by Clang due to
the use of __builtin* functions.  Their presence here was a dubious
optimization anyway, so their usage has been converted to standard
library functions.  A popcnt (population count of 1 bits in a word)
function is provided here because there isn't one in the standard library
or elsewhere in the Cygwin DLL.

The "#include <sys/cdefs>" here to define __inline can be removed since
both of the new includes sub-include it.

Addresses: https://cygwin.com/pipermail/cygwin/2023-July/253927.html
Fixes: 9cc910dd33a5 (Cygwin: Make <sys/cpuset.h> safe for c89 compilations)
Signed-off-by: Mark Geisert <mark@maxrnd.com>

---
 winsup/cygwin/include/sys/cpuset.h | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/winsup/cygwin/include/sys/cpuset.h b/winsup/cygwin/include/sys/cpuset.h
index 0c95134ff..f76e788d5 100644
--- a/winsup/cygwin/include/sys/cpuset.h
+++ b/winsup/cygwin/include/sys/cpuset.h
@@ -9,7 +9,8 @@ details. */
 #ifndef _SYS_CPUSET_H_
 #define _SYS_CPUSET_H_
 
-#include <sys/cdefs.h>
+#include <stdlib.h>
+#include <string.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -31,6 +32,23 @@ typedef struct
 #if __GNU_VISIBLE
 int __sched_getaffinity_sys (pid_t, size_t, cpu_set_t *);
 
+/* Modern CPUs have popcnt* instructions but the need here is not worth
+ * worrying about builtins or inline assembler for different compilers. */ 
+static inline int
+__maskpopcnt (__cpu_mask mask)
+{
+  int res = 0;
+  unsigned long ulmask = (unsigned long) mask;
+
+  while (ulmask != 0)
+    {
+      if (ulmask & 1)
+        ++res;
+      ulmask >>= 1;
+    }
+  return res;
+}
+
 /* These macros alloc or free dynamically-sized cpu sets of size 'num' cpus.
    Allocations are padded such that full-word operations can be done easily. */
 #define CPU_ALLOC_SIZE(num) __cpuset_alloc_size (num)
@@ -44,14 +62,14 @@ __cpuset_alloc_size (int num)
 static __inline cpu_set_t *
 __cpuset_alloc (int num)
 {
-  return (cpu_set_t *) __builtin_malloc (CPU_ALLOC_SIZE(num));
+  return (cpu_set_t *) malloc (CPU_ALLOC_SIZE(num));
 }
 
 #define CPU_FREE(set) __cpuset_free (set)
 static __inline void
 __cpuset_free (cpu_set_t *set)
 {
-  __builtin_free (set);
+  free (set);
 }
 
 /* These _S macros operate on dynamically-sized cpu sets of size 'siz' bytes */
@@ -59,7 +77,7 @@ __cpuset_free (cpu_set_t *set)
 static __inline void
 __cpuset_zero_s (size_t siz, cpu_set_t *set)
 {
-  (void) __builtin_memset (set, 0, siz);
+  (void) memset (set, 0, siz);
 }
 
 #define CPU_SET_S(cpu, siz, set) __cpuset_set_s (cpu, siz, set)
@@ -94,7 +112,7 @@ __cpuset_count_s (size_t siz, cpu_set_t *set)
 {
   int i, res = 0;
   for (i = 0; i < siz / sizeof (__cpu_mask); i++)
-    res += __builtin_popcountl ((set)->__bits[i]);
+    res += __maskpopcnt ((set)->__bits[i]);
   return res;
 }
 
-- 
2.39.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07  7:41 [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic Mark Geisert
@ 2023-07-07  9:44 ` Corinna Vinschen
  2023-07-07 10:13   ` Mark Geisert
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Corinna Vinschen @ 2023-07-07  9:44 UTC (permalink / raw)
  To: cygwin-patches

Hi Mark,

On Jul  7 00:41, Mark Geisert wrote:
> The current version of <sys/cpuset.h> cannot be compiled by Clang due to
> the use of __builtin* functions.  Their presence here was a dubious
> optimization anyway, so their usage has been converted to standard
> library functions.  A popcnt (population count of 1 bits in a word)
> function is provided here because there isn't one in the standard library
> or elsewhere in the Cygwin DLL.

And clang really doesn't provide it?  That's unfortunate.

Do you really think it's not worth to use it if it's available?

You could workaround it like this:

> +/* Modern CPUs have popcnt* instructions but the need here is not worth
> + * worrying about builtins or inline assembler for different compilers. */ 
> +static inline int
> +__maskpopcnt (__cpu_mask mask)
> +{
#if (__GNUC__ >= 4)
     return __builtin_popcountl (mask);
#else
> +  int res = 0;
> +  unsigned long ulmask = (unsigned long) mask;
> +
> +  while (ulmask != 0)
> +    {
> +      if (ulmask & 1)
> +        ++res;
> +      ulmask >>= 1;
> +    }
> +  return res;
#endif
> +}
> +

But, if you think that's not worth it, I'll push your patch as is.


Thanks,
Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07  9:44 ` Corinna Vinschen
@ 2023-07-07 10:13   ` Mark Geisert
  2023-07-07 10:45     ` Corinna Vinschen
  2023-07-07 15:46   ` Jon Turney
  2023-07-07 18:54   ` Brian Inglis
  2 siblings, 1 reply; 12+ messages in thread
From: Mark Geisert @ 2023-07-07 10:13 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

Corinna Vinschen wrote:
> On Jul  7 00:41, Mark Geisert wrote:
>> The current version of <sys/cpuset.h> cannot be compiled by Clang due to
>> the use of __builtin* functions.  Their presence here was a dubious
>> optimization anyway, so their usage has been converted to standard
>> library functions.  A popcnt (population count of 1 bits in a word)
>> function is provided here because there isn't one in the standard library
>> or elsewhere in the Cygwin DLL.
> 
> And clang really doesn't provide it?  That's unfortunate.
> 
> Do you really think it's not worth to use it if it's available?

I don't know for sure.  I'd guess the popcnt op should be optimized if available; 
the others probably don't need it.

> You could workaround it like this:
> 
>> +/* Modern CPUs have popcnt* instructions but the need here is not worth
>> + * worrying about builtins or inline assembler for different compilers. */
>> +static inline int
>> +__maskpopcnt (__cpu_mask mask)
>> +{
> #if (__GNUC__ >= 4)
>       return __builtin_popcountl (mask);
> #else
>> +  int res = 0;
>> +  unsigned long ulmask = (unsigned long) mask;
>> +
>> +  while (ulmask != 0)
>> +    {
>> +      if (ulmask & 1)
>> +        ++res;
>> +      ulmask >>= 1;
>> +    }
>> +  return res;
> #endif
>> +}
>> +

The first version of the patch (unsubmitted) worked something like that, though it 
was a chore figuring out how to tell the difference between gcc and clang.  clang 
#defines __GNUC__ (?!) for example.  I ended up using __GNUC_PREREQ__ with the 
hope clang version numbers stay lower than gcc version numbers.  Has to be a 
better way than that.

On the other hand, one compilation with clang or clang++, I forget which, and with 
some optimization flag, recognized the 'while' loop in that function and turned it 
into the Hackers Delight algorithm for popcnt in ~20 instructions and no loop.

TL;DR let me ponder this over the weekend.
Thanks for listening,

..mark


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07 10:13   ` Mark Geisert
@ 2023-07-07 10:45     ` Corinna Vinschen
  0 siblings, 0 replies; 12+ messages in thread
From: Corinna Vinschen @ 2023-07-07 10:45 UTC (permalink / raw)
  To: cygwin-patches

On Jul  7 03:13, Mark Geisert wrote:
> Hi Corinna,
> 
> Corinna Vinschen wrote:
> > On Jul  7 00:41, Mark Geisert wrote:
> > > The current version of <sys/cpuset.h> cannot be compiled by Clang due to
> > > the use of __builtin* functions.  Their presence here was a dubious
> > > optimization anyway, so their usage has been converted to standard
> > > library functions.  A popcnt (population count of 1 bits in a word)
> > > function is provided here because there isn't one in the standard library
> > > or elsewhere in the Cygwin DLL.
> > 
> > And clang really doesn't provide it?  That's unfortunate.
> > 
> > Do you really think it's not worth to use it if it's available?
> 
> I don't know for sure.  I'd guess the popcnt op should be optimized if
> available; the others probably don't need it.
> 
> > You could workaround it like this:
> > 
> > > +/* Modern CPUs have popcnt* instructions but the need here is not worth
> > > + * worrying about builtins or inline assembler for different compilers. */
> > > +static inline int
> > > +__maskpopcnt (__cpu_mask mask)
> > > +{
> > #if (__GNUC__ >= 4)
> >       return __builtin_popcountl (mask);
> > #else
> > > +  int res = 0;
> > > +  unsigned long ulmask = (unsigned long) mask;
> > > +
> > > +  while (ulmask != 0)
> > > +    {
> > > +      if (ulmask & 1)
> > > +        ++res;
> > > +      ulmask >>= 1;
> > > +    }
> > > +  return res;
> > #endif
> > > +}
> > > +
> 
> The first version of the patch (unsubmitted) worked something like that,
> though it was a chore figuring out how to tell the difference between gcc
> and clang.  clang #defines __GNUC__ (?!) for example.  I ended up using

Oh well...

> __GNUC_PREREQ__ with the hope clang version numbers stay lower than gcc
> version numbers.  Has to be a better way than that.
> 
> On the other hand, one compilation with clang or clang++, I forget which,
> and with some optimization flag, recognized the 'while' loop in that
> function and turned it into the Hackers Delight algorithm for popcnt in ~20
> instructions and no loop.
> 
> TL;DR let me ponder this over the weekend.
> Thanks for listening,

No worries,
Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07  9:44 ` Corinna Vinschen
  2023-07-07 10:13   ` Mark Geisert
@ 2023-07-07 15:46   ` Jon Turney
  2023-07-07 15:49     ` Corinna Vinschen
  2023-07-07 18:54   ` Brian Inglis
  2 siblings, 1 reply; 12+ messages in thread
From: Jon Turney @ 2023-07-07 15:46 UTC (permalink / raw)
  To: Cygwin Patches

On 07/07/2023 10:44, Corinna Vinschen wrote:
> Hi Mark,
> 
> On Jul  7 00:41, Mark Geisert wrote:
>> The current version of <sys/cpuset.h> cannot be compiled by Clang due to
>> the use of __builtin* functions.  Their presence here was a dubious
>> optimization anyway, so their usage has been converted to standard
>> library functions.  A popcnt (population count of 1 bits in a word)
>> function is provided here because there isn't one in the standard library
>> or elsewhere in the Cygwin DLL.
> 
> And clang really doesn't provide it?  That's unfortunate.

I suspect if we had a current clang this would not be a problem.

There's probably something to be said for solving this problem by 
removing our old and unmaintained clang packages...


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07 15:46   ` Jon Turney
@ 2023-07-07 15:49     ` Corinna Vinschen
  0 siblings, 0 replies; 12+ messages in thread
From: Corinna Vinschen @ 2023-07-07 15:49 UTC (permalink / raw)
  To: cygwin-patches

On Jul  7 16:46, Jon Turney wrote:
> On 07/07/2023 10:44, Corinna Vinschen wrote:
> > Hi Mark,
> > 
> > On Jul  7 00:41, Mark Geisert wrote:
> > > The current version of <sys/cpuset.h> cannot be compiled by Clang due to
> > > the use of __builtin* functions.  Their presence here was a dubious
> > > optimization anyway, so their usage has been converted to standard
> > > library functions.  A popcnt (population count of 1 bits in a word)
> > > function is provided here because there isn't one in the standard library
> > > or elsewhere in the Cygwin DLL.
> > 
> > And clang really doesn't provide it?  That's unfortunate.
> 
> I suspect if we had a current clang this would not be a problem.
> 
> There's probably something to be said for solving this problem by removing
> our old and unmaintained clang packages...

Good point.


Corinna

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07  9:44 ` Corinna Vinschen
  2023-07-07 10:13   ` Mark Geisert
  2023-07-07 15:46   ` Jon Turney
@ 2023-07-07 18:54   ` Brian Inglis
  2023-07-08 19:22     ` Brian Inglis
  2 siblings, 1 reply; 12+ messages in thread
From: Brian Inglis @ 2023-07-07 18:54 UTC (permalink / raw)
  To: cygwin-patches

On 2023-07-07 03:44, Corinna Vinschen wrote:
> Hi Mark,
> 
> On Jul  7 00:41, Mark Geisert wrote:
>> The current version of <sys/cpuset.h> cannot be compiled by Clang due to
>> the use of __builtin* functions.  Their presence here was a dubious
>> optimization anyway, so their usage has been converted to standard
>> library functions.  A popcnt (population count of 1 bits in a word)
>> function is provided here because there isn't one in the standard library
>> or elsewhere in the Cygwin DLL.
> 
> And clang really doesn't provide it?  That's unfortunate.
> 
> Do you really think it's not worth to use it if it's available?
> 
> You could workaround it like this:
> 
>> +/* Modern CPUs have popcnt* instructions but the need here is not worth
>> + * worrying about builtins or inline assembler for different compilers. */
>> +static inline int
>> +__maskpopcnt (__cpu_mask mask)
>> +{
> #if (__GNUC__ >= 4)
>       return __builtin_popcountl (mask);
> #else
>> +  int res = 0;
>> +  unsigned long ulmask = (unsigned long) mask;
>> +
>> +  while (ulmask != 0)
>> +    {
>> +      if (ulmask & 1)
>> +        ++res;
>> +      ulmask >>= 1;
>> +    }
>> +  return res;
> #endif
>> +}
>> +
> 
> But, if you think that's not worth it, I'll push your patch as is.

In the cygwin list clang iostream discussion, I pointed out the clang 
configuration using gcc default paths and skipping the provided equivalent clang 
libraries including package libclang8 for clang intrinsics and builtins in:

	/usr/lib/clang/8.0.1/include/

which appear very similar to those provided by gcc-core in:

	/usr/lib/gcc/x86_64-pc-cygwin/11/include/

e.g.

$ l /usr/lib/{gcc/x86_64-pc-cygwin/11,clang/8.0.1}/include/*popcnt*
/usr/lib/clang/8.0.1/include/avx512vpopcntdqintrin.h
/usr/lib/clang/8.0.1/include/avx512vpopcntdqvlintrin.h
/usr/lib/clang/8.0.1/include/popcntintrin.h
/usr/lib/gcc/x86_64-pc-cygwin/11/include/avx512vpopcntdqintrin.h
/usr/lib/gcc/x86_64-pc-cygwin/11/include/avx512vpopcntdqvlintrin.h
/usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h

and from comparing features, the gcc test will work just fine with the clang 
compiler builtins or the clang intrinsics:

$ grep 'popc\(ou\)\?nt' 
/usr/lib/{gcc/x86_64-pc-cygwin/11,clang/8.0.1}/include/popcnt*
/usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:#pragma GCC target("popcnt")
/usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:_mm_popcnt_u32 (unsigned 
int __X)
/usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:  return 
__builtin_popcount (__X);
/usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:_mm_popcnt_u64 (unsigned 
long long __X)
/usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:  return 
__builtin_popcountll (__X);
/usr/lib/clang/8.0.1/include/popcntintrin.h:/*===---- popcntintrin.h - POPCNT 
intrinsics -------------------------------===
/usr/lib/clang/8.0.1/include/popcntintrin.h:#define __DEFAULT_FN_ATTRS 
__attribute__((__always_inline__, __nodebug__, __target__("popcnt")))
/usr/lib/clang/8.0.1/include/popcntintrin.h:_mm_popcnt_u32(unsigned int __A)
/usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcount(__A);
/usr/lib/clang/8.0.1/include/popcntintrin.h:_popcnt32(int __A)
/usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcount(__A);
/usr/lib/clang/8.0.1/include/popcntintrin.h:_mm_popcnt_u64(unsigned long long __A)
/usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcountll(__A);
/usr/lib/clang/8.0.1/include/popcntintrin.h:_popcnt64(long long __A)
/usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcountll(__A);

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-07 18:54   ` Brian Inglis
@ 2023-07-08 19:22     ` Brian Inglis
  2023-07-08 20:59       ` Mark Geisert
  0 siblings, 1 reply; 12+ messages in thread
From: Brian Inglis @ 2023-07-08 19:22 UTC (permalink / raw)
  To: cygwin-patches

On 2023-07-07 12:54, Brian Inglis wrote:
> On 2023-07-07 03:44, Corinna Vinschen wrote:
>> Hi Mark,
>>
>> On Jul  7 00:41, Mark Geisert wrote:
>>> The current version of <sys/cpuset.h> cannot be compiled by Clang due to
>>> the use of __builtin* functions.  Their presence here was a dubious
>>> optimization anyway, so their usage has been converted to standard
>>> library functions.  A popcnt (population count of 1 bits in a word)
>>> function is provided here because there isn't one in the standard library
>>> or elsewhere in the Cygwin DLL.
>>
>> And clang really doesn't provide it?  That's unfortunate.
>>
>> Do you really think it's not worth to use it if it's available?
>>
>> You could workaround it like this:
>>
>>> +/* Modern CPUs have popcnt* instructions but the need here is not worth
>>> + * worrying about builtins or inline assembler for different compilers. */
>>> +static inline int
>>> +__maskpopcnt (__cpu_mask mask)
>>> +{
>> #if (__GNUC__ >= 4)
>>       return __builtin_popcountl (mask);

Missed the difference in spelling, but clang supports the same builtin functions 
__builtin_popcount{,l,ll} et. al. or provides *optimized* inline functions if 
not directly available as an instruction on the architecture.

>> #else
>>> +  int res = 0;
>>> +  unsigned long ulmask = (unsigned long) mask;
>>> +
>>> +  while (ulmask != 0)
>>> +    {
>>> +      if (ulmask & 1)
>>> +        ++res;
>>> +      ulmask >>= 1;
>>> +    }
>>> +  return res;
>> #endif
>>> +}
>>> +
>>
>> But, if you think that's not worth it, I'll push your patch as is.
> 
> In the cygwin list clang iostream discussion, I pointed out the clang 
> configuration using gcc default paths and skipping the provided equivalent clang 
> libraries including package libclang8 for clang intrinsics and builtins in:
> 
>      /usr/lib/clang/8.0.1/include/
> 
> which appear very similar to those provided by gcc-core in:
> 
>      /usr/lib/gcc/x86_64-pc-cygwin/11/include/
> 
> e.g.
> 
> $ l /usr/lib/{gcc/x86_64-pc-cygwin/11,clang/8.0.1}/include/*popcnt*
> /usr/lib/clang/8.0.1/include/avx512vpopcntdqintrin.h
> /usr/lib/clang/8.0.1/include/avx512vpopcntdqvlintrin.h
> /usr/lib/clang/8.0.1/include/popcntintrin.h
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/avx512vpopcntdqintrin.h
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/avx512vpopcntdqvlintrin.h
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h
> 
> and from comparing features, the gcc test will work just fine with the clang 
> compiler builtins or the clang intrinsics:
> 
> $ grep 'popc\(ou\)\?nt' 
> /usr/lib/{gcc/x86_64-pc-cygwin/11,clang/8.0.1}/include/popcnt*
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:#pragma GCC 
> target("popcnt")
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:_mm_popcnt_u32 (unsigned 
> int __X)
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:  return 
> __builtin_popcount (__X);
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:_mm_popcnt_u64 (unsigned 
> long long __X)
> /usr/lib/gcc/x86_64-pc-cygwin/11/include/popcntintrin.h:  return 
> __builtin_popcountll (__X);
> /usr/lib/clang/8.0.1/include/popcntintrin.h:/*===---- popcntintrin.h - POPCNT 
> intrinsics -------------------------------===
> /usr/lib/clang/8.0.1/include/popcntintrin.h:#define __DEFAULT_FN_ATTRS 
> __attribute__((__always_inline__, __nodebug__, __target__("popcnt")))
> /usr/lib/clang/8.0.1/include/popcntintrin.h:_mm_popcnt_u32(unsigned int __A)
> /usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcount(__A);
> /usr/lib/clang/8.0.1/include/popcntintrin.h:_popcnt32(int __A)
> /usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcount(__A);
> /usr/lib/clang/8.0.1/include/popcntintrin.h:_mm_popcnt_u64(unsigned long long __A)
> /usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcountll(__A);
> /usr/lib/clang/8.0.1/include/popcntintrin.h:_popcnt64(long long __A)
> /usr/lib/clang/8.0.1/include/popcntintrin.h:  return __builtin_popcountll(__A);

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-08 19:22     ` Brian Inglis
@ 2023-07-08 20:59       ` Mark Geisert
  2023-07-08 21:53         ` Mark Geisert
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Geisert @ 2023-07-08 20:59 UTC (permalink / raw)
  To: cygwin-patches

Brian Inglis wrote:
> On 2023-07-07 12:54, Brian Inglis wrote:
>> On 2023-07-07 03:44, Corinna Vinschen wrote:
>>> Hi Mark,
>>>
>>> On Jul  7 00:41, Mark Geisert wrote:
>>>> The current version of <sys/cpuset.h> cannot be compiled by Clang due to
>>>> the use of __builtin* functions.  Their presence here was a dubious
>>>> optimization anyway, so their usage has been converted to standard
>>>> library functions.  A popcnt (population count of 1 bits in a word)
>>>> function is provided here because there isn't one in the standard library
>>>> or elsewhere in the Cygwin DLL.
>>>
>>> And clang really doesn't provide it?  That's unfortunate.
>>>
>>> Do you really think it's not worth to use it if it's available?
>>>
>>> You could workaround it like this:
>>>
>>>> +/* Modern CPUs have popcnt* instructions but the need here is not worth
>>>> + * worrying about builtins or inline assembler for different compilers. */
>>>> +static inline int
>>>> +__maskpopcnt (__cpu_mask mask)
>>>> +{
>>> #if (__GNUC__ >= 4)
>>>       return __builtin_popcountl (mask);
> 
> Missed the difference in spelling, but clang supports the same builtin functions 
> __builtin_popcount{,l,ll} et. al. or provides *optimized* inline functions if not 
> directly available as an instruction on the architecture.

Clang in its current version 16.0.x has __builtin_popcount*, but not in the 
ancient version 8.0.1 that is currently packaged for Cygwin.  I think that's what 
was behind Jon's earlier comment that perhaps we should remove clang from Cygwin 
unless/until somebody can adopt and maintain an up-to-date version.  :-(.

..mark

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-08 20:59       ` Mark Geisert
@ 2023-07-08 21:53         ` Mark Geisert
  2023-07-09  4:27           ` Brian Inglis
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Geisert @ 2023-07-08 21:53 UTC (permalink / raw)
  To: cygwin-patches

Mark Geisert wrote:
[... blah blah ...]

I got tripped up by misspelling and not being able to link clang{,++} programs on 
my test system.  I checked the .o files with objdump: Clang and clang++ both 
support __builtin_popcountl, but they emit code for the Hackers Delight algorithm 
rather than using the single-instruction popcnt.  Sorry for the confustion [sic].

..mark


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-08 21:53         ` Mark Geisert
@ 2023-07-09  4:27           ` Brian Inglis
  2023-07-09  7:58             ` Mark Geisert
  0 siblings, 1 reply; 12+ messages in thread
From: Brian Inglis @ 2023-07-09  4:27 UTC (permalink / raw)
  To: cygwin-patches

On 2023-07-08 15:53, Mark Geisert wrote:
> Mark Geisert wrote:
> I got tripped up by misspelling and not being able to link clang{,++} programs 
> on my test system.  I checked the .o files with objdump: Clang and clang++ both 
> support __builtin_popcountl, but they emit code for the Hackers Delight 
> algorithm rather than using the single-instruction popcnt.  Sorry for the 
> confustion [sic].

That's what I meant - clang 8 "identifies as" gcc 4, and builtin and intrinsic 
function support are almost the same (and fairly close to gcc 11) builtin and 
intrinsic function support.

And as you mentioned, any support for builtins is better than what we can whip 
up off the top of our heads (unless you use HD/2!)

For our purposes, the main differences between clang 8 and current are latest 
language, library, and processor support, but it also supports useful tools like 
the analyzer and formatter, which gcc does not provide.

And it is convenient to be able to run another compiler side by side for 
comparisons without copying files and remoting to another system.

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic
  2023-07-09  4:27           ` Brian Inglis
@ 2023-07-09  7:58             ` Mark Geisert
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Geisert @ 2023-07-09  7:58 UTC (permalink / raw)
  To: cygwin-patches

Hi all,

Brian Inglis wrote:
> On 2023-07-08 15:53, Mark Geisert wrote:
>> Mark Geisert wrote:
>> I got tripped up by misspelling and not being able to link clang{,++} programs 
>> on my test system.  I checked the .o files with objdump: Clang and clang++ both 
>> support __builtin_popcountl, but they emit code for the Hackers Delight 
>> algorithm rather than using the single-instruction popcnt.  Sorry for the 
>> confustion [sic].
> 
> That's what I meant - clang 8 "identifies as" gcc 4, and builtin and intrinsic 
> function support are almost the same (and fairly close to gcc 11) builtin and 
> intrinsic function support.
> 
> And as you mentioned, any support for builtins is better than what we can whip up 
> off the top of our heads (unless you use HD/2!)
> 
> For our purposes, the main differences between clang 8 and current are latest 
> language, library, and processor support, but it also supports useful tools like 
> the analyzer and formatter, which gcc does not provide.
> 
> And it is convenient to be able to run another compiler side by side for 
> comparisons without copying files and remoting to another system.

I now understand what you were getting at.  We are in agreement.  Thanks to your 
persistence and everybody elses comments and to my belatedly getting a working 
clang/clang++ build environment, there is a much smaller v2 patch incoming.
Thanks and Regards All,

..mark


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-07-09  7:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-07  7:41 [PATCH] Cygwin: Make gcc-specific code in <sys/cpuset.h> compiler-agnostic Mark Geisert
2023-07-07  9:44 ` Corinna Vinschen
2023-07-07 10:13   ` Mark Geisert
2023-07-07 10:45     ` Corinna Vinschen
2023-07-07 15:46   ` Jon Turney
2023-07-07 15:49     ` Corinna Vinschen
2023-07-07 18:54   ` Brian Inglis
2023-07-08 19:22     ` Brian Inglis
2023-07-08 20:59       ` Mark Geisert
2023-07-08 21:53         ` Mark Geisert
2023-07-09  4:27           ` Brian Inglis
2023-07-09  7:58             ` Mark Geisert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).