* [PATCH 01/31] bitops: add parity functions
@ 2016-03-24 3:03 Zhaoxiu Zeng
2016-03-24 8:38 ` Denys Vlasenko
0 siblings, 1 reply; 15+ messages in thread
From: Zhaoxiu Zeng @ 2016-03-24 3:03 UTC (permalink / raw)
To: Arnd Bergmann, Andrew Morton, Martin Kepplinger, Sasha Levin,
Denys Vlasenko, Ingo Molnar, Yury Norov
Cc: linux-kernel, linux-arch, Zeng Zhaoxiu
From: Zeng Zhaoxiu <zhaoxiu.zeng@gmail.com>
When I do "grep parity -r linux", I found many parity calculations distributed in many drivers.
These patches provide generic and architecture-specific parity calculations.
Signed-off-by: Zeng Zhaoxiu <zhaoxiu.zeng@gmail.com>
---
include/asm-generic/bitops.h | 1 +
include/asm-generic/bitops/arch_parity.h | 39 +++++++++++++++++++++++++++++++
include/asm-generic/bitops/const_parity.h | 36 ++++++++++++++++++++++++++++
include/asm-generic/bitops/parity.h | 7 ++++++
include/linux/bitops.h | 5 ++++
5 files changed, 88 insertions(+)
create mode 100644 include/asm-generic/bitops/arch_parity.h
create mode 100644 include/asm-generic/bitops/const_parity.h
create mode 100644 include/asm-generic/bitops/parity.h
diff --git a/include/asm-generic/bitops.h b/include/asm-generic/bitops.h
index dcdcacf..d85722f 100644
--- a/include/asm-generic/bitops.h
+++ b/include/asm-generic/bitops.h
@@ -27,6 +27,7 @@
#include <asm-generic/bitops/sched.h>
#include <asm-generic/bitops/ffs.h>
#include <asm-generic/bitops/hweight.h>
+#include <asm-generic/bitops/parity.h>
#include <asm-generic/bitops/lock.h>
#include <asm-generic/bitops/atomic.h>
diff --git a/include/asm-generic/bitops/arch_parity.h b/include/asm-generic/bitops/arch_parity.h
new file mode 100644
index 0000000..8d51eb3
--- /dev/null
+++ b/include/asm-generic/bitops/arch_parity.h
@@ -0,0 +1,39 @@
+#ifndef _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
+#define _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
+
+#include <asm/types.h>
+
+/*
+ * Refrence to 'https://graphics.stanford.edu/~seander/bithacks.html#ParityParallel'.
+ */
+
+static inline unsigned int __arch_parity4(unsigned int w)
+{
+ w &= 0xf;
+ return (0x6996 >> w) & 1;
+}
+
+static inline unsigned int __arch_parity8(unsigned int w)
+{
+ w ^= w >> 4;
+ return __arch_parity4(w);
+}
+
+static inline unsigned int __arch_parity16(unsigned int w)
+{
+ w ^= w >> 8;
+ return __arch_parity8(w);
+}
+
+static inline unsigned int __arch_parity32(unsigned int w)
+{
+ w ^= w >> 16;
+ return __arch_parity16(w);
+}
+
+static inline unsigned int __arch_parity64(__u64 w)
+{
+ return __arch_parity32((unsigned int)(w >> 32) ^ (unsigned int)w);
+}
+
+#endif /* _ASM_GENERIC_BITOPS_ARCH_PARITY_H_ */
diff --git a/include/asm-generic/bitops/const_parity.h b/include/asm-generic/bitops/const_parity.h
new file mode 100644
index 0000000..9590315
--- /dev/null
+++ b/include/asm-generic/bitops/const_parity.h
@@ -0,0 +1,36 @@
+#ifndef _ASM_GENERIC_BITOPS_CONST_PARITY_H_
+#define _ASM_GENERIC_BITOPS_CONST_PARITY_H_
+
+/*
+ * Compile time versions of __arch_parityN()
+ */
+#define __const_parity4(w) ((0x6996 >> ((w) & 0xf)) & 1)
+#define __const_parity8(w) (__const_parity4(w ^ (w >> 4)))
+#define __const_parity16(w) (__const_parity8(w ^ (w >> 8)))
+#define __const_parity32(w) (__const_parity16(w ^ (w >> 16)))
+#define __const_parity64(w) (__const_parity32(w ^ (w >> 32)))
+
+/*
+ * Generic interface.
+ */
+#define parity4(w) (__builtin_constant_p(w) ? __const_parity4(w) : __arch_parity4(w))
+#define parity8(w) (__builtin_constant_p(w) ? __const_parity8(w) : __arch_parity8(w))
+#define parity16(w) (__builtin_constant_p(w) ? __const_parity16(w) : __arch_parity16(w))
+#define parity32(w) (__builtin_constant_p(w) ? __const_parity32(w) : __arch_parity32(w))
+#define parity64(w) (__builtin_constant_p(w) ? __const_parity64(w) : __arch_parity64(w))
+
+/*
+ * Interface for known constant arguments
+ */
+#define PARITY4(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity4(w))
+#define PARITY8(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity8(w))
+#define PARITY16(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity16(w))
+#define PARITY32(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity32(w))
+#define PARITY64(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity64(w))
+
+/*
+ * Type invariant interface to the compile time constant parity functions.
+ */
+#define PARITY(w) PARITY64((u64)w)
+
+#endif /* _ASM_GENERIC_BITOPS_CONST_PARITY_H_ */
diff --git a/include/asm-generic/bitops/parity.h b/include/asm-generic/bitops/parity.h
new file mode 100644
index 0000000..a91dce7
--- /dev/null
+++ b/include/asm-generic/bitops/parity.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_GENERIC_BITOPS_PARITY_H_
+#define _ASM_GENERIC_BITOPS_PARITY_H_
+
+#include <asm-generic/bitops/arch_parity.h>
+#include <asm-generic/bitops/const_parity.h>
+
+#endif /* _ASM_GENERIC_BITOPS_PARITY_H_ */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index defeaac..f91cb44 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -80,6 +80,11 @@ static __always_inline unsigned long hweight_long(unsigned long w)
return sizeof(w) == 4 ? hweight32(w) : hweight64(w);
}
+static __always_inline unsigned int parity_long(unsigned long w)
+{
+ return sizeof(w) == 4 ? parity32(w) : parity64(w);
+}
+
/**
* rol64 - rotate a 64-bit value left
* @word: value to rotate
--
2.5.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-24 3:03 [PATCH 01/31] bitops: add parity functions Zhaoxiu Zeng
@ 2016-03-24 8:38 ` Denys Vlasenko
2016-03-24 22:28 ` Andrew Morton
2016-03-27 3:33 ` zhaoxiu.zeng
0 siblings, 2 replies; 15+ messages in thread
From: Denys Vlasenko @ 2016-03-24 8:38 UTC (permalink / raw)
To: Zhaoxiu Zeng, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov
Cc: linux-kernel, linux-arch
On 03/24/2016 04:03 AM, Zhaoxiu Zeng wrote:
> +/*
> + * Type invariant interface to the compile time constant parity functions.
> + */
> +#define PARITY(w) PARITY64((u64)w)
Can result in incorrect expansion of w. Should be PARITY64((u64)(w))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-24 8:38 ` Denys Vlasenko
@ 2016-03-24 22:28 ` Andrew Morton
2016-03-26 22:08 ` Martin Kepplinger
2016-03-27 3:33 ` zhaoxiu.zeng
1 sibling, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2016-03-24 22:28 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Zhaoxiu Zeng, Arnd Bergmann, Martin Kepplinger, Sasha Levin,
Ingo Molnar, Yury Norov, linux-kernel, linux-arch
On Thu, 24 Mar 2016 09:38:21 +0100 Denys Vlasenko <dvlasenk@redhat.com> wrote:
> On 03/24/2016 04:03 AM, Zhaoxiu Zeng wrote:
> > +/*
> > + * Type invariant interface to the compile time constant parity functions.
> > + */
> > +#define PARITY(w) PARITY64((u64)w)
>
> Can result in incorrect expansion of w. Should be PARITY64((u64)(w))
And we seem to be missing the other 30 patches.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-24 22:28 ` Andrew Morton
@ 2016-03-26 22:08 ` Martin Kepplinger
2016-03-27 7:51 ` zhaoxiu.zeng
0 siblings, 1 reply; 15+ messages in thread
From: Martin Kepplinger @ 2016-03-26 22:08 UTC (permalink / raw)
To: Andrew Morton, Denys Vlasenko
Cc: Zhaoxiu Zeng, Arnd Bergmann, Sasha Levin, Ingo Molnar,
Yury Norov, linux-kernel, linux-arch
We do.
Am 24. März 2016 23:28:15 MEZ, schrieb Andrew Morton <akpm@linux-foundation.org>:
>On Thu, 24 Mar 2016 09:38:21 +0100 Denys Vlasenko <dvlasenk@redhat.com>
>wrote:
>
>> On 03/24/2016 04:03 AM, Zhaoxiu Zeng wrote:
>> > +/*
>> > + * Type invariant interface to the compile time constant parity
>functions.
>> > + */
>> > +#define PARITY(w) PARITY64((u64)w)
>>
>> Can result in incorrect expansion of w. Should be PARITY64((u64)(w))
>
>And we seem to be missing the other 30 patches.
--
Martin Kepplinger
http://martinkepplinger.com
sent from mobile
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-26 22:08 ` Martin Kepplinger
@ 2016-03-27 7:51 ` zhaoxiu.zeng
0 siblings, 0 replies; 15+ messages in thread
From: zhaoxiu.zeng @ 2016-03-27 7:51 UTC (permalink / raw)
To: Martin Kepplinger, Andrew Morton, Denys Vlasenko
Cc: Arnd Bergmann, Sasha Levin, Ingo Molnar, Yury Norov,
linux-kernel, linux-arch
On 2016/3/27 6:08, Martin Kepplinger wrote:
> We do.
>
> Am 24. März 2016 23:28:15 MEZ, schrieb Andrew Morton <akpm@linux-foundation.org>:
>> On Thu, 24 Mar 2016 09:38:21 +0100 Denys Vlasenko <dvlasenk@redhat.com>
>> wrote:
>>
>>> On 03/24/2016 04:03 AM, Zhaoxiu Zeng wrote:
>>>> +/*
>>>> + * Type invariant interface to the compile time constant parity
>> functions.
>>>> + */
>>>> +#define PARITY(w) PARITY64((u64)w)
>>>
>>> Can result in incorrect expansion of w. Should be PARITY64((u64)(w))
>>
>> And we seem to be missing the other 30 patches.
>
Sorry,I got some problems with my gmail.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-24 8:38 ` Denys Vlasenko
2016-03-24 22:28 ` Andrew Morton
@ 2016-03-27 3:33 ` zhaoxiu.zeng
2016-03-27 12:44 ` Sam Ravnborg
2016-03-28 6:51 ` Sam Ravnborg
1 sibling, 2 replies; 15+ messages in thread
From: zhaoxiu.zeng @ 2016-03-27 3:33 UTC (permalink / raw)
To: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov
Cc: linux-kernel, linux-arch
On 2016/3/24 16:38, Denys Vlasenko wrote:
> On 03/24/2016 04:03 AM, Zhaoxiu Zeng wrote:
>> +/*
>> + * Type invariant interface to the compile time constant parity functions.
>> + */
>> +#define PARITY(w) PARITY64((u64)w)
>
> Can result in incorrect expansion of w. Should be PARITY64((u64)(w))
>
> .
>
Thanks. The new version has been fixed.
Signed-off-by: Zeng Zhaoxiu <zhaoxiu.zeng@gmail.com>
---
include/asm-generic/bitops.h | 1 +
include/asm-generic/bitops/arch_parity.h | 39 +++++++++++++++++++++++++++++++
include/asm-generic/bitops/const_parity.h | 36 ++++++++++++++++++++++++++++
include/asm-generic/bitops/parity.h | 7 ++++++
include/linux/bitops.h | 5 ++++
5 files changed, 88 insertions(+)
create mode 100644 include/asm-generic/bitops/arch_parity.h
create mode 100644 include/asm-generic/bitops/const_parity.h
create mode 100644 include/asm-generic/bitops/parity.h
diff --git a/include/asm-generic/bitops.h b/include/asm-generic/bitops.h
index dcdcacf..d85722f 100644
--- a/include/asm-generic/bitops.h
+++ b/include/asm-generic/bitops.h
@@ -27,6 +27,7 @@
#include <asm-generic/bitops/sched.h>
#include <asm-generic/bitops/ffs.h>
#include <asm-generic/bitops/hweight.h>
+#include <asm-generic/bitops/parity.h>
#include <asm-generic/bitops/lock.h>
#include <asm-generic/bitops/atomic.h>
diff --git a/include/asm-generic/bitops/arch_parity.h b/include/asm-generic/bitops/arch_parity.h
new file mode 100644
index 0000000..cddc555
--- /dev/null
+++ b/include/asm-generic/bitops/arch_parity.h
@@ -0,0 +1,39 @@
+#ifndef _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
+#define _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
+
+#include <asm/types.h>
+
+/*
+ * Refrence to 'https://graphics.stanford.edu/~seander/bithacks.html#ParityParallel'.
+ */
+
+static inline unsigned int __arch_parity4(unsigned int w)
+{
+ w &= 0xf;
+ return (0x6996 >> w) & 1;
+}
+
+static inline unsigned int __arch_parity8(unsigned int w)
+{
+ w ^= w >> 4;
+ return __arch_parity4(w);
+}
+
+static inline unsigned int __arch_parity16(unsigned int w)
+{
+ w ^= w >> 8;
+ return __arch_parity8(w);
+}
+
+static inline unsigned int __arch_parity32(unsigned int w)
+{
+ w ^= w >> 16;
+ return __arch_parity16(w);
+}
+
+static inline unsigned int __arch_parity64(__u64 w)
+{
+ return __arch_parity32((unsigned int)(w >> 32) ^ (unsigned int)w);
+}
+
+#endif /* _ASM_GENERIC_BITOPS_ARCH_PARITY_H_ */
diff --git a/include/asm-generic/bitops/const_parity.h b/include/asm-generic/bitops/const_parity.h
new file mode 100644
index 0000000..6af7987
--- /dev/null
+++ b/include/asm-generic/bitops/const_parity.h
@@ -0,0 +1,36 @@
+#ifndef _ASM_GENERIC_BITOPS_CONST_PARITY_H_
+#define _ASM_GENERIC_BITOPS_CONST_PARITY_H_
+
+/*
+ * Compile time versions of __arch_parityN()
+ */
+#define __const_parity4(w) ((0x6996 >> ((w) & 0xf)) & 1)
+#define __const_parity8(w) (__const_parity4((w) ^ ((w) >> 4)))
+#define __const_parity16(w) (__const_parity8((w) ^ ((w) >> 8)))
+#define __const_parity32(w) (__const_parity16((w) ^ ((w) >> 16)))
+#define __const_parity64(w) (__const_parity32((w) ^ ((w) >> 32)))
+
+/*
+ * Generic interface.
+ */
+#define parity4(w) (__builtin_constant_p(w) ? __const_parity4(w) : __arch_parity4(w))
+#define parity8(w) (__builtin_constant_p(w) ? __const_parity8(w) : __arch_parity8(w))
+#define parity16(w) (__builtin_constant_p(w) ? __const_parity16(w) : __arch_parity16(w))
+#define parity32(w) (__builtin_constant_p(w) ? __const_parity32(w) : __arch_parity32(w))
+#define parity64(w) (__builtin_constant_p(w) ? __const_parity64(w) : __arch_parity64(w))
+
+/*
+ * Interface for known constant arguments
+ */
+#define PARITY4(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity4(w))
+#define PARITY8(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity8(w))
+#define PARITY16(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity16(w))
+#define PARITY32(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity32(w))
+#define PARITY64(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_parity64(w))
+
+/*
+ * Type invariant interface to the compile time constant parity functions.
+ */
+#define PARITY(w) PARITY64((u64)(w))
+
+#endif /* _ASM_GENERIC_BITOPS_CONST_PARITY_H_ */
diff --git a/include/asm-generic/bitops/parity.h b/include/asm-generic/bitops/parity.h
new file mode 100644
index 0000000..a91dce7
--- /dev/null
+++ b/include/asm-generic/bitops/parity.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_GENERIC_BITOPS_PARITY_H_
+#define _ASM_GENERIC_BITOPS_PARITY_H_
+
+#include <asm-generic/bitops/arch_parity.h>
+#include <asm-generic/bitops/const_parity.h>
+
+#endif /* _ASM_GENERIC_BITOPS_PARITY_H_ */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index defeaac..8952f88 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -80,6 +80,11 @@ static __always_inline unsigned long hweight_long(unsigned long w)
return sizeof(w) == 4 ? hweight32(w) : hweight64(w);
}
+static __always_inline unsigned int parity_long(unsigned long w)
+{
+ return sizeof(w) == 4 ? parity32(w) : parity64(w);
+}
+
/**
* rol64 - rotate a 64-bit value left
* @word: value to rotate
--
2.5.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-27 3:33 ` zhaoxiu.zeng
@ 2016-03-27 12:44 ` Sam Ravnborg
2016-03-27 13:38 ` zhaoxiu.zeng
2016-03-28 6:51 ` Sam Ravnborg
1 sibling, 1 reply; 15+ messages in thread
From: Sam Ravnborg @ 2016-03-27 12:44 UTC (permalink / raw)
To: zhaoxiu.zeng
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch
Hi Zeng.
Looking through the arch specific implementations of __arch_parity().
Some architectures uses #defines, other uses inline static functions.
Any particular reason that you select one approach over the other
in the different cases?
ia64:
+#define __arch_parity32(x) ((unsigned int) __arch_parity64((x) & 0xfffffffful))
+#define __arch_parity16(x) ((unsigned int) __arch_parity64((x) & 0xfffful))
+#define __arch_parity8(x) ((unsigned int) __arch_parity64((x) & 0xfful))
+#define __arch_parity4(x) ((unsigned int) __arch_parity64((x) & 0xful))
tile:
+static inline unsigned int __arch_parity32(unsigned int w)
+{
+ return __builtin_popcount(w) & 1;
+}
+
+static inline unsigned int __arch_parity16(unsigned int w)
+{
+ return __arch_parity32(w & 0xffff);
+}
+
+static inline unsigned int __arch_parity8(unsigned int w)
+{
+ return __arch_parity32(w & 0xff);
+}
+
+static inline unsigned int __arch_parity4(unsigned int w)
+{
+ return __arch_parity32(w & 0xf);
+}
Just two examples.
Adding the parity helpers seems like veny nice simplifications.
A few comments to some of those I looked at.
(I am not subscribed to lkml, so you get it as comments here)
[PATCH 21/31] mtd: use parity16 in ssfdc.c
The original code semes to check that the parity equals the
value of first bit in the address.
This seems lost after the conversion.
[PATCH 20/31] scsi: use parity32 in isci/phy.c
+ if (parity32(phy_cap.all))
phy_cap.parity = 1;
Could be written like this - simpler IMO:
phy_cap.parity = parity32(phy_cap.all);
Sam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-27 12:44 ` Sam Ravnborg
@ 2016-03-27 13:38 ` zhaoxiu.zeng
2016-03-27 17:56 ` Sam Ravnborg
2016-03-28 2:15 ` Zeng Zhaoxiu
0 siblings, 2 replies; 15+ messages in thread
From: zhaoxiu.zeng @ 2016-03-27 13:38 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch
On 2016/3/27 20:44, Sam Ravnborg wrote:
> Hi Zeng.
>
> Looking through the arch specific implementations of __arch_parity().
> Some architectures uses #defines, other uses inline static functions.
>
> Any particular reason that you select one approach over the other
> in the different cases?
>
> ia64:
> +#define __arch_parity32(x) ((unsigned int) __arch_parity64((x) & 0xfffffffful))
> +#define __arch_parity16(x) ((unsigned int) __arch_parity64((x) & 0xfffful))
> +#define __arch_parity8(x) ((unsigned int) __arch_parity64((x) & 0xfful))
> +#define __arch_parity4(x) ((unsigned int) __arch_parity64((x) & 0xful))
>
> tile:
> +static inline unsigned int __arch_parity32(unsigned int w)
> +{
> + return __builtin_popcount(w) & 1;
> +}
> +
> +static inline unsigned int __arch_parity16(unsigned int w)
> +{
> + return __arch_parity32(w & 0xffff);
> +}
> +
> +static inline unsigned int __arch_parity8(unsigned int w)
> +{
> + return __arch_parity32(w & 0xff);
> +}
> +
> +static inline unsigned int __arch_parity4(unsigned int w)
> +{
> + return __arch_parity32(w & 0xf);
> +}
>
No particular reason, just like the architecture's __arch_hweightN.
> Just two examples.
>
> Adding the parity helpers seems like veny nice simplifications.
>
> A few comments to some of those I looked at.
> (I am not subscribed to lkml, so you get it as comments here)
>
I think the conversion is simple and readable.
> [PATCH 21/31] mtd: use parity16 in ssfdc.c
> The original code semes to check that the parity equals the
> value of first bit in the address.
> This seems lost after the conversion.
>
The original get_parity return 1 if the number is even, so
if block_address is valid, "block_address & 0x7ff" must be odd.
> [PATCH 20/31] scsi: use parity32 in isci/phy.c
> + if (parity32(phy_cap.all))
> phy_cap.parity = 1;
> Could be written like this - simpler IMO:
> phy_cap.parity = parity32(phy_cap.all);
>
>
> Sam
>
Yes. Thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-27 13:38 ` zhaoxiu.zeng
@ 2016-03-27 17:56 ` Sam Ravnborg
2016-03-28 2:44 ` Zeng Zhaoxiu
2016-03-28 2:15 ` Zeng Zhaoxiu
1 sibling, 1 reply; 15+ messages in thread
From: Sam Ravnborg @ 2016-03-27 17:56 UTC (permalink / raw)
To: zhaoxiu.zeng
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch
> > Any particular reason that you select one approach over the other
> > in the different cases?
>
> No particular reason, just like the architecture's __arch_hweightN.
The general recommendatiosn these days are to use static inline
for code to get better type check.
And it would also be nice to be consistent across architectures.
Sam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-27 17:56 ` Sam Ravnborg
@ 2016-03-28 2:44 ` Zeng Zhaoxiu
0 siblings, 0 replies; 15+ messages in thread
From: Zeng Zhaoxiu @ 2016-03-28 2:44 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch
OK, I will do the V2 patches soon.
In addition, the best is to provide asm version parity functions for
powerpc, sparc, and x86.
在 2016年03月28日 01:56, Sam Ravnborg 写道:
>>> Any particular reason that you select one approach over the other
>>> in the different cases?
>> No particular reason, just like the architecture's __arch_hweightN.
> The general recommendatiosn these days are to use static inline
> for code to get better type check.
> And it would also be nice to be consistent across architectures.
>
> Sam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-27 13:38 ` zhaoxiu.zeng
2016-03-27 17:56 ` Sam Ravnborg
@ 2016-03-28 2:15 ` Zeng Zhaoxiu
1 sibling, 0 replies; 15+ messages in thread
From: Zeng Zhaoxiu @ 2016-03-28 2:15 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch
On 2016年03月27日 21:38, zhaoxiu.zeng wrote:
> On 2016/3/27 20:44, Sam Ravnborg wrote:
>> Hi Zeng.
>>
>> Looking through the arch specific implementations of __arch_parity().
>> Some architectures uses #defines, other uses inline static functions.
>>
>> Any particular reason that you select one approach over the other
>> in the different cases?
>>
>> ia64:
>> +#define __arch_parity32(x) ((unsigned int) __arch_parity64((x) & 0xfffffffful))
>> +#define __arch_parity16(x) ((unsigned int) __arch_parity64((x) & 0xfffful))
>> +#define __arch_parity8(x) ((unsigned int) __arch_parity64((x) & 0xfful))
>> +#define __arch_parity4(x) ((unsigned int) __arch_parity64((x) & 0xful))
>>
>> tile:
>> +static inline unsigned int __arch_parity32(unsigned int w)
>> +{
>> + return __builtin_popcount(w) & 1;
>> +}
>> +
>> +static inline unsigned int __arch_parity16(unsigned int w)
>> +{
>> + return __arch_parity32(w & 0xffff);
>> +}
>> +
>> +static inline unsigned int __arch_parity8(unsigned int w)
>> +{
>> + return __arch_parity32(w & 0xff);
>> +}
>> +
>> +static inline unsigned int __arch_parity4(unsigned int w)
>> +{
>> + return __arch_parity32(w & 0xf);
>> +}
>>
> No particular reason, just like the architecture's __arch_hweightN.
>
>> Just two examples.
>>
>> Adding the parity helpers seems like veny nice simplifications.
>>
>> A few comments to some of those I looked at.
>> (I am not subscribed to lkml, so you get it as comments here)
>>
> I think the conversion is simple and readable.
>
>> [PATCH 21/31] mtd: use parity16 in ssfdc.c
>> The original code semes to check that the parity equals the
>> value of first bit in the address.
>> This seems lost after the conversion.
>>
> The original get_parity return 1 if the number is even, so
> if block_address is valid, "block_address & 0x7ff" must be odd.
Make corrections:
The original get_parity return 1 if hweight of the input number is even, so
if block_address is valid, hweight of "block_address & 0x7ff" must be odd.
>
>> [PATCH 20/31] scsi: use parity32 in isci/phy.c
>> + if (parity32(phy_cap.all))
>> phy_cap.parity = 1;
>> Could be written like this - simpler IMO:
>> phy_cap.parity = parity32(phy_cap.all);
>>
>>
>> Sam
>>
> Yes. Thanks!
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-27 3:33 ` zhaoxiu.zeng
2016-03-27 12:44 ` Sam Ravnborg
@ 2016-03-28 6:51 ` Sam Ravnborg
2016-03-29 2:27 ` Zeng Zhaoxiu
1 sibling, 1 reply; 15+ messages in thread
From: Sam Ravnborg @ 2016-03-28 6:51 UTC (permalink / raw)
To: zhaoxiu.zeng
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch,
David S. Miller
> diff --git a/include/asm-generic/bitops/arch_parity.h b/include/asm-generic/bitops/arch_parity.h
> new file mode 100644
> index 0000000..cddc555
> --- /dev/null
> +++ b/include/asm-generic/bitops/arch_parity.h
> @@ -0,0 +1,39 @@
> +#ifndef _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
> +#define _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
> +
> +#include <asm/types.h>
> +
> +/*
> + * Refrence to 'https://graphics.stanford.edu/~seander/bithacks.html#ParityParallel'.
> + */
> +
> +static inline unsigned int __arch_parity4(unsigned int w)
> +{
> + w &= 0xf;
> + return (0x6996 >> w) & 1;
> +}
> +
> +static inline unsigned int __arch_parity8(unsigned int w)
> +{
> + w ^= w >> 4;
> + return __arch_parity4(w);
> +}
> +
> +static inline unsigned int __arch_parity16(unsigned int w)
> +{
> + w ^= w >> 8;
> + return __arch_parity8(w);
> +}
> +
> +static inline unsigned int __arch_parity32(unsigned int w)
> +{
> + w ^= w >> 16;
> + return __arch_parity16(w);
> +}
> +
> +static inline unsigned int __arch_parity64(__u64 w)
> +{
> + return __arch_parity32((unsigned int)(w >> 32) ^ (unsigned int)w);
> +}
Defining these as static inlines in asm-generic prevent an architecture
from selecting between a more optimal asm version or the generic version
at run-time.
sparc would benefit from this as only some sparc chips supports popc.
See how this is done for hweight*
Sam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-28 6:51 ` Sam Ravnborg
@ 2016-03-29 2:27 ` Zeng Zhaoxiu
2016-03-29 2:56 ` Joe Perches
0 siblings, 1 reply; 15+ messages in thread
From: Zeng Zhaoxiu @ 2016-03-29 2:27 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch,
David S. Miller
在 2016年03月28日 14:51, Sam Ravnborg 写道:
>> diff --git a/include/asm-generic/bitops/arch_parity.h b/include/asm-generic/bitops/arch_parity.h
>> new file mode 100644
>> index 0000000..cddc555
>> --- /dev/null
>> +++ b/include/asm-generic/bitops/arch_parity.h
>> @@ -0,0 +1,39 @@
>> +#ifndef _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
>> +#define _ASM_GENERIC_BITOPS_ARCH_PARITY_H_
>> +
>> +#include <asm/types.h>
>> +
>> +/*
>> + * Refrence to 'https://graphics.stanford.edu/~seander/bithacks.html#ParityParallel'.
>> + */
>> +
>> +static inline unsigned int __arch_parity4(unsigned int w)
>> +{
>> + w &= 0xf;
>> + return (0x6996 >> w) & 1;
>> +}
>> +
>> +static inline unsigned int __arch_parity8(unsigned int w)
>> +{
>> + w ^= w >> 4;
>> + return __arch_parity4(w);
>> +}
>> +
>> +static inline unsigned int __arch_parity16(unsigned int w)
>> +{
>> + w ^= w >> 8;
>> + return __arch_parity8(w);
>> +}
>> +
>> +static inline unsigned int __arch_parity32(unsigned int w)
>> +{
>> + w ^= w >> 16;
>> + return __arch_parity16(w);
>> +}
>> +
>> +static inline unsigned int __arch_parity64(__u64 w)
>> +{
>> + return __arch_parity32((unsigned int)(w >> 32) ^ (unsigned int)w);
>> +}
> Defining these as static inlines in asm-generic prevent an architecture
> from selecting between a more optimal asm version or the generic version
> at run-time.
> sparc would benefit from this as only some sparc chips supports popc.
> See how this is done for hweight*
>
> Sam
Thanks. I will try.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-29 2:27 ` Zeng Zhaoxiu
@ 2016-03-29 2:56 ` Joe Perches
2016-03-29 2:56 ` Joe Perches
0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2016-03-29 2:56 UTC (permalink / raw)
To: Zeng Zhaoxiu, Sam Ravnborg
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch,
David S. Miller
On Tue, 2016-03-29 at 10:27 +0800, Zeng Zhaoxiu wrote:
> 在 2016年03月28日 14:51, Sam Ravnborg 写道:
[]
> > Defining these as static inlines in asm-generic prevent an
> > architecture
> > from selecting between a more optimal asm version or the generic version
> > at run-time.
> > sparc would benefit from this as only some sparc chips supports popc.
> > See how this is done for hweight*
> >
> > Sam
> Thanks. I will try.
You might also try to describe in any commit message
and perhaps the internal documentation why using gcc's
__builtin_parity isn't appropriate.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 01/31] bitops: add parity functions
2016-03-29 2:56 ` Joe Perches
@ 2016-03-29 2:56 ` Joe Perches
0 siblings, 0 replies; 15+ messages in thread
From: Joe Perches @ 2016-03-29 2:56 UTC (permalink / raw)
To: Zeng Zhaoxiu, Sam Ravnborg
Cc: Denys Vlasenko, Arnd Bergmann, Andrew Morton, Martin Kepplinger,
Sasha Levin, Ingo Molnar, Yury Norov, linux-kernel, linux-arch,
David S. Miller
On Tue, 2016-03-29 at 10:27 +0800, Zeng Zhaoxiu wrote:
> 在 2016年03月28日 14:51, Sam Ravnborg 写道:
[]
> > Defining these as static inlines in asm-generic prevent an
> > architecture
> > from selecting between a more optimal asm version or the generic version
> > at run-time.
> > sparc would benefit from this as only some sparc chips supports popc.
> > See how this is done for hweight*
> >
> > Sam
> Thanks. I will try.
You might also try to describe in any commit message
and perhaps the internal documentation why using gcc's
__builtin_parity isn't appropriate.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-03-29 3:01 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-24 3:03 [PATCH 01/31] bitops: add parity functions Zhaoxiu Zeng
2016-03-24 8:38 ` Denys Vlasenko
2016-03-24 22:28 ` Andrew Morton
2016-03-26 22:08 ` Martin Kepplinger
2016-03-27 7:51 ` zhaoxiu.zeng
2016-03-27 3:33 ` zhaoxiu.zeng
2016-03-27 12:44 ` Sam Ravnborg
2016-03-27 13:38 ` zhaoxiu.zeng
2016-03-27 17:56 ` Sam Ravnborg
2016-03-28 2:44 ` Zeng Zhaoxiu
2016-03-28 2:15 ` Zeng Zhaoxiu
2016-03-28 6:51 ` Sam Ravnborg
2016-03-29 2:27 ` Zeng Zhaoxiu
2016-03-29 2:56 ` Joe Perches
2016-03-29 2:56 ` Joe Perches
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 NNTP newsgroup(s).