* [PATCH] Properly check __cpu_mask size [BZ #21696]
@ 2017-07-01 1:14 H.J. Lu
2017-07-01 6:35 ` Florian Weimer
0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2017-07-01 1:14 UTC (permalink / raw)
To: GNU C Library
posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
which is incorrect for x32. This patch generates size of __cpu_mask and
uses it in posix/sched_cpucount.c.
Tested on i686, x86-64 and x32 with multi-arch disabled.
OK for master?
H.J.
[BZ #21696]
* posix/Makefile (gen-as-const-headers): New.
* posix/cpu_mask.sym: New file.
* posix/sched_cpucount.c: Include <cpu_mask.h>.
(__sched_cpucount): Check CPU_MASK_SIZE instead of LONG_BIT.
Replace the ul suffix with the ull suffix for 64-bit cpu_mask.
---
posix/Makefile | 2 ++
posix/cpu_mask.sym | 5 +++++
posix/sched_cpucount.c | 15 ++++++++-------
3 files changed, 15 insertions(+), 7 deletions(-)
create mode 100644 posix/cpu_mask.sym
diff --git a/posix/Makefile b/posix/Makefile
index 1c328b8..963cba8 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -144,6 +144,8 @@ tests-special += $(objpfx)bug-regex2-mem.out $(objpfx)bug-regex14-mem.out \
xtests-special += $(objpfx)bug-ga2-mem.out
endif
+gen-as-const-headers = cpu_mask.sym
+
include ../Rules
ifeq ($(run-built-tests),yes)
diff --git a/posix/cpu_mask.sym b/posix/cpu_mask.sym
new file mode 100644
index 0000000..33519e6
--- /dev/null
+++ b/posix/cpu_mask.sym
@@ -0,0 +1,5 @@
+#include <limits.h>
+#include <sched.h>
+
+--
+CPU_MASK_SIZE sizeof (__cpu_mask)
diff --git a/posix/sched_cpucount.c b/posix/sched_cpucount.c
index eaddf61..e3622b8 100644
--- a/posix/sched_cpucount.c
+++ b/posix/sched_cpucount.c
@@ -17,6 +17,7 @@
#include <limits.h>
#include <sched.h>
+#include <cpu_mask.h>
int
@@ -36,13 +37,13 @@ __sched_cpucount (size_t setsize, const cpu_set_t *setp)
if (l == 0)
continue;
-# if LONG_BIT > 32
- l = (l & 0x5555555555555555ul) + ((l >> 1) & 0x5555555555555555ul);
- l = (l & 0x3333333333333333ul) + ((l >> 2) & 0x3333333333333333ul);
- l = (l & 0x0f0f0f0f0f0f0f0ful) + ((l >> 4) & 0x0f0f0f0f0f0f0f0ful);
- l = (l & 0x00ff00ff00ff00fful) + ((l >> 8) & 0x00ff00ff00ff00fful);
- l = (l & 0x0000ffff0000fffful) + ((l >> 16) & 0x0000ffff0000fffful);
- l = (l & 0x00000000fffffffful) + ((l >> 32) & 0x00000000fffffffful);
+# if CPU_MASK_SIZE > 4
+ l = (l & 0x5555555555555555ull) + ((l >> 1) & 0x5555555555555555ull);
+ l = (l & 0x3333333333333333ull) + ((l >> 2) & 0x3333333333333333ull);
+ l = (l & 0x0f0f0f0f0f0f0f0full) + ((l >> 4) & 0x0f0f0f0f0f0f0f0full);
+ l = (l & 0x00ff00ff00ff00ffull) + ((l >> 8) & 0x00ff00ff00ff00ffull);
+ l = (l & 0x0000ffff0000ffffull) + ((l >> 16) & 0x0000ffff0000ffffull);
+ l = (l & 0x00000000ffffffffull) + ((l >> 32) & 0x00000000ffffffffull);
# else
l = (l & 0x55555555ul) + ((l >> 1) & 0x55555555ul);
l = (l & 0x33333333ul) + ((l >> 2) & 0x33333333ul);
--
2.9.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Properly check __cpu_mask size [BZ #21696]
2017-07-01 1:14 [PATCH] Properly check __cpu_mask size [BZ #21696] H.J. Lu
@ 2017-07-01 6:35 ` Florian Weimer
2017-07-01 14:12 ` [PATCH] Use __builtin_popcount in __sched_cpucount " H.J. Lu
0 siblings, 1 reply; 5+ messages in thread
From: Florian Weimer @ 2017-07-01 6:35 UTC (permalink / raw)
To: H.J. Lu, GNU C Library
On 07/01/2017 03:14 AM, H.J. Lu wrote:
> posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
> which is incorrect for x32. This patch generates size of __cpu_mask and
> uses it in posix/sched_cpucount.c.
>
> Tested on i686, x86-64 and x32 with multi-arch disabled.
>
> OK for master?
>
> H.J.
> [BZ #21696]
> * posix/Makefile (gen-as-const-headers): New.
> * posix/cpu_mask.sym: New file.
> * posix/sched_cpucount.c: Include <cpu_mask.h>.
> (__sched_cpucount): Check CPU_MASK_SIZE instead of LONG_BIT.
> Replace the ul suffix with the ull suffix for 64-bit cpu_mask.
Does one of the existing test cases fail reliably due to this?
> +gen-as-const-headers = cpu_mask.sym
This is not needed if you use a regular #if and not a preprocessor
conditional.
_Static_assert (sizeof (l) == sizeof (unsigned int)
|| sizeof (l) == sizeof (unsigned long)
|| sizeof (l) == sizeof (unsigned long long),
"sizeof (__cpu_mask"));
if (sizeof (__cpu_mask) == sizeof (unsigned int))
s += __builtin_popcount (l);
else if (sizeof (__cpu_mask) == sizeof (unsigned long))
s += __builtin_popcountl (l);
else
s += __builtin_popcountll (l);
Untested, but __builtin_popcount⦠has fallback emulation GCC, so this
work. (And GCC 4.8 already has it.)
Thanks,
Florian
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Use __builtin_popcount in __sched_cpucount [BZ #21696]
2017-07-01 6:35 ` Florian Weimer
@ 2017-07-01 14:12 ` H.J. Lu
2017-07-01 14:26 ` Florian Weimer
0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2017-07-01 14:12 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Sat, Jul 01, 2017 at 08:33:19AM +0200, Florian Weimer wrote:
> On 07/01/2017 03:14 AM, H.J. Lu wrote:
> > posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
> > which is incorrect for x32. This patch generates size of __cpu_mask and
> > uses it in posix/sched_cpucount.c.
> >
> > Tested on i686, x86-64 and x32 with multi-arch disabled.
> >
> > OK for master?
> >
> > H.J.
> > [BZ #21696]
> > * posix/Makefile (gen-as-const-headers): New.
> > * posix/cpu_mask.sym: New file.
> > * posix/sched_cpucount.c: Include <cpu_mask.h>.
> > (__sched_cpucount): Check CPU_MASK_SIZE instead of LONG_BIT.
> > Replace the ul suffix with the ull suffix for 64-bit cpu_mask.
>
> Does one of the existing test cases fail reliably due to this?
>
Yes, posix/tst-cpuset and posix/tst-cpucount fail when multi-arch is
disabled or the processor doesn't have popcnt. Otherwise, popcnt
instruction is used.
> > +gen-as-const-headers = cpu_mask.sym
>
> This is not needed if you use a regular #if and not a preprocessor
> conditional.
>
> _Static_assert (sizeof (l) == sizeof (unsigned int)
> || sizeof (l) == sizeof (unsigned long)
> || sizeof (l) == sizeof (unsigned long long),
> "sizeof (__cpu_mask"));
> if (sizeof (__cpu_mask) == sizeof (unsigned int))
> s += __builtin_popcount (l);
> else if (sizeof (__cpu_mask) == sizeof (unsigned long))
> s += __builtin_popcountl (l);
> else
> s += __builtin_popcountll (l);
>
> Untested, but __builtin_popcount⦠has fallback emulation GCC, so this
> work. (And GCC 4.8 already has it.)
>
Thanks. It works. Here is the updated patch. Tested on i686, x86-64
and x32 with multi-arch disabled. OK for master?
Thanks.
H.J.
----
posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
which is incorrect for x32. This patch uses __builtin_popcount, which
is availabe in GCC 4.9, in posix/sched_cpucount.c.
Tested on i686, x86-64 and x32 with multi-arch disabled.
2017-07-01 Florian Weimer <fweimer@redhat.com>
H.J. Lu <hongjiu.lu@intel.com>
[BZ #21696]
* posix/sched_cpucount.c: Don't include <limits.h>.
(__sched_cpucount): Use __builtin_popcount.
---
posix/sched_cpucount.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/posix/sched_cpucount.c b/posix/sched_cpucount.c
index eaddf61..ab1ff49 100644
--- a/posix/sched_cpucount.c
+++ b/posix/sched_cpucount.c
@@ -15,7 +15,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <limits.h>
#include <sched.h>
@@ -36,22 +35,16 @@ __sched_cpucount (size_t setsize, const cpu_set_t *setp)
if (l == 0)
continue;
-# if LONG_BIT > 32
- l = (l & 0x5555555555555555ul) + ((l >> 1) & 0x5555555555555555ul);
- l = (l & 0x3333333333333333ul) + ((l >> 2) & 0x3333333333333333ul);
- l = (l & 0x0f0f0f0f0f0f0f0ful) + ((l >> 4) & 0x0f0f0f0f0f0f0f0ful);
- l = (l & 0x00ff00ff00ff00fful) + ((l >> 8) & 0x00ff00ff00ff00fful);
- l = (l & 0x0000ffff0000fffful) + ((l >> 16) & 0x0000ffff0000fffful);
- l = (l & 0x00000000fffffffful) + ((l >> 32) & 0x00000000fffffffful);
-# else
- l = (l & 0x55555555ul) + ((l >> 1) & 0x55555555ul);
- l = (l & 0x33333333ul) + ((l >> 2) & 0x33333333ul);
- l = (l & 0x0f0f0f0ful) + ((l >> 4) & 0x0f0f0f0ful);
- l = (l & 0x00ff00fful) + ((l >> 8) & 0x00ff00fful);
- l = (l & 0x0000fffful) + ((l >> 16) & 0x0000fffful);
-# endif
-
- s += l;
+ _Static_assert (sizeof (l) == sizeof (unsigned int)
+ || sizeof (l) == sizeof (unsigned long)
+ || sizeof (l) == sizeof (unsigned long long),
+ "sizeof (__cpu_mask");
+ if (sizeof (__cpu_mask) == sizeof (unsigned int))
+ s += __builtin_popcount (l);
+ else if (sizeof (__cpu_mask) == sizeof (unsigned long))
+ s += __builtin_popcountl (l);
+ else
+ s += __builtin_popcountll (l);
#endif
}
--
2.9.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Use __builtin_popcount in __sched_cpucount [BZ #21696]
2017-07-01 14:12 ` [PATCH] Use __builtin_popcount in __sched_cpucount " H.J. Lu
@ 2017-07-01 14:26 ` Florian Weimer
2017-07-01 14:31 ` H.J. Lu
0 siblings, 1 reply; 5+ messages in thread
From: Florian Weimer @ 2017-07-01 14:26 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 07/01/2017 04:12 PM, H.J. Lu wrote:
> posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
> which is incorrect for x32. This patch uses __builtin_popcount, which
> is availabe in GCC 4.9, in posix/sched_cpucount.c.
>
> Tested on i686, x86-64 and x32 with multi-arch disabled.
>
> 2017-07-01 Florian Weimer <fweimer@redhat.com>
> H.J. Lu <hongjiu.lu@intel.com>
>
> [BZ #21696]
> * posix/sched_cpucount.c: Don't include <limits.h>.
> (__sched_cpucount): Use __builtin_popcount.
Patch looks good to me. Thanks.
Florian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Use __builtin_popcount in __sched_cpucount [BZ #21696]
2017-07-01 14:26 ` Florian Weimer
@ 2017-07-01 14:31 ` H.J. Lu
0 siblings, 0 replies; 5+ messages in thread
From: H.J. Lu @ 2017-07-01 14:31 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Sat, Jul 1, 2017 at 7:26 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/01/2017 04:12 PM, H.J. Lu wrote:
>> posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
>> which is incorrect for x32. This patch uses __builtin_popcount, which
>> is availabe in GCC 4.9, in posix/sched_cpucount.c.
>>
>> Tested on i686, x86-64 and x32 with multi-arch disabled.
>>
>> 2017-07-01 Florian Weimer <fweimer@redhat.com>
>> H.J. Lu <hongjiu.lu@intel.com>
>>
>> [BZ #21696]
>> * posix/sched_cpucount.c: Don't include <limits.h>.
>> (__sched_cpucount): Use __builtin_popcount.
>
> Patch looks good to me. Thanks.
>
Checked it in.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-01 14:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-01 1:14 [PATCH] Properly check __cpu_mask size [BZ #21696] H.J. Lu
2017-07-01 6:35 ` Florian Weimer
2017-07-01 14:12 ` [PATCH] Use __builtin_popcount in __sched_cpucount " H.J. Lu
2017-07-01 14:26 ` Florian Weimer
2017-07-01 14:31 ` H.J. Lu
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).