[PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

Several parts of the kernel contain redundant implementations of parity
calculations for 16/32/64-bit values. Introduces generic
parity16/32/64() helpers in bitops.h, providing a standardized
and optimized implementation. 

Subsequent patches refactor various kernel components to replace
open-coded parity calculations with the new helpers, reducing code
duplication and improving maintainability.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
In v3, I use parityXX() instead of the parity() macro since the
parity() macro may generate suboptimal code and requires special hacks
to make GCC happy. If anyone still prefers a single parity() macro,
please let me know.

Additionally, I changed parityXX() << y users to !!parityXX() << y
because, unlike C++, C does not guarantee that true casts to int as 1.

Changes in v3:
- Avoid using __builtin_parity.
- Change return type to bool.
- Drop parity() macro.
- Change parityXX() << y to !!parityXX() << y.


Changes in v2:
- Provide fallback functions for __builtin_parity() when the compiler
  decides not to inline it
- Use __builtin_parity() when no architecture-specific implementation
  is available
- Optimize for constant folding when val is a compile-time constant
- Add a generic parity() macro
- Drop the x86 bootflag conversion patch since it has been merged into
  the tip tree

v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitorckw@gmail.com/
v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitorckw@gmail.com/

Kuan-Wei Chiu (16):
  bitops: Change parity8() return type to bool
  bitops: Add parity16(), parity32(), and parity64() helpers
  media: media/test_drivers: Replace open-coded parity calculation with
    parity8()
  media: pci: cx18-av-vbi: Replace open-coded parity calculation with
    parity8()
  media: saa7115: Replace open-coded parity calculation with parity8()
  serial: max3100: Replace open-coded parity calculation with parity8()
  lib/bch: Replace open-coded parity calculation with parity32()
  Input: joystick - Replace open-coded parity calculation with
    parity32()
  net: ethernet: oa_tc6: Replace open-coded parity calculation with
    parity32()
  wifi: brcm80211: Replace open-coded parity calculation with parity32()
  drm/bridge: dw-hdmi: Replace open-coded parity calculation with
    parity32()
  mtd: ssfdc: Replace open-coded parity calculation with parity32()
  fsi: i2cr: Replace open-coded parity calculation with parity32()
  fsi: i2cr: Replace open-coded parity calculation with parity64()
  Input: joystick - Replace open-coded parity calculation with
    parity64()
  nfp: bpf: Replace open-coded parity calculation with parity64()

 drivers/fsi/fsi-master-i2cr.c                 | 18 ++-----
 .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  8 +--
 drivers/input/joystick/grip_mp.c              | 17 +-----
 drivers/input/joystick/sidewinder.c           | 24 ++-------
 drivers/media/i2c/saa7115.c                   | 12 +----
 drivers/media/pci/cx18/cx18-av-vbi.c          | 12 +----
 .../media/test-drivers/vivid/vivid-vbi-gen.c  |  8 +--
 drivers/mtd/ssfdc.c                           | 20 ++-----
 drivers/net/ethernet/netronome/nfp/nfp_asm.c  |  7 +--
 drivers/net/ethernet/oa_tc6.c                 | 19 ++-----
 .../broadcom/brcm80211/brcmsmac/dma.c         | 16 +-----
 drivers/tty/serial/max3100.c                  |  3 +-
 include/linux/bitops.h                        | 52 +++++++++++++++++--
 lib/bch.c                                     | 14 +----
 14 files changed, 77 insertions(+), 153 deletions(-)

-- 
2.34.1

[PATCH v3 01/16] bitops: Change parity8() return type to bool

[Kuan-Wei Chiu]

Change return type to bool for better clarity. Update the kernel doc
comment accordingly, including fixing "@value" to "@val" and adjusting
examples. Also mark the function with __attribute_const__ to allow
potential compiler optimizations.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 include/linux/bitops.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index c1cb53cf2f0f..44e5765b8bec 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
 
 /**
  * parity8 - get the parity of an u8 value
- * @value: the value to be examined
+ * @val: the value to be examined
  *
  * Determine the parity of the u8 argument.
  *
  * Returns:
- * 0 for even parity, 1 for odd parity
+ * false for even parity, true for odd parity
  *
  * Note: This function informs you about the current parity. Example to bail
  * out when parity is odd:
  *
- *	if (parity8(val) == 1)
+ *	if (parity8(val) == true)
  *		return -EBADMSG;
  *
  * If you need to calculate a parity bit, you need to draw the conclusion from
  * this result yourself. Example to enforce odd parity, parity bit is bit 7:
  *
- *	if (parity8(val) == 0)
+ *	if (parity8(val) == false)
  *		val ^= BIT(7);
  */
-static inline int parity8(u8 val)
+static inline __attribute_const__ bool parity8(u8 val)
 {
 	/*
 	 * One explanation of this algorithm:
-- 
2.34.1

[PATCH v3 02/16] bitops: Add parity16(), parity32(), and parity64() helpers

[Kuan-Wei Chiu]

Add parity16(), parity32(), and parity64() to compute the parity of
16-bit, 32-bit, and 64-bit values, respectively. Each function extends
parity8() by XOR-ing upper and lower halves, reducing the input size
progressively.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 include/linux/bitops.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 44e5765b8bec..906757e1ddf8 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -260,6 +260,48 @@ static inline __attribute_const__ bool parity8(u8 val)
 	return (0x6996 >> (val & 0xf)) & 1;
 }
 
+/**
+ * parity16 - get the parity of an u16 value
+ * @val: the value to be examined
+ *
+ * Determine the parity of the u16 argument.
+ *
+ * Returns:
+ * false for even parity, true for odd parity
+ */
+static inline __attribute_const__ bool parity16(u16 val)
+{
+	return parity8(val ^ (val >> 8));
+}
+
+/**
+ * parity32 - get the parity of an u32 value
+ * @val: the value to be examined
+ *
+ * Determine the parity of the u32 argument.
+ *
+ * Returns:
+ * false for even parity, true for odd parity
+ */
+static inline __attribute_const__ bool parity32(u32 val)
+{
+	return parity16(val ^ (val >> 16));
+}
+
+/**
+ * parity64 - get the parity of an u64 value
+ * @val: the value to be examined
+ *
+ * Determine the parity of the u64 argument.
+ *
+ * Returns:
+ * false for even parity, true for odd parity
+ */
+static inline __attribute_const__ bool parity64(u64 val)
+{
+	return parity32(val ^ (val >> 32));
+}
+
 /**
  * __ffs64 - find first set bit in a 64 bit word
  * @word: The 64 bit word
-- 
2.34.1

[PATCH v3 03/16] media: media/test_drivers: Replace open-coded parity calculation with parity8()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity8() helper. This
change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 drivers/media/test-drivers/vivid/vivid-vbi-gen.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-gen.c b/drivers/media/test-drivers/vivid/vivid-vbi-gen.c
index 70a4024d461e..e0f4151bda18 100644
--- a/drivers/media/test-drivers/vivid/vivid-vbi-gen.c
+++ b/drivers/media/test-drivers/vivid/vivid-vbi-gen.c
@@ -5,6 +5,7 @@
  * Copyright 2014 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
  */
 
+#include <linux/bitops.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/ktime.h>
@@ -165,12 +166,7 @@ static const u8 vivid_cc_sequence2[30] = {
 
 static u8 calc_parity(u8 val)
 {
-	unsigned i;
-	unsigned tot = 0;
-
-	for (i = 0; i < 7; i++)
-		tot += (val & (1 << i)) ? 1 : 0;
-	return val | ((tot & 1) ? 0 : 0x80);
+	return val | (parity8(val) ? 0 : 0x80);
 }
 
 static void vivid_vbi_gen_set_time_of_day(u8 *packet)
-- 
2.34.1

[PATCH v3 04/16] media: pci: cx18-av-vbi: Replace open-coded parity calculation with parity8()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity8() helper. This
change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 drivers/media/pci/cx18/cx18-av-vbi.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/cx18/cx18-av-vbi.c b/drivers/media/pci/cx18/cx18-av-vbi.c
index 65281d40c681..1a113aad9cd4 100644
--- a/drivers/media/pci/cx18/cx18-av-vbi.c
+++ b/drivers/media/pci/cx18/cx18-av-vbi.c
@@ -8,6 +8,7 @@
  */
 
 
+#include <linux/bitops.h>
 #include "cx18-driver.h"
 
 /*
@@ -56,15 +57,6 @@ struct vbi_anc_data {
 	/* u8 fill[]; Variable number of fill bytes */
 };
 
-static int odd_parity(u8 c)
-{
-	c ^= (c >> 4);
-	c ^= (c >> 2);
-	c ^= (c >> 1);
-
-	return c & 1;
-}
-
 static int decode_vps(u8 *dst, u8 *p)
 {
 	static const u8 biphase_tbl[] = {
@@ -278,7 +270,7 @@ int cx18_av_decode_vbi_line(struct v4l2_subdev *sd,
 		break;
 	case 6:
 		sdid = V4L2_SLICED_CAPTION_525;
-		err = !odd_parity(p[0]) || !odd_parity(p[1]);
+		err = !parity8(p[0]) || !parity8(p[1]);
 		break;
 	case 9:
 		sdid = V4L2_SLICED_VPS;
-- 
2.34.1

[PATCH v3 05/16] media: saa7115: Replace open-coded parity calculation with parity8()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity8() helper. This
change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 drivers/media/i2c/saa7115.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
index a1c71187e773..b8b8f206ec3a 100644
--- a/drivers/media/i2c/saa7115.c
+++ b/drivers/media/i2c/saa7115.c
@@ -25,6 +25,7 @@
 
 #include "saa711x_regs.h"
 
+#include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -664,15 +665,6 @@ static const unsigned char saa7115_init_misc[] = {
 	0x00, 0x00
 };
 
-static int saa711x_odd_parity(u8 c)
-{
-	c ^= (c >> 4);
-	c ^= (c >> 2);
-	c ^= (c >> 1);
-
-	return c & 1;
-}
-
 static int saa711x_decode_vps(u8 *dst, u8 *p)
 {
 	static const u8 biphase_tbl[] = {
@@ -1227,7 +1219,7 @@ static int saa711x_decode_vbi_line(struct v4l2_subdev *sd, struct v4l2_decode_vb
 		vbi->type = V4L2_SLICED_TELETEXT_B;
 		break;
 	case 4:
-		if (!saa711x_odd_parity(p[0]) || !saa711x_odd_parity(p[1]))
+		if (!parity8(p[0]) || !parity8(p[1]))
 			return 0;
 		vbi->type = V4L2_SLICED_CAPTION_525;
 		break;
-- 
2.34.1

[PATCH v3 06/16] serial: max3100: Replace open-coded parity calculation with parity8()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity8() helper. This
change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change parity8(c) to !!parity8(c).

 drivers/tty/serial/max3100.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/max3100.c b/drivers/tty/serial/max3100.c
index cde5f1c86353..419d74043498 100644
--- a/drivers/tty/serial/max3100.c
+++ b/drivers/tty/serial/max3100.c
@@ -16,6 +16,7 @@
 /* 4 MAX3100s should be enough for everyone */
 #define MAX_MAX3100 4
 
+#include <linux/bitops.h>
 #include <linux/container_of.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -133,7 +134,7 @@ static int max3100_do_parity(struct max3100_port *s, u16 c)
 	else
 		c &= 0xff;
 
-	parity = parity ^ (hweight8(c) & 1);
+	parity = parity ^ !!parity8(c);
 	return parity;
 }
 
-- 
2.34.1

[PATCH v3 07/16] lib/bch: Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change parity32(mask) to !!parity32(mask).

 lib/bch.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/bch.c b/lib/bch.c
index 1c0cb07cdfeb..6c29122c0982 100644
--- a/lib/bch.c
+++ b/lib/bch.c
@@ -311,18 +311,6 @@ static inline int deg(unsigned int poly)
 	return fls(poly)-1;
 }
 
-static inline int parity(unsigned int x)
-{
-	/*
-	 * public domain code snippet, lifted from
-	 * http://www-graphics.stanford.edu/~seander/bithacks.html
-	 */
-	x ^= x >> 1;
-	x ^= x >> 2;
-	x = (x & 0x11111111U) * 0x11111111U;
-	return (x >> 28) & 1;
-}
-
 /* Galois field basic operations: multiply, divide, inverse, etc. */
 
 static inline unsigned int gf_mul(struct bch_control *bch, unsigned int a,
@@ -524,7 +512,7 @@ static int solve_linear_system(struct bch_control *bch, unsigned int *rows,
 		tmp = 0;
 		for (r = m-1; r >= 0; r--) {
 			mask = rows[r] & (tmp|1);
-			tmp |= parity(mask) << (m-r);
+			tmp |= !!parity32(mask) << (m-r);
 		}
 		sol[p] = tmp >> 1;
 	}
-- 
2.34.1

[PATCH v3 08/16] Input: joystick - Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change condition if(parity32(pkt) == 1) to if(parity32(pkt)).

 drivers/input/joystick/grip_mp.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/input/joystick/grip_mp.c b/drivers/input/joystick/grip_mp.c
index 5eadb5a3ca37..26a7ae785942 100644
--- a/drivers/input/joystick/grip_mp.c
+++ b/drivers/input/joystick/grip_mp.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/proc_fs.h>
 #include <linux/jiffies.h>
+#include <linux/bitops.h>
 
 #define DRIVER_DESC	"Gravis Grip Multiport driver"
 
@@ -112,20 +113,6 @@ static const int axis_map[] = { 5, 9, 1, 5, 6, 10, 2, 6, 4, 8, 0, 4, 5, 9, 1, 5
 
 static int register_slot(int i, struct grip_mp *grip);
 
-/*
- * Returns whether an odd or even number of bits are on in pkt.
- */
-
-static int bit_parity(u32 pkt)
-{
-	int x = pkt ^ (pkt >> 16);
-	x ^= x >> 8;
-	x ^= x >> 4;
-	x ^= x >> 2;
-	x ^= x >> 1;
-	return x & 1;
-}
-
 /*
  * Poll gameport; return true if all bits set in 'onbits' are on and
  * all bits set in 'offbits' are off.
@@ -236,7 +223,7 @@ static int mp_io(struct gameport* gameport, int sendflags, int sendcode, u32 *pa
 		pkt = (pkt >> 2) | 0xf0000000;
 	}
 
-	if (bit_parity(pkt) == 1)
+	if (parity32(pkt))
 		return IO_RESET;
 
 	/* Acknowledge packet receipt */
-- 
2.34.1

[PATCH v3 09/16] net: ethernet: oa_tc6: Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 drivers/net/ethernet/oa_tc6.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
index db200e4ec284..f02dba7b89a1 100644
--- a/drivers/net/ethernet/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/iopoll.h>
 #include <linux/mdio.h>
 #include <linux/phy.h>
@@ -177,19 +178,6 @@ static int oa_tc6_spi_transfer(struct oa_tc6 *tc6,
 	return spi_sync(tc6->spi, &msg);
 }
 
-static int oa_tc6_get_parity(u32 p)
-{
-	/* Public domain code snippet, lifted from
-	 * http://www-graphics.stanford.edu/~seander/bithacks.html
-	 */
-	p ^= p >> 1;
-	p ^= p >> 2;
-	p = (p & 0x11111111U) * 0x11111111U;
-
-	/* Odd parity is used here */
-	return !((p >> 28) & 1);
-}
-
 static __be32 oa_tc6_prepare_ctrl_header(u32 addr, u8 length,
 					 enum oa_tc6_register_op reg_op)
 {
@@ -202,7 +190,7 @@ static __be32 oa_tc6_prepare_ctrl_header(u32 addr, u8 length,
 		 FIELD_PREP(OA_TC6_CTRL_HEADER_ADDR, addr) |
 		 FIELD_PREP(OA_TC6_CTRL_HEADER_LENGTH, length - 1);
 	header |= FIELD_PREP(OA_TC6_CTRL_HEADER_PARITY,
-			     oa_tc6_get_parity(header));
+			     !parity32(header));
 
 	return cpu_to_be32(header);
 }
@@ -940,8 +928,7 @@ static __be32 oa_tc6_prepare_data_header(bool data_valid, bool start_valid,
 		     FIELD_PREP(OA_TC6_DATA_HEADER_END_BYTE_OFFSET,
 				end_byte_offset);
 
-	header |= FIELD_PREP(OA_TC6_DATA_HEADER_PARITY,
-			     oa_tc6_get_parity(header));
+	header |= FIELD_PREP(OA_TC6_DATA_HEADER_PARITY, !parity32(header));
 
 	return cpu_to_be32(header);
 }
-- 
2.34.1

[PATCH v3 10/16] wifi: brcm80211: Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmsmac/dma.c   | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
index 80c35027787a..d1a1ecd97d42 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/pci.h>
+#include <linux/bitops.h>
 #include <net/cfg80211.h>
 #include <net/mac80211.h>
 
@@ -283,21 +284,6 @@ struct dma_info {
 	bool aligndesc_4k;
 };
 
-/* Check for odd number of 1's */
-static u32 parity32(__le32 data)
-{
-	/* no swap needed for counting 1's */
-	u32 par_data = *(u32 *)&data;
-
-	par_data ^= par_data >> 16;
-	par_data ^= par_data >> 8;
-	par_data ^= par_data >> 4;
-	par_data ^= par_data >> 2;
-	par_data ^= par_data >> 1;
-
-	return par_data & 1;
-}
-
 static bool dma64_dd_parity(struct dma64desc *dd)
 {
 	return parity32(dd->addrlow ^ dd->addrhigh ^ dd->ctrl1 ^ dd->ctrl2);
-- 
2.34.1

[PATCH v3 11/16] drm/bridge: dw-hdmi: Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change parity32(sample) to !!parity32(sample).

 drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
index cf1f66b7b192..a992ecb149d7 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
@@ -4,6 +4,7 @@
  *
  * Written and tested against the Designware HDMI Tx found in iMX6.
  */
+#include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
@@ -171,12 +172,7 @@ static void dw_hdmi_reformat_iec958(struct snd_dw_hdmi *dw,
 
 static u32 parity(u32 sample)
 {
-	sample ^= sample >> 16;
-	sample ^= sample >> 8;
-	sample ^= sample >> 4;
-	sample ^= sample >> 2;
-	sample ^= sample >> 1;
-	return (sample & 1) << 27;
+	return !!parity32(sample) << 27;
 }
 
 static void dw_hdmi_reformat_s24(struct snd_dw_hdmi *dw,
-- 
2.34.1

[PATCH v3 12/16] mtd: ssfdc: Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change variable 'parity' type from int to bool.

 drivers/mtd/ssfdc.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
index 46c01fa2ec46..53a72576a646 100644
--- a/drivers/mtd/ssfdc.c
+++ b/drivers/mtd/ssfdc.c
@@ -7,6 +7,7 @@
  * Based on NTFL and MTDBLOCK_RO drivers
  */
 
+#include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -178,24 +179,11 @@ static int read_raw_oob(struct mtd_info *mtd, loff_t offs, uint8_t *buf)
 	return 0;
 }
 
-/* Parity calculator on a word of n bit size */
-static int get_parity(int number, int size)
-{
- 	int k;
-	int parity;
-
-	parity = 1;
-	for (k = 0; k < size; k++) {
-		parity += (number >> k);
-		parity &= 1;
-	}
-	return parity;
-}
-
 /* Read and validate the logical block address field stored in the OOB */
 static int get_logical_address(uint8_t *oob_buf)
 {
-	int block_address, parity;
+	int block_address;
+	bool parity;
 	int offset[2] = {6, 11}; /* offset of the 2 address fields within OOB */
 	int j;
 	int ok = 0;
@@ -215,7 +203,7 @@ static int get_logical_address(uint8_t *oob_buf)
 			block_address &= 0x7FF;
 			block_address >>= 1;
 
-			if (get_parity(block_address, 10) != parity) {
+			if (parity32(block_address & 0x3ff) == parity) {
 				pr_debug("SSFDC_RO: logical address field%d"
 					"parity error(0x%04X)\n", j+1,
 					block_address);
-- 
2.34.1

[PATCH v3 13/16] fsi: i2cr: Replace open-coded parity calculation with parity32()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity32() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change parity ^= parity32(v) to parity != parity32(v).

 drivers/fsi/fsi-master-i2cr.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/fsi/fsi-master-i2cr.c b/drivers/fsi/fsi-master-i2cr.c
index 40f1f4d231e5..46511236bbca 100644
--- a/drivers/fsi/fsi-master-i2cr.c
+++ b/drivers/fsi/fsi-master-i2cr.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (C) IBM Corporation 2023 */
 
+#include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/fsi.h>
 #include <linux/i2c.h>
@@ -38,14 +39,7 @@ static const u8 i2cr_cfam[] = {
 
 static bool i2cr_check_parity32(u32 v, bool parity)
 {
-	u32 i;
-
-	for (i = 0; i < 32; ++i) {
-		if (v & (1u << i))
-			parity = !parity;
-	}
-
-	return parity;
+	return parity != parity32(v);
 }
 
 static bool i2cr_check_parity64(u64 v)
-- 
2.34.1

[PATCH v3 14/16] fsi: i2cr: Replace open-coded parity calculation with parity64()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity64() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change parity ^= parity64(v) to parity != parity64(v).

 drivers/fsi/fsi-master-i2cr.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/fsi/fsi-master-i2cr.c b/drivers/fsi/fsi-master-i2cr.c
index 46511236bbca..3356c478e395 100644
--- a/drivers/fsi/fsi-master-i2cr.c
+++ b/drivers/fsi/fsi-master-i2cr.c
@@ -44,15 +44,9 @@ static bool i2cr_check_parity32(u32 v, bool parity)
 
 static bool i2cr_check_parity64(u64 v)
 {
-	u32 i;
 	bool parity = I2CR_INITIAL_PARITY;
 
-	for (i = 0; i < 64; ++i) {
-		if (v & (1llu << i))
-			parity = !parity;
-	}
-
-	return parity;
+	return parity != parity64(v);
 }
 
 static u32 i2cr_get_command(u32 address, bool parity)
-- 
2.34.1

[PATCH v3 15/16] Input: joystick - Replace open-coded parity calculation with parity64()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity64() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 drivers/input/joystick/sidewinder.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/input/joystick/sidewinder.c b/drivers/input/joystick/sidewinder.c
index 3a5873e5fcb3..9fe980096f70 100644
--- a/drivers/input/joystick/sidewinder.c
+++ b/drivers/input/joystick/sidewinder.c
@@ -7,6 +7,7 @@
  * Microsoft SideWinder joystick family driver for Linux
  */
 
+#include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -240,21 +241,6 @@ static void sw_init_digital(struct gameport *gameport)
 	local_irq_restore(flags);
 }
 
-/*
- * sw_parity() computes parity of __u64
- */
-
-static int sw_parity(__u64 t)
-{
-	int x = t ^ (t >> 32);
-
-	x ^= x >> 16;
-	x ^= x >> 8;
-	x ^= x >> 4;
-	x ^= x >> 2;
-	x ^= x >> 1;
-	return x & 1;
-}
 
 /*
  * sw_ccheck() checks synchronization bits and computes checksum of nibbles.
@@ -316,7 +302,7 @@ static int sw_parse(unsigned char *buf, struct sw *sw)
 
 			for (i = 0; i < sw->number; i ++) {
 
-				if (sw_parity(GB(i*15,15)))
+				if (parity64(GB(i*15,15)))
 					return -1;
 
 				input_report_abs(sw->dev[i], ABS_X, GB(i*15+3,1) - GB(i*15+2,1));
@@ -333,7 +319,7 @@ static int sw_parse(unsigned char *buf, struct sw *sw)
 		case SW_ID_PP:
 		case SW_ID_FFP:
 
-			if (!sw_parity(GB(0,48)) || (hat = GB(42,4)) > 8)
+			if (!parity64(GB(0,48)) || (hat = GB(42,4)) > 8)
 				return -1;
 
 			dev = sw->dev[0];
@@ -354,7 +340,7 @@ static int sw_parse(unsigned char *buf, struct sw *sw)
 
 		case SW_ID_FSP:
 
-			if (!sw_parity(GB(0,43)) || (hat = GB(28,4)) > 8)
+			if (!parity64(GB(0,43)) || (hat = GB(28,4)) > 8)
 				return -1;
 
 			dev = sw->dev[0];
@@ -379,7 +365,7 @@ static int sw_parse(unsigned char *buf, struct sw *sw)
 
 		case SW_ID_FFW:
 
-			if (!sw_parity(GB(0,33)))
+			if (!parity64(GB(0,33)))
 				return -1;
 
 			dev = sw->dev[0];
-- 
2.34.1

[PATCH v3 16/16] nfp: bpf: Replace open-coded parity calculation with parity64()

[Kuan-Wei Chiu]

Refactor parity calculations to use the standard parity64() helper.
This change eliminates redundant implementations and improves code
efficiency.

Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
Changes in v3:
- Change parity64() to !!parity64().

 drivers/net/ethernet/netronome/nfp/nfp_asm.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
index 154399c5453f..14306f128497 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
@@ -295,11 +295,6 @@ static const u64 nfp_ustore_ecc_polynomials[NFP_USTORE_ECC_POLY_WORDS] = {
 	0x0daf69a46910ULL,
 };
 
-static bool parity(u64 value)
-{
-	return hweight64(value) & 1;
-}
-
 int nfp_ustore_check_valid_no_ecc(u64 insn)
 {
 	if (insn & ~GENMASK_ULL(NFP_USTORE_OP_BITS, 0))
@@ -314,7 +309,7 @@ u64 nfp_ustore_calc_ecc_insn(u64 insn)
 	int i;
 
 	for (i = 0; i < NFP_USTORE_ECC_POLY_WORDS; i++)
-		ecc |= parity(nfp_ustore_ecc_polynomials[i] & insn) << i;
+		ecc |= !!parity64(nfp_ustore_ecc_polynomials[i] & insn) << i;
 
 	return insn | (u64)ecc << NFP_USTORE_OP_BITS;
 }
-- 
2.34.1

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[David Laight]

On Fri,  7 Mar 2025 00:25:26 +0800
Kuan-Wei Chiu <visitorckw@gmail.com> wrote:

> Change return type to bool for better clarity. Update the kernel doc
> comment accordingly, including fixing "@value" to "@val" and adjusting
> examples. Also mark the function with __attribute_const__ to allow
> potential compiler optimizations.

If the result type is 'bool' you should just check it.
Not compare against true/false.

	David

> 
> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---
>  include/linux/bitops.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index c1cb53cf2f0f..44e5765b8bec 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>  
>  /**
>   * parity8 - get the parity of an u8 value
> - * @value: the value to be examined
> + * @val: the value to be examined
>   *
>   * Determine the parity of the u8 argument.
>   *
>   * Returns:
> - * 0 for even parity, 1 for odd parity
> + * false for even parity, true for odd parity
>   *
>   * Note: This function informs you about the current parity. Example to bail
>   * out when parity is odd:
>   *
> - *	if (parity8(val) == 1)
> + *	if (parity8(val) == true)
>   *		return -EBADMSG;
>   *
>   * If you need to calculate a parity bit, you need to draw the conclusion from
>   * this result yourself. Example to enforce odd parity, parity bit is bit 7:
>   *
> - *	if (parity8(val) == 0)
> + *	if (parity8(val) == false)
>   *		val ^= BIT(7);
>   */
> -static inline int parity8(u8 val)
> +static inline __attribute_const__ bool parity8(u8 val)
>  {
>  	/*
>  	 * One explanation of this algorithm:

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 6, 2025 8:25:25 AM PST, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
>Several parts of the kernel contain redundant implementations of parity
>calculations for 16/32/64-bit values. Introduces generic
>parity16/32/64() helpers in bitops.h, providing a standardized
>and optimized implementation. 
>
>Subsequent patches refactor various kernel components to replace
>open-coded parity calculations with the new helpers, reducing code
>duplication and improving maintainability.
>
>Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>---
>In v3, I use parityXX() instead of the parity() macro since the
>parity() macro may generate suboptimal code and requires special hacks
>to make GCC happy. If anyone still prefers a single parity() macro,
>please let me know.
>
>Additionally, I changed parityXX() << y users to !!parityXX() << y
>because, unlike C++, C does not guarantee that true casts to int as 1.
>
>Changes in v3:
>- Avoid using __builtin_parity.
>- Change return type to bool.
>- Drop parity() macro.
>- Change parityXX() << y to !!parityXX() << y.
>
>
>Changes in v2:
>- Provide fallback functions for __builtin_parity() when the compiler
>  decides not to inline it
>- Use __builtin_parity() when no architecture-specific implementation
>  is available
>- Optimize for constant folding when val is a compile-time constant
>- Add a generic parity() macro
>- Drop the x86 bootflag conversion patch since it has been merged into
>  the tip tree
>
>v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitorckw@gmail.com/
>v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitorckw@gmail.com/
>
>Kuan-Wei Chiu (16):
>  bitops: Change parity8() return type to bool
>  bitops: Add parity16(), parity32(), and parity64() helpers
>  media: media/test_drivers: Replace open-coded parity calculation with
>    parity8()
>  media: pci: cx18-av-vbi: Replace open-coded parity calculation with
>    parity8()
>  media: saa7115: Replace open-coded parity calculation with parity8()
>  serial: max3100: Replace open-coded parity calculation with parity8()
>  lib/bch: Replace open-coded parity calculation with parity32()
>  Input: joystick - Replace open-coded parity calculation with
>    parity32()
>  net: ethernet: oa_tc6: Replace open-coded parity calculation with
>    parity32()
>  wifi: brcm80211: Replace open-coded parity calculation with parity32()
>  drm/bridge: dw-hdmi: Replace open-coded parity calculation with
>    parity32()
>  mtd: ssfdc: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity64()
>  Input: joystick - Replace open-coded parity calculation with
>    parity64()
>  nfp: bpf: Replace open-coded parity calculation with parity64()
>
> drivers/fsi/fsi-master-i2cr.c                 | 18 ++-----
> .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  8 +--
> drivers/input/joystick/grip_mp.c              | 17 +-----
> drivers/input/joystick/sidewinder.c           | 24 ++-------
> drivers/media/i2c/saa7115.c                   | 12 +----
> drivers/media/pci/cx18/cx18-av-vbi.c          | 12 +----
> .../media/test-drivers/vivid/vivid-vbi-gen.c  |  8 +--
> drivers/mtd/ssfdc.c                           | 20 ++-----
> drivers/net/ethernet/netronome/nfp/nfp_asm.c  |  7 +--
> drivers/net/ethernet/oa_tc6.c                 | 19 ++-----
> .../broadcom/brcm80211/brcmsmac/dma.c         | 16 +-----
> drivers/tty/serial/max3100.c                  |  3 +-
> include/linux/bitops.h                        | 52 +++++++++++++++++--
> lib/bch.c                                     | 14 +----
> 14 files changed, 77 insertions(+), 153 deletions(-)
>

(int)true most definitely is guaranteed to be 1.

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 6, 2025 8:25:25 AM PST, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
>Several parts of the kernel contain redundant implementations of parity
>calculations for 16/32/64-bit values. Introduces generic
>parity16/32/64() helpers in bitops.h, providing a standardized
>and optimized implementation. 
>
>Subsequent patches refactor various kernel components to replace
>open-coded parity calculations with the new helpers, reducing code
>duplication and improving maintainability.
>
>Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>---
>In v3, I use parityXX() instead of the parity() macro since the
>parity() macro may generate suboptimal code and requires special hacks
>to make GCC happy. If anyone still prefers a single parity() macro,
>please let me know.
>
>Additionally, I changed parityXX() << y users to !!parityXX() << y
>because, unlike C++, C does not guarantee that true casts to int as 1.
>
>Changes in v3:
>- Avoid using __builtin_parity.
>- Change return type to bool.
>- Drop parity() macro.
>- Change parityXX() << y to !!parityXX() << y.
>
>
>Changes in v2:
>- Provide fallback functions for __builtin_parity() when the compiler
>  decides not to inline it
>- Use __builtin_parity() when no architecture-specific implementation
>  is available
>- Optimize for constant folding when val is a compile-time constant
>- Add a generic parity() macro
>- Drop the x86 bootflag conversion patch since it has been merged into
>  the tip tree
>
>v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitorckw@gmail.com/
>v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitorckw@gmail.com/
>
>Kuan-Wei Chiu (16):
>  bitops: Change parity8() return type to bool
>  bitops: Add parity16(), parity32(), and parity64() helpers
>  media: media/test_drivers: Replace open-coded parity calculation with
>    parity8()
>  media: pci: cx18-av-vbi: Replace open-coded parity calculation with
>    parity8()
>  media: saa7115: Replace open-coded parity calculation with parity8()
>  serial: max3100: Replace open-coded parity calculation with parity8()
>  lib/bch: Replace open-coded parity calculation with parity32()
>  Input: joystick - Replace open-coded parity calculation with
>    parity32()
>  net: ethernet: oa_tc6: Replace open-coded parity calculation with
>    parity32()
>  wifi: brcm80211: Replace open-coded parity calculation with parity32()
>  drm/bridge: dw-hdmi: Replace open-coded parity calculation with
>    parity32()
>  mtd: ssfdc: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity32()
>  fsi: i2cr: Replace open-coded parity calculation with parity64()
>  Input: joystick - Replace open-coded parity calculation with
>    parity64()
>  nfp: bpf: Replace open-coded parity calculation with parity64()
>
> drivers/fsi/fsi-master-i2cr.c                 | 18 ++-----
> .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  8 +--
> drivers/input/joystick/grip_mp.c              | 17 +-----
> drivers/input/joystick/sidewinder.c           | 24 ++-------
> drivers/media/i2c/saa7115.c                   | 12 +----
> drivers/media/pci/cx18/cx18-av-vbi.c          | 12 +----
> .../media/test-drivers/vivid/vivid-vbi-gen.c  |  8 +--
> drivers/mtd/ssfdc.c                           | 20 ++-----
> drivers/net/ethernet/netronome/nfp/nfp_asm.c  |  7 +--
> drivers/net/ethernet/oa_tc6.c                 | 19 ++-----
> .../broadcom/brcm80211/brcmsmac/dma.c         | 16 +-----
> drivers/tty/serial/max3100.c                  |  3 +-
> include/linux/bitops.h                        | 52 +++++++++++++++++--
> lib/bch.c                                     | 14 +----
> 14 files changed, 77 insertions(+), 153 deletions(-)
>

!!x is used with a value that is not necessary booleanized already, and is exactly equivalent to (x ? true : false). It is totally redundant on a value known to be bool.

If (int)true wasn't inherently 1, then !! wouldn't work either. 

There was a time when some code would use as a temporary hack: 

typedef enum { false, true } bool;

... when compiling on pre-C99 compilers; in that case a (bool) case wouldn't necessarily work as expected, whereas !! would. Furthermore, unlike (bool), !! works in the preprocessor.

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Jiri Slaby]

On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> Change return type to bool for better clarity. Update the kernel doc
> comment accordingly, including fixing "@value" to "@val" and adjusting
> examples. Also mark the function with __attribute_const__ to allow
> potential compiler optimizations.
> 
> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---
>   include/linux/bitops.h | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index c1cb53cf2f0f..44e5765b8bec 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>   
>   /**
>    * parity8 - get the parity of an u8 value
> - * @value: the value to be examined
> + * @val: the value to be examined
>    *
>    * Determine the parity of the u8 argument.
>    *
>    * Returns:
> - * 0 for even parity, 1 for odd parity
> + * false for even parity, true for odd parity

This occurs somehow inverted to me. When something is in parity means 
that it has equal number of 1s and 0s. I.e. return true for even 
distribution. Dunno what others think? Or perhaps this should be dubbed 
odd_parity() when bool is returned? Then you'd return true for odd.

thanks,
-- 
js
suse labs

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Jiri Slaby]

On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> Several parts of the kernel contain redundant implementations of parity
> calculations for 16/32/64-bit values. Introduces generic
> parity16/32/64() helpers in bitops.h, providing a standardized
> and optimized implementation.
> 
> Subsequent patches refactor various kernel components to replace
> open-coded parity calculations with the new helpers, reducing code
> duplication and improving maintainability.
> 
> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---
> In v3, I use parityXX() instead of the parity() macro since the
> parity() macro may generate suboptimal code and requires special hacks
> to make GCC happy. If anyone still prefers a single parity() macro,
> please let me know.

What is suboptimal and where exactly it matters? Have you actually 
measured it?

> Additionally, I changed parityXX() << y users to !!parityXX() << y
> because, unlike C++, C does not guarantee that true casts to int as 1.

How comes? ANSI C99 exactly states:
===
true
which expands to the integer constant 1,
===

thanks,
-- 
js
suse labs

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

+Cc Waiman Long for bool cast to int discussion

Hi Peter,

On Thu, Mar 06, 2025 at 07:14:13PM -0800, H. Peter Anvin wrote:
> On March 6, 2025 8:25:25 AM PST, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> >Several parts of the kernel contain redundant implementations of parity
> >calculations for 16/32/64-bit values. Introduces generic
> >parity16/32/64() helpers in bitops.h, providing a standardized
> >and optimized implementation. 
> >
> >Subsequent patches refactor various kernel components to replace
> >open-coded parity calculations with the new helpers, reducing code
> >duplication and improving maintainability.
> >
> >Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> >Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> >Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> >---
> >In v3, I use parityXX() instead of the parity() macro since the
> >parity() macro may generate suboptimal code and requires special hacks
> >to make GCC happy. If anyone still prefers a single parity() macro,
> >please let me know.
> >
> >Additionally, I changed parityXX() << y users to !!parityXX() << y
> >because, unlike C++, C does not guarantee that true casts to int as 1.
> >
> >Changes in v3:
> >- Avoid using __builtin_parity.
> >- Change return type to bool.
> >- Drop parity() macro.
> >- Change parityXX() << y to !!parityXX() << y.
> >
> >
> >Changes in v2:
> >- Provide fallback functions for __builtin_parity() when the compiler
> >  decides not to inline it
> >- Use __builtin_parity() when no architecture-specific implementation
> >  is available
> >- Optimize for constant folding when val is a compile-time constant
> >- Add a generic parity() macro
> >- Drop the x86 bootflag conversion patch since it has been merged into
> >  the tip tree
> >
> >v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitorckw@gmail.com/
> >v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitorckw@gmail.com/
> >
> >Kuan-Wei Chiu (16):
> >  bitops: Change parity8() return type to bool
> >  bitops: Add parity16(), parity32(), and parity64() helpers
> >  media: media/test_drivers: Replace open-coded parity calculation with
> >    parity8()
> >  media: pci: cx18-av-vbi: Replace open-coded parity calculation with
> >    parity8()
> >  media: saa7115: Replace open-coded parity calculation with parity8()
> >  serial: max3100: Replace open-coded parity calculation with parity8()
> >  lib/bch: Replace open-coded parity calculation with parity32()
> >  Input: joystick - Replace open-coded parity calculation with
> >    parity32()
> >  net: ethernet: oa_tc6: Replace open-coded parity calculation with
> >    parity32()
> >  wifi: brcm80211: Replace open-coded parity calculation with parity32()
> >  drm/bridge: dw-hdmi: Replace open-coded parity calculation with
> >    parity32()
> >  mtd: ssfdc: Replace open-coded parity calculation with parity32()
> >  fsi: i2cr: Replace open-coded parity calculation with parity32()
> >  fsi: i2cr: Replace open-coded parity calculation with parity64()
> >  Input: joystick - Replace open-coded parity calculation with
> >    parity64()
> >  nfp: bpf: Replace open-coded parity calculation with parity64()
> >
> > drivers/fsi/fsi-master-i2cr.c                 | 18 ++-----
> > .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  8 +--
> > drivers/input/joystick/grip_mp.c              | 17 +-----
> > drivers/input/joystick/sidewinder.c           | 24 ++-------
> > drivers/media/i2c/saa7115.c                   | 12 +----
> > drivers/media/pci/cx18/cx18-av-vbi.c          | 12 +----
> > .../media/test-drivers/vivid/vivid-vbi-gen.c  |  8 +--
> > drivers/mtd/ssfdc.c                           | 20 ++-----
> > drivers/net/ethernet/netronome/nfp/nfp_asm.c  |  7 +--
> > drivers/net/ethernet/oa_tc6.c                 | 19 ++-----
> > .../broadcom/brcm80211/brcmsmac/dma.c         | 16 +-----
> > drivers/tty/serial/max3100.c                  |  3 +-
> > include/linux/bitops.h                        | 52 +++++++++++++++++--
> > lib/bch.c                                     | 14 +----
> > 14 files changed, 77 insertions(+), 153 deletions(-)
> >
> 
> !!x is used with a value that is not necessary booleanized already, and is exactly equivalent to (x ? true : false). It is totally redundant on a value known to be bool.
> 
I used to believe that casting a boolean variable to int would always
result in 0 or 1 until a few months ago when Waiman Long explicitly
pointed out during a review that C does not guarantee this.

So I revisited the C11 standard, which states that casting to _Bool
always results in 0 or 1 [1]. Another section specifies that bool,
true, and false are macros defined in <stdbool.h>, with true expanding
to 1 and false to 0. However, these macros can be #undef and redefined
to other values [2]. I'm not sure if this is sufficient to conclude
that casting bool to int will always result in 0 or 1, but if the
consensus is that it does, I'll remove the !! hack in the next version.

> If (int)true wasn't inherently 1, then !! wouldn't work either. 
> 
The C standard guarantees that the ! operator returns an int, either 0
or 1. So regardless of how true casts, using !! should work. Right?

> There was a time when some code would use as a temporary hack: 
> 
> typedef enum { false, true } bool;
> 
> ... when compiling on pre-C99 compilers; in that case a (bool) case wouldn't necessarily work as expected, whereas !! would. Furthermore, unlike (bool), !! works in the preprocessor.

I'm not entirely sure how !! works in the preprocessor. I always
thought it was handled by the compiler. Could you elaborate on this?

Regards,
Kuan-Wei

[1]: 6.3.1.2 Boolean type
1 When any scalar value is converted to _Bool, the result is 0 if the value compares equal
to 0; otherwise, the result is 1.59)

[2]: 7.18 Boolean type and values <stdbool.h>
1 The header <stdbool.h> defines four macros.
2 The macro
bool
expands to _Bool.
3 The remaining three macros are suitable for use in #if preprocessing directives. They
are
true
which expands to the integer constant 1,
false
which expands to the integer constant 0, and
_ _bool_true_false_are_defined
which expands to the integer constant 1.
4 Notwithstanding the provisions of 7.1.3, a program may undefine and perhaps then
redefine the macros bool, true, and false.
259)

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

Hi Jiri,

On Fri, Mar 07, 2025 at 07:57:48AM +0100, Jiri Slaby wrote:
> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> > Several parts of the kernel contain redundant implementations of parity
> > calculations for 16/32/64-bit values. Introduces generic
> > parity16/32/64() helpers in bitops.h, providing a standardized
> > and optimized implementation.
> > 
> > Subsequent patches refactor various kernel components to replace
> > open-coded parity calculations with the new helpers, reducing code
> > duplication and improving maintainability.
> > 
> > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> > In v3, I use parityXX() instead of the parity() macro since the
> > parity() macro may generate suboptimal code and requires special hacks
> > to make GCC happy. If anyone still prefers a single parity() macro,
> > please let me know.
> 
> What is suboptimal and where exactly it matters? Have you actually measured
> it?
> 
In the previous thread, David and Yury had different opinions regarding
the implementation details of the parity() macro. I am trying to find a
solution that satisfies most people while keeping it as simple as
possible.

I cannot point to any specific users who are particularly concerned
about efficiency, so personally, I am not really concerned about the
generated code either. However, I am not a fan of the #if gcc #else
approach, and Yury also mentioned that he does not like the >> 16 >> 16
hack. At the same time, David pointed out that GCC might generate
double-register math. Given these concerns, I leaned toward reverting
to the parityXX() approach.

If you still prefer using the parity() macro, we can revisit and
discuss its implementation details further.

> > Additionally, I changed parityXX() << y users to !!parityXX() << y
> > because, unlike C++, C does not guarantee that true casts to int as 1.
> 
> How comes? ANSI C99 exactly states:
> ===
> true
> which expands to the integer constant 1,
> ===
> 
I gave a more detailed response in my reply to Peter. If we can confirm
that casting bool to int will only result in 1 or 0, I will remove the
!! hack in the next version.

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Jiri Slaby]

On 07. 03. 25, 10:19, Kuan-Wei Chiu wrote:
> I used to believe that casting a boolean variable to int would always
> result in 0 or 1 until a few months ago when Waiman Long explicitly
> pointed out during a review that C does not guarantee this.
> 
> So I revisited the C11 standard, which states that casting to _Bool
> always results in 0 or 1 [1]. Another section specifies that bool,
> true, and false are macros defined in <stdbool.h>, with true expanding
> to 1 and false to 0. However, these macros can be #undef and redefined
> to other values [2].

Note that we do not have/use user's stdbool.h in kernel at all. Instead, 
in linux/stddef.h, we define:
enum {
         false   = 0,
         true    = 1
};

So all is blue.

thanks,
-- 
js
suse labs

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Ingo Molnar]

* Jiri Slaby <jirislaby@kernel.org> wrote:

> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> > Change return type to bool for better clarity. Update the kernel doc
> > comment accordingly, including fixing "@value" to "@val" and adjusting
> > examples. Also mark the function with __attribute_const__ to allow
> > potential compiler optimizations.
> > 
> > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> >   include/linux/bitops.h | 10 +++++-----
> >   1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index c1cb53cf2f0f..44e5765b8bec 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
> >   /**
> >    * parity8 - get the parity of an u8 value
> > - * @value: the value to be examined
> > + * @val: the value to be examined
> >    *
> >    * Determine the parity of the u8 argument.
> >    *
> >    * Returns:
> > - * 0 for even parity, 1 for odd parity
> > + * false for even parity, true for odd parity
> 
> This occurs somehow inverted to me. When something is in parity means that
> it has equal number of 1s and 0s. I.e. return true for even distribution.
> Dunno what others think? Or perhaps this should be dubbed odd_parity() when
> bool is returned? Then you'd return true for odd.

OTOH:

 - '0' is an even number and is returned for even parity,
 - '1' is an odd  number and is returned for odd  parity.

Thanks,

	Ingo

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Jiri Slaby]

On 07. 03. 25, 12:38, Ingo Molnar wrote:
> 
> * Jiri Slaby <jirislaby@kernel.org> wrote:
> 
>> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
>>> Change return type to bool for better clarity. Update the kernel doc
>>> comment accordingly, including fixing "@value" to "@val" and adjusting
>>> examples. Also mark the function with __attribute_const__ to allow
>>> potential compiler optimizations.
>>>
>>> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>>> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>>> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>>> ---
>>>    include/linux/bitops.h | 10 +++++-----
>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>>> index c1cb53cf2f0f..44e5765b8bec 100644
>>> --- a/include/linux/bitops.h
>>> +++ b/include/linux/bitops.h
>>> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>>>    /**
>>>     * parity8 - get the parity of an u8 value
>>> - * @value: the value to be examined
>>> + * @val: the value to be examined
>>>     *
>>>     * Determine the parity of the u8 argument.
>>>     *
>>>     * Returns:
>>> - * 0 for even parity, 1 for odd parity
>>> + * false for even parity, true for odd parity
>>
>> This occurs somehow inverted to me. When something is in parity means that
>> it has equal number of 1s and 0s. I.e. return true for even distribution.
>> Dunno what others think? Or perhaps this should be dubbed odd_parity() when
>> bool is returned? Then you'd return true for odd.
> 
> OTOH:
> 
>   - '0' is an even number and is returned for even parity,
>   - '1' is an odd  number and is returned for odd  parity.

Yes, that used to make sense for me. For bool/true/false, it no longer 
does. But as I wrote, it might be only me...

thanks,
-- 
js
suse labs

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Ingo Molnar]

* Jiri Slaby <jirislaby@kernel.org> wrote:

> On 07. 03. 25, 12:38, Ingo Molnar wrote:
> > 
> > * Jiri Slaby <jirislaby@kernel.org> wrote:
> > 
> > > On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> > > > Change return type to bool for better clarity. Update the kernel doc
> > > > comment accordingly, including fixing "@value" to "@val" and adjusting
> > > > examples. Also mark the function with __attribute_const__ to allow
> > > > potential compiler optimizations.
> > > > 
> > > > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > > > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > > > ---
> > > >    include/linux/bitops.h | 10 +++++-----
> > > >    1 file changed, 5 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > > > index c1cb53cf2f0f..44e5765b8bec 100644
> > > > --- a/include/linux/bitops.h
> > > > +++ b/include/linux/bitops.h
> > > > @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
> > > >    /**
> > > >     * parity8 - get the parity of an u8 value
> > > > - * @value: the value to be examined
> > > > + * @val: the value to be examined
> > > >     *
> > > >     * Determine the parity of the u8 argument.
> > > >     *
> > > >     * Returns:
> > > > - * 0 for even parity, 1 for odd parity
> > > > + * false for even parity, true for odd parity
> > > 
> > > This occurs somehow inverted to me. When something is in parity means that
> > > it has equal number of 1s and 0s. I.e. return true for even distribution.
> > > Dunno what others think? Or perhaps this should be dubbed odd_parity() when
> > > bool is returned? Then you'd return true for odd.
> > 
> > OTOH:
> > 
> >   - '0' is an even number and is returned for even parity,
> >   - '1' is an odd  number and is returned for odd  parity.
> 
> Yes, that used to make sense for me. For bool/true/false, it no longer does.
> But as I wrote, it might be only me...

No strong opinion on this from me either, I'd guess existing practice 
with other parity functions should probably control. (If a coherent 
praxis exists.).

Thanks,

	Ingo

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[H. Peter Anvin]

On March 7, 2025 4:13:26 AM PST, Ingo Molnar <mingo@kernel.org> wrote:
>
>* Jiri Slaby <jirislaby@kernel.org> wrote:
>
>> On 07. 03. 25, 12:38, Ingo Molnar wrote:
>> > 
>> > * Jiri Slaby <jirislaby@kernel.org> wrote:
>> > 
>> > > On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
>> > > > Change return type to bool for better clarity. Update the kernel doc
>> > > > comment accordingly, including fixing "@value" to "@val" and adjusting
>> > > > examples. Also mark the function with __attribute_const__ to allow
>> > > > potential compiler optimizations.
>> > > > 
>> > > > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>> > > > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>> > > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>> > > > ---
>> > > >    include/linux/bitops.h | 10 +++++-----
>> > > >    1 file changed, 5 insertions(+), 5 deletions(-)
>> > > > 
>> > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>> > > > index c1cb53cf2f0f..44e5765b8bec 100644
>> > > > --- a/include/linux/bitops.h
>> > > > +++ b/include/linux/bitops.h
>> > > > @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>> > > >    /**
>> > > >     * parity8 - get the parity of an u8 value
>> > > > - * @value: the value to be examined
>> > > > + * @val: the value to be examined
>> > > >     *
>> > > >     * Determine the parity of the u8 argument.
>> > > >     *
>> > > >     * Returns:
>> > > > - * 0 for even parity, 1 for odd parity
>> > > > + * false for even parity, true for odd parity
>> > > 
>> > > This occurs somehow inverted to me. When something is in parity means that
>> > > it has equal number of 1s and 0s. I.e. return true for even distribution.
>> > > Dunno what others think? Or perhaps this should be dubbed odd_parity() when
>> > > bool is returned? Then you'd return true for odd.
>> > 
>> > OTOH:
>> > 
>> >   - '0' is an even number and is returned for even parity,
>> >   - '1' is an odd  number and is returned for odd  parity.
>> 
>> Yes, that used to make sense for me. For bool/true/false, it no longer does.
>> But as I wrote, it might be only me...
>
>No strong opinion on this from me either, I'd guess existing practice 
>with other parity functions should probably control. (If a coherent 
>praxis exists.).
>
>Thanks,
>
>	Ingo

Instead of "bool" think of it as "bit" and it makes more sense

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Yury Norov]

On Fri, Mar 07, 2025 at 07:57:48AM +0100, Jiri Slaby wrote:
> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> > Several parts of the kernel contain redundant implementations of parity
> > calculations for 16/32/64-bit values. Introduces generic
> > parity16/32/64() helpers in bitops.h, providing a standardized
> > and optimized implementation.
> > 
> > Subsequent patches refactor various kernel components to replace
> > open-coded parity calculations with the new helpers, reducing code
> > duplication and improving maintainability.
> > 
> > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > ---
> > In v3, I use parityXX() instead of the parity() macro since the
> > parity() macro may generate suboptimal code and requires special hacks
> > to make GCC happy. If anyone still prefers a single parity() macro,
> > please let me know.
> 
> What is suboptimal and where exactly it matters? Have you actually measured
> it?

I asked exactly this question at least 3 times, and have never
received perf tests or asm listings - nothing. I've never received
any comments from driver maintainers about how performance of the
parity() is important for them, as well.

With the absence of _any_ feedback, I'm not going to take this series,
of course, for the reason: overengineering.

With that said, the simplest way would be replacing parity8(u8) with
parity(u64) 'one size fits all' thing. I even made a one extra step,
suggesting a macro that would generate a better code for smaller types
with almost no extra maintenance burden. This is another acceptable
option to me.

Thanks,
Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

Hi Yury,

On Fri, Mar 07, 2025 at 10:55:13AM -0500, Yury Norov wrote:
> On Fri, Mar 07, 2025 at 07:57:48AM +0100, Jiri Slaby wrote:
> > On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> > > Several parts of the kernel contain redundant implementations of parity
> > > calculations for 16/32/64-bit values. Introduces generic
> > > parity16/32/64() helpers in bitops.h, providing a standardized
> > > and optimized implementation.
> > > 
> > > Subsequent patches refactor various kernel components to replace
> > > open-coded parity calculations with the new helpers, reducing code
> > > duplication and improving maintainability.
> > > 
> > > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > > ---
> > > In v3, I use parityXX() instead of the parity() macro since the
> > > parity() macro may generate suboptimal code and requires special hacks
> > > to make GCC happy. If anyone still prefers a single parity() macro,
> > > please let me know.
> > 
> > What is suboptimal and where exactly it matters? Have you actually measured
> > it?
> 
> I asked exactly this question at least 3 times, and have never
> received perf tests or asm listings - nothing. I've never received
> any comments from driver maintainers about how performance of the
> parity() is important for them, as well.
> 
To be clear, I use parityXX() was mainly because you dislike the >>
16 >> 16 hack, and I dislike the #if gcc #else hack—not due to
performance or generated code considerations.

> With the absence of _any_ feedback, I'm not going to take this series,
> of course, for the reason: overengineering.
> 
I'm quite surprised that three separate one-line functions are
considered overengineering compared to a multi-line approach that
requires special handling to satisfy gcc.

> With that said, the simplest way would be replacing parity8(u8) with
> parity(u64) 'one size fits all' thing. I even made a one extra step,
> suggesting a macro that would generate a better code for smaller types
> with almost no extra maintenance burden. This is another acceptable
> option to me.
> 
I'm fine with unifying everything to a single parity(u64) function.
Before I submit the next version, please let me know if anyone has
objections to this approach.

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Andrew Cooper]

> (int)true most definitely is guaranteed to be 1.

That's not technically correct any more.

GCC has introduced hardened bools that intentionally have bit patterns
other than 0 and 1.

https://gcc.gnu.org/gcc-14/changes.html

~Andrew

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Yury Norov]

On Fri, Mar 07, 2025 at 04:14:34AM -0800, H. Peter Anvin wrote:
> On March 7, 2025 4:13:26 AM PST, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >* Jiri Slaby <jirislaby@kernel.org> wrote:
> >
> >> On 07. 03. 25, 12:38, Ingo Molnar wrote:
> >> > 
> >> > * Jiri Slaby <jirislaby@kernel.org> wrote:
> >> > 
> >> > > On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
> >> > > > Change return type to bool for better clarity. Update the kernel doc
> >> > > > comment accordingly, including fixing "@value" to "@val" and adjusting
> >> > > > examples. Also mark the function with __attribute_const__ to allow
> >> > > > potential compiler optimizations.
> >> > > > 
> >> > > > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> >> > > > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> >> > > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> >> > > > ---
> >> > > >    include/linux/bitops.h | 10 +++++-----
> >> > > >    1 file changed, 5 insertions(+), 5 deletions(-)
> >> > > > 
> >> > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> >> > > > index c1cb53cf2f0f..44e5765b8bec 100644
> >> > > > --- a/include/linux/bitops.h
> >> > > > +++ b/include/linux/bitops.h
> >> > > > @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
> >> > > >    /**
> >> > > >     * parity8 - get the parity of an u8 value
> >> > > > - * @value: the value to be examined
> >> > > > + * @val: the value to be examined
> >> > > >     *
> >> > > >     * Determine the parity of the u8 argument.
> >> > > >     *
> >> > > >     * Returns:
> >> > > > - * 0 for even parity, 1 for odd parity
> >> > > > + * false for even parity, true for odd parity
> >> > > 
> >> > > This occurs somehow inverted to me. When something is in parity means that
> >> > > it has equal number of 1s and 0s. I.e. return true for even distribution.
> >> > > Dunno what others think? Or perhaps this should be dubbed odd_parity() when
> >> > > bool is returned? Then you'd return true for odd.
> >> > 
> >> > OTOH:
> >> > 
> >> >   - '0' is an even number and is returned for even parity,
> >> >   - '1' is an odd  number and is returned for odd  parity.
> >> 
> >> Yes, that used to make sense for me. For bool/true/false, it no longer does.
> >> But as I wrote, it might be only me...
> >
> >No strong opinion on this from me either, I'd guess existing practice 
> >with other parity functions should probably control. (If a coherent 
> >praxis exists.).
> >
> >Thanks,
> >
> >	Ingo
> 
> Instead of "bool" think of it as "bit" and it makes more sense

So, to help people thinking that way we can introduce a corresponding
type:
        typedef unsigned _BitInt(1) u1;

It already works for clang, and GCC is going to adopt it with std=c23.
We can make u1 an alias to bool for GCC for a while. If you guys like
it, I can send a patch.

For clang it prints quite a nice overflow warning:

tst.c:59:9: warning: implicit conversion from 'int' to 'u1' (aka 'unsigned _BitInt(1)') changes value from 2 to 0 [-Wconstant-conversion]
   59 |         u1 r = 2;
      |            ~   ^

Thanks,
Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> (int)true most definitely is guaranteed to be 1.
>
>That's not technically correct any more.
>
>GCC has introduced hardened bools that intentionally have bit patterns
>other than 0 and 1.
>
>https://gcc.gnu.org/gcc-14/changes.html
>
>~Andrew

Bit patterns in memory maybe (not that I can see the Linux kernel using them) but for compiler-generated conversations that's still a given, or the manager isn't C or anything even remotely like it.

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[H. Peter Anvin]

On March 7, 2025 11:30:08 AM PST, Yury Norov <yury.norov@gmail.com> wrote:
>On Fri, Mar 07, 2025 at 04:14:34AM -0800, H. Peter Anvin wrote:
>> On March 7, 2025 4:13:26 AM PST, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> >* Jiri Slaby <jirislaby@kernel.org> wrote:
>> >
>> >> On 07. 03. 25, 12:38, Ingo Molnar wrote:
>> >> > 
>> >> > * Jiri Slaby <jirislaby@kernel.org> wrote:
>> >> > 
>> >> > > On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:
>> >> > > > Change return type to bool for better clarity. Update the kernel doc
>> >> > > > comment accordingly, including fixing "@value" to "@val" and adjusting
>> >> > > > examples. Also mark the function with __attribute_const__ to allow
>> >> > > > potential compiler optimizations.
>> >> > > > 
>> >> > > > Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>> >> > > > Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>> >> > > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>> >> > > > ---
>> >> > > >    include/linux/bitops.h | 10 +++++-----
>> >> > > >    1 file changed, 5 insertions(+), 5 deletions(-)
>> >> > > > 
>> >> > > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>> >> > > > index c1cb53cf2f0f..44e5765b8bec 100644
>> >> > > > --- a/include/linux/bitops.h
>> >> > > > +++ b/include/linux/bitops.h
>> >> > > > @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>> >> > > >    /**
>> >> > > >     * parity8 - get the parity of an u8 value
>> >> > > > - * @value: the value to be examined
>> >> > > > + * @val: the value to be examined
>> >> > > >     *
>> >> > > >     * Determine the parity of the u8 argument.
>> >> > > >     *
>> >> > > >     * Returns:
>> >> > > > - * 0 for even parity, 1 for odd parity
>> >> > > > + * false for even parity, true for odd parity
>> >> > > 
>> >> > > This occurs somehow inverted to me. When something is in parity means that
>> >> > > it has equal number of 1s and 0s. I.e. return true for even distribution.
>> >> > > Dunno what others think? Or perhaps this should be dubbed odd_parity() when
>> >> > > bool is returned? Then you'd return true for odd.
>> >> > 
>> >> > OTOH:
>> >> > 
>> >> >   - '0' is an even number and is returned for even parity,
>> >> >   - '1' is an odd  number and is returned for odd  parity.
>> >> 
>> >> Yes, that used to make sense for me. For bool/true/false, it no longer does.
>> >> But as I wrote, it might be only me...
>> >
>> >No strong opinion on this from me either, I'd guess existing practice 
>> >with other parity functions should probably control. (If a coherent 
>> >praxis exists.).
>> >
>> >Thanks,
>> >
>> >	Ingo
>> 
>> Instead of "bool" think of it as "bit" and it makes more sense
>
>So, to help people thinking that way we can introduce a corresponding
>type:
>        typedef unsigned _BitInt(1) u1;
>
>It already works for clang, and GCC is going to adopt it with std=c23.
>We can make u1 an alias to bool for GCC for a while. If you guys like
>it, I can send a patch.
>
>For clang it prints quite a nice overflow warning:
>
>tst.c:59:9: warning: implicit conversion from 'int' to 'u1' (aka 'unsigned _BitInt(1)') changes value from 2 to 0 [-Wconstant-conversion]
>   59 |         u1 r = 2;
>      |            ~   ^
>
>Thanks,
>Yury

No, for a whole bunch of reasons.

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[David Laight]

On Fri, 7 Mar 2025 12:42:41 +0100
Jiri Slaby <jirislaby@kernel.org> wrote:

> On 07. 03. 25, 12:38, Ingo Molnar wrote:
> > 
> > * Jiri Slaby <jirislaby@kernel.org> wrote:
> >   
> >> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:  
> >>> Change return type to bool for better clarity. Update the kernel doc
> >>> comment accordingly, including fixing "@value" to "@val" and adjusting
> >>> examples. Also mark the function with __attribute_const__ to allow
> >>> potential compiler optimizations.
> >>>
> >>> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
> >>> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
> >>> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> >>> ---
> >>>    include/linux/bitops.h | 10 +++++-----
> >>>    1 file changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> >>> index c1cb53cf2f0f..44e5765b8bec 100644
> >>> --- a/include/linux/bitops.h
> >>> +++ b/include/linux/bitops.h
> >>> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
> >>>    /**
> >>>     * parity8 - get the parity of an u8 value
> >>> - * @value: the value to be examined
> >>> + * @val: the value to be examined
> >>>     *
> >>>     * Determine the parity of the u8 argument.
> >>>     *
> >>>     * Returns:
> >>> - * 0 for even parity, 1 for odd parity
> >>> + * false for even parity, true for odd parity  
> >>
> >> This occurs somehow inverted to me. When something is in parity means that
> >> it has equal number of 1s and 0s. I.e. return true for even distribution.
> >> Dunno what others think? Or perhaps this should be dubbed odd_parity() when
> >> bool is returned? Then you'd return true for odd.  
> > 
> > OTOH:
> > 
> >   - '0' is an even number and is returned for even parity,
> >   - '1' is an odd  number and is returned for odd  parity.  
> 
> Yes, that used to make sense for me. For bool/true/false, it no longer 
> does. But as I wrote, it might be only me...

No me as well, I've made the same comment before.
When reading code I don't want to have to look up a function definition.
There is even scope for having parity_odd() and parity_even().
And, with the version that shifts a constant right you want to invert
the constant!

	David

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[H. Peter Anvin]

On March 7, 2025 11:36:43 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
>On Fri, 7 Mar 2025 12:42:41 +0100
>Jiri Slaby <jirislaby@kernel.org> wrote:
>
>> On 07. 03. 25, 12:38, Ingo Molnar wrote:
>> > 
>> > * Jiri Slaby <jirislaby@kernel.org> wrote:
>> >   
>> >> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:  
>> >>> Change return type to bool for better clarity. Update the kernel doc
>> >>> comment accordingly, including fixing "@value" to "@val" and adjusting
>> >>> examples. Also mark the function with __attribute_const__ to allow
>> >>> potential compiler optimizations.
>> >>>
>> >>> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>> >>> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>> >>> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>> >>> ---
>> >>>    include/linux/bitops.h | 10 +++++-----
>> >>>    1 file changed, 5 insertions(+), 5 deletions(-)
>> >>>
>> >>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>> >>> index c1cb53cf2f0f..44e5765b8bec 100644
>> >>> --- a/include/linux/bitops.h
>> >>> +++ b/include/linux/bitops.h
>> >>> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>> >>>    /**
>> >>>     * parity8 - get the parity of an u8 value
>> >>> - * @value: the value to be examined
>> >>> + * @val: the value to be examined
>> >>>     *
>> >>>     * Determine the parity of the u8 argument.
>> >>>     *
>> >>>     * Returns:
>> >>> - * 0 for even parity, 1 for odd parity
>> >>> + * false for even parity, true for odd parity  
>> >>
>> >> This occurs somehow inverted to me. When something is in parity means that
>> >> it has equal number of 1s and 0s. I.e. return true for even distribution.
>> >> Dunno what others think? Or perhaps this should be dubbed odd_parity() when
>> >> bool is returned? Then you'd return true for odd.  
>> > 
>> > OTOH:
>> > 
>> >   - '0' is an even number and is returned for even parity,
>> >   - '1' is an odd  number and is returned for odd  parity.  
>> 
>> Yes, that used to make sense for me. For bool/true/false, it no longer 
>> does. But as I wrote, it might be only me...
>
>No me as well, I've made the same comment before.
>When reading code I don't want to have to look up a function definition.
>There is even scope for having parity_odd() and parity_even().
>And, with the version that shifts a constant right you want to invert
>the constant!
>
>	David
>
>
>
>

Of course, for me, if I saw "parity_odd()" I would think of it as a function that caused the parity to become odd, i.e.

if (!parity(x))
  x ^= 1 << 7;

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[David Laight]

On Fri, 07 Mar 2025 11:30:35 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> (int)true most definitely is guaranteed to be 1.  
> >
> >That's not technically correct any more.
> >
> >GCC has introduced hardened bools that intentionally have bit patterns
> >other than 0 and 1.
> >
> >https://gcc.gnu.org/gcc-14/changes.html
> >
> >~Andrew  
> 
> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> for compiler-generated conversations that's still a given, or the manager isn't C
> or anything even remotely like it.
> 

The whole idea of 'bool' is pretty much broken by design.
The underlying problem is that values other than 'true' and 'false' can
always get into 'bool' variables.

Once that has happened it is all fubar.

Trying to sanitise a value with (say):
int f(bool v)
{
	return (int)v & 1;
}    
just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)

I really don't see how using (say) 0xaa and 0x55 helps.
What happens if the value is wrong? a trap or exception?, good luck recovering
from that.

	David

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
>On Fri, 07 Mar 2025 11:30:35 -0800
>"H. Peter Anvin" <hpa@zytor.com> wrote:
>
>> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> >> (int)true most definitely is guaranteed to be 1.  
>> >
>> >That's not technically correct any more.
>> >
>> >GCC has introduced hardened bools that intentionally have bit patterns
>> >other than 0 and 1.
>> >
>> >https://gcc.gnu.org/gcc-14/changes.html
>> >
>> >~Andrew  
>> 
>> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
>> for compiler-generated conversations that's still a given, or the manager isn't C
>> or anything even remotely like it.
>> 
>
>The whole idea of 'bool' is pretty much broken by design.
>The underlying problem is that values other than 'true' and 'false' can
>always get into 'bool' variables.
>
>Once that has happened it is all fubar.
>
>Trying to sanitise a value with (say):
>int f(bool v)
>{
>	return (int)v & 1;
>}    
>just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
>
>I really don't see how using (say) 0xaa and 0x55 helps.
>What happens if the value is wrong? a trap or exception?, good luck recovering
>from that.
>
>	David

Did you just discover GIGO?

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> >On Fri, 07 Mar 2025 11:30:35 -0800
> >"H. Peter Anvin" <hpa@zytor.com> wrote:
> >
> >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> >> (int)true most definitely is guaranteed to be 1.  
> >> >
> >> >That's not technically correct any more.
> >> >
> >> >GCC has introduced hardened bools that intentionally have bit patterns
> >> >other than 0 and 1.
> >> >
> >> >https://gcc.gnu.org/gcc-14/changes.html
> >> >
> >> >~Andrew  
> >> 
> >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> >> for compiler-generated conversations that's still a given, or the manager isn't C
> >> or anything even remotely like it.
> >> 
> >
> >The whole idea of 'bool' is pretty much broken by design.
> >The underlying problem is that values other than 'true' and 'false' can
> >always get into 'bool' variables.
> >
> >Once that has happened it is all fubar.
> >
> >Trying to sanitise a value with (say):
> >int f(bool v)
> >{
> >	return (int)v & 1;
> >}    
> >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> >
> >I really don't see how using (say) 0xaa and 0x55 helps.
> >What happens if the value is wrong? a trap or exception?, good luck recovering
> >from that.
> >
> >	David
> 
> Did you just discover GIGO?

Thanks for all the suggestions.

I don't have a strong opinion on the naming or return type. I'm still a
bit confused about whether I can assume that casting bool to int always
results in 0 or 1.

If that's the case, since most people prefer bool over int as the
return type and some are against introducing u1, my current plan is to
use the following in the next version:

bool parity_odd(u64 val);

This keeps the bool return type, renames the function for better
clarity, and avoids extra maintenance burden by having just one
function.

If I can't assume that casting bool to int always results in 0 or 1,
would it be acceptable to keep the return type as int?

Would this work for everyone?

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 9, 2025 8:48:26 AM PDT, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
>On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
>> On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
>> >On Fri, 07 Mar 2025 11:30:35 -0800
>> >"H. Peter Anvin" <hpa@zytor.com> wrote:
>> >
>> >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> >> >> (int)true most definitely is guaranteed to be 1.  
>> >> >
>> >> >That's not technically correct any more.
>> >> >
>> >> >GCC has introduced hardened bools that intentionally have bit patterns
>> >> >other than 0 and 1.
>> >> >
>> >> >https://gcc.gnu.org/gcc-14/changes.html
>> >> >
>> >> >~Andrew  
>> >> 
>> >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
>> >> for compiler-generated conversations that's still a given, or the manager isn't C
>> >> or anything even remotely like it.
>> >> 
>> >
>> >The whole idea of 'bool' is pretty much broken by design.
>> >The underlying problem is that values other than 'true' and 'false' can
>> >always get into 'bool' variables.
>> >
>> >Once that has happened it is all fubar.
>> >
>> >Trying to sanitise a value with (say):
>> >int f(bool v)
>> >{
>> >	return (int)v & 1;
>> >}    
>> >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
>> >
>> >I really don't see how using (say) 0xaa and 0x55 helps.
>> >What happens if the value is wrong? a trap or exception?, good luck recovering
>> >from that.
>> >
>> >	David
>> 
>> Did you just discover GIGO?
>
>Thanks for all the suggestions.
>
>I don't have a strong opinion on the naming or return type. I'm still a
>bit confused about whether I can assume that casting bool to int always
>results in 0 or 1.
>
>If that's the case, since most people prefer bool over int as the
>return type and some are against introducing u1, my current plan is to
>use the following in the next version:
>
>bool parity_odd(u64 val);
>
>This keeps the bool return type, renames the function for better
>clarity, and avoids extra maintenance burden by having just one
>function.
>
>If I can't assume that casting bool to int always results in 0 or 1,
>would it be acceptable to keep the return type as int?
>
>Would this work for everyone?
>
>Regards,
>Kuan-Wei

You *CAN* safely assume that bool is an integer type which always has the value 0 or 1.

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Jiri Slaby]

On 09. 03. 25, 16:48, Kuan-Wei Chiu wrote:
> Would this work for everyone?

+1 for /me.

-- 
js
suse labs

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Yury Norov]

On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> > >On Fri, 07 Mar 2025 11:30:35 -0800
> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> > >
> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > >> >> (int)true most definitely is guaranteed to be 1.  
> > >> >
> > >> >That's not technically correct any more.
> > >> >
> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> > >> >other than 0 and 1.
> > >> >
> > >> >https://gcc.gnu.org/gcc-14/changes.html
> > >> >
> > >> >~Andrew  
> > >> 
> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> > >> or anything even remotely like it.
> > >> 
> > >
> > >The whole idea of 'bool' is pretty much broken by design.
> > >The underlying problem is that values other than 'true' and 'false' can
> > >always get into 'bool' variables.
> > >
> > >Once that has happened it is all fubar.
> > >
> > >Trying to sanitise a value with (say):
> > >int f(bool v)
> > >{
> > >	return (int)v & 1;
> > >}    
> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> > >
> > >I really don't see how using (say) 0xaa and 0x55 helps.
> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> > >from that.
> > >
> > >	David
> > 
> > Did you just discover GIGO?
> 
> Thanks for all the suggestions.
> 
> I don't have a strong opinion on the naming or return type. I'm still a
> bit confused about whether I can assume that casting bool to int always
> results in 0 or 1.
> 
> If that's the case, since most people prefer bool over int as the
> return type and some are against introducing u1, my current plan is to
> use the following in the next version:
> 
> bool parity_odd(u64 val);
> 
> This keeps the bool return type, renames the function for better
> clarity, and avoids extra maintenance burden by having just one
> function.
> 
> If I can't assume that casting bool to int always results in 0 or 1,
> would it be acceptable to keep the return type as int?
> 
> Would this work for everyone?

Alright, it's clearly a split opinion. So what I would do myself in
such case is to look at existing code and see what people who really
need parity invent in their drivers:

                                     bool      parity_odd
static inline int parity8(u8 val)       -               -
static u8 calc_parity(u8 val)           -               -
static int odd_parity(u8 c)             -               +
static int saa711x_odd_parity           -               +
static int max3100_do_parity            -               -
static inline int parity(unsigned x)    -               -
static int bit_parity(u32 pkt)          -               -
static int oa_tc6_get_parity(u32 p)     -               -
static u32 parity32(__le32 data)        -               -
static u32 parity(u32 sample)           -               -
static int get_parity(int number,       -               -
                      int size)
static bool i2cr_check_parity32(u32 v,  +               -
                        bool parity)
static bool i2cr_check_parity64(u64 v)  +               -
static int sw_parity(__u64 t)           -               -
static bool parity(u64 value)           +               -

Now you can refer to that table say that int parity(uXX) is what
people want to see in their drivers.

Whichever interface you choose, please discuss it's pros and cons.
What bloat-o-meter says for each option? What's maintenance burden?
Perf test? Look at generated code?

I personally for a macro returning boolean, something like I
proposed at the very beginning.

Thanks,
Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
>On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
>> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
>> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
>> > >On Fri, 07 Mar 2025 11:30:35 -0800
>> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
>> > >
>> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> > >> >> (int)true most definitely is guaranteed to be 1.  
>> > >> >
>> > >> >That's not technically correct any more.
>> > >> >
>> > >> >GCC has introduced hardened bools that intentionally have bit patterns
>> > >> >other than 0 and 1.
>> > >> >
>> > >> >https://gcc.gnu.org/gcc-14/changes.html
>> > >> >
>> > >> >~Andrew  
>> > >> 
>> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
>> > >> for compiler-generated conversations that's still a given, or the manager isn't C
>> > >> or anything even remotely like it.
>> > >> 
>> > >
>> > >The whole idea of 'bool' is pretty much broken by design.
>> > >The underlying problem is that values other than 'true' and 'false' can
>> > >always get into 'bool' variables.
>> > >
>> > >Once that has happened it is all fubar.
>> > >
>> > >Trying to sanitise a value with (say):
>> > >int f(bool v)
>> > >{
>> > >	return (int)v & 1;
>> > >}    
>> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
>> > >
>> > >I really don't see how using (say) 0xaa and 0x55 helps.
>> > >What happens if the value is wrong? a trap or exception?, good luck recovering
>> > >from that.
>> > >
>> > >	David
>> > 
>> > Did you just discover GIGO?
>> 
>> Thanks for all the suggestions.
>> 
>> I don't have a strong opinion on the naming or return type. I'm still a
>> bit confused about whether I can assume that casting bool to int always
>> results in 0 or 1.
>> 
>> If that's the case, since most people prefer bool over int as the
>> return type and some are against introducing u1, my current plan is to
>> use the following in the next version:
>> 
>> bool parity_odd(u64 val);
>> 
>> This keeps the bool return type, renames the function for better
>> clarity, and avoids extra maintenance burden by having just one
>> function.
>> 
>> If I can't assume that casting bool to int always results in 0 or 1,
>> would it be acceptable to keep the return type as int?
>> 
>> Would this work for everyone?
>
>Alright, it's clearly a split opinion. So what I would do myself in
>such case is to look at existing code and see what people who really
>need parity invent in their drivers:
>
>                                     bool      parity_odd
>static inline int parity8(u8 val)       -               -
>static u8 calc_parity(u8 val)           -               -
>static int odd_parity(u8 c)             -               +
>static int saa711x_odd_parity           -               +
>static int max3100_do_parity            -               -
>static inline int parity(unsigned x)    -               -
>static int bit_parity(u32 pkt)          -               -
>static int oa_tc6_get_parity(u32 p)     -               -
>static u32 parity32(__le32 data)        -               -
>static u32 parity(u32 sample)           -               -
>static int get_parity(int number,       -               -
>                      int size)
>static bool i2cr_check_parity32(u32 v,  +               -
>                        bool parity)
>static bool i2cr_check_parity64(u64 v)  +               -
>static int sw_parity(__u64 t)           -               -
>static bool parity(u64 value)           +               -
>
>Now you can refer to that table say that int parity(uXX) is what
>people want to see in their drivers.
>
>Whichever interface you choose, please discuss it's pros and cons.
>What bloat-o-meter says for each option? What's maintenance burden?
>Perf test? Look at generated code?
>
>I personally for a macro returning boolean, something like I
>proposed at the very beginning.
>
>Thanks,
>Yury

Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Yury Norov]

On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
> On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
> >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> >> > >On Fri, 07 Mar 2025 11:30:35 -0800
> >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> >> > >
> >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> > >> >> (int)true most definitely is guaranteed to be 1.  
> >> > >> >
> >> > >> >That's not technically correct any more.
> >> > >> >
> >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> >> > >> >other than 0 and 1.
> >> > >> >
> >> > >> >https://gcc.gnu.org/gcc-14/changes.html
> >> > >> >
> >> > >> >~Andrew  
> >> > >> 
> >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> >> > >> or anything even remotely like it.
> >> > >> 
> >> > >
> >> > >The whole idea of 'bool' is pretty much broken by design.
> >> > >The underlying problem is that values other than 'true' and 'false' can
> >> > >always get into 'bool' variables.
> >> > >
> >> > >Once that has happened it is all fubar.
> >> > >
> >> > >Trying to sanitise a value with (say):
> >> > >int f(bool v)
> >> > >{
> >> > >	return (int)v & 1;
> >> > >}    
> >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> >> > >
> >> > >I really don't see how using (say) 0xaa and 0x55 helps.
> >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> >> > >from that.
> >> > >
> >> > >	David
> >> > 
> >> > Did you just discover GIGO?
> >> 
> >> Thanks for all the suggestions.
> >> 
> >> I don't have a strong opinion on the naming or return type. I'm still a
> >> bit confused about whether I can assume that casting bool to int always
> >> results in 0 or 1.
> >> 
> >> If that's the case, since most people prefer bool over int as the
> >> return type and some are against introducing u1, my current plan is to
> >> use the following in the next version:
> >> 
> >> bool parity_odd(u64 val);
> >> 
> >> This keeps the bool return type, renames the function for better
> >> clarity, and avoids extra maintenance burden by having just one
> >> function.
> >> 
> >> If I can't assume that casting bool to int always results in 0 or 1,
> >> would it be acceptable to keep the return type as int?
> >> 
> >> Would this work for everyone?
> >
> >Alright, it's clearly a split opinion. So what I would do myself in
> >such case is to look at existing code and see what people who really
> >need parity invent in their drivers:
> >
> >                                     bool      parity_odd
> >static inline int parity8(u8 val)       -               -
> >static u8 calc_parity(u8 val)           -               -
> >static int odd_parity(u8 c)             -               +
> >static int saa711x_odd_parity           -               +
> >static int max3100_do_parity            -               -
> >static inline int parity(unsigned x)    -               -
> >static int bit_parity(u32 pkt)          -               -
> >static int oa_tc6_get_parity(u32 p)     -               -
> >static u32 parity32(__le32 data)        -               -
> >static u32 parity(u32 sample)           -               -
> >static int get_parity(int number,       -               -
> >                      int size)
> >static bool i2cr_check_parity32(u32 v,  +               -
> >                        bool parity)
> >static bool i2cr_check_parity64(u64 v)  +               -
> >static int sw_parity(__u64 t)           -               -
> >static bool parity(u64 value)           +               -
> >
> >Now you can refer to that table say that int parity(uXX) is what
> >people want to see in their drivers.
> >
> >Whichever interface you choose, please discuss it's pros and cons.
> >What bloat-o-meter says for each option? What's maintenance burden?
> >Perf test? Look at generated code?
> >
> >I personally for a macro returning boolean, something like I
> >proposed at the very beginning.
> >
> >Thanks,
> >Yury
> 
> Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.

Yeah. And because linux/bitops.h already includes asm/bitops.h
the simplest way would be wrapping generic implementation with
the #ifndef parity, similarly to how we handle find_next_bit case.

So:
1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
2. This may, and probably should, be a separate follow-up series,
   likely created by corresponding arch experts.

Thanks,
Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Wed, Mar 12, 2025 at 11:51:12AM -0400, Yury Norov wrote:
> On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
> > On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
> > >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> > >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> > >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> > >> > >On Fri, 07 Mar 2025 11:30:35 -0800
> > >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> > >> > >
> > >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > >> > >> >> (int)true most definitely is guaranteed to be 1.  
> > >> > >> >
> > >> > >> >That's not technically correct any more.
> > >> > >> >
> > >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> > >> > >> >other than 0 and 1.
> > >> > >> >
> > >> > >> >https://gcc.gnu.org/gcc-14/changes.html
> > >> > >> >
> > >> > >> >~Andrew  
> > >> > >> 
> > >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> > >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> > >> > >> or anything even remotely like it.
> > >> > >> 
> > >> > >
> > >> > >The whole idea of 'bool' is pretty much broken by design.
> > >> > >The underlying problem is that values other than 'true' and 'false' can
> > >> > >always get into 'bool' variables.
> > >> > >
> > >> > >Once that has happened it is all fubar.
> > >> > >
> > >> > >Trying to sanitise a value with (say):
> > >> > >int f(bool v)
> > >> > >{
> > >> > >	return (int)v & 1;
> > >> > >}    
> > >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> > >> > >
> > >> > >I really don't see how using (say) 0xaa and 0x55 helps.
> > >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> > >> > >from that.
> > >> > >
> > >> > >	David
> > >> > 
> > >> > Did you just discover GIGO?
> > >> 
> > >> Thanks for all the suggestions.
> > >> 
> > >> I don't have a strong opinion on the naming or return type. I'm still a
> > >> bit confused about whether I can assume that casting bool to int always
> > >> results in 0 or 1.
> > >> 
> > >> If that's the case, since most people prefer bool over int as the
> > >> return type and some are against introducing u1, my current plan is to
> > >> use the following in the next version:
> > >> 
> > >> bool parity_odd(u64 val);
> > >> 
> > >> This keeps the bool return type, renames the function for better
> > >> clarity, and avoids extra maintenance burden by having just one
> > >> function.
> > >> 
> > >> If I can't assume that casting bool to int always results in 0 or 1,
> > >> would it be acceptable to keep the return type as int?
> > >> 
> > >> Would this work for everyone?
> > >
> > >Alright, it's clearly a split opinion. So what I would do myself in
> > >such case is to look at existing code and see what people who really
> > >need parity invent in their drivers:
> > >
> > >                                     bool      parity_odd
> > >static inline int parity8(u8 val)       -               -
> > >static u8 calc_parity(u8 val)           -               -
> > >static int odd_parity(u8 c)             -               +
> > >static int saa711x_odd_parity           -               +
> > >static int max3100_do_parity            -               -
> > >static inline int parity(unsigned x)    -               -
> > >static int bit_parity(u32 pkt)          -               -
> > >static int oa_tc6_get_parity(u32 p)     -               -
> > >static u32 parity32(__le32 data)        -               -
> > >static u32 parity(u32 sample)           -               -
> > >static int get_parity(int number,       -               -
> > >                      int size)
> > >static bool i2cr_check_parity32(u32 v,  +               -
> > >                        bool parity)
> > >static bool i2cr_check_parity64(u64 v)  +               -
> > >static int sw_parity(__u64 t)           -               -
> > >static bool parity(u64 value)           +               -
> > >
> > >Now you can refer to that table say that int parity(uXX) is what
> > >people want to see in their drivers.
> > >
> > >Whichever interface you choose, please discuss it's pros and cons.
> > >What bloat-o-meter says for each option? What's maintenance burden?
> > >Perf test? Look at generated code?
> > >
> > >I personally for a macro returning boolean, something like I
> > >proposed at the very beginning.
> > >
> > >Thanks,
> > >Yury
> > 
> > Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.
> 
> Yeah. And because linux/bitops.h already includes asm/bitops.h
> the simplest way would be wrapping generic implementation with
> the #ifndef parity, similarly to how we handle find_next_bit case.
> 
> So:
> 1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
> 2. This may, and probably should, be a separate follow-up series,
>    likely created by corresponding arch experts.
> 
I saw discussions in the previous email thread about both
__builtin_parity and x86-specific implementations. However, from the
discussion, I learned that before considering any optimization, we
should first ask: which driver or subsystem actually cares about parity
efficiency? If someone does, I can help with a micro-benchmark to
provide performance numbers, but I don't have enough domain knowledge
to identify hot paths where parity efficiency matters.

Regards,
Kuan-Wei

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Jacob Keller]

On 3/7/2025 11:36 AM, David Laight wrote:
> On Fri, 7 Mar 2025 12:42:41 +0100
> Jiri Slaby <jirislaby@kernel.org> wrote:
> 
>> On 07. 03. 25, 12:38, Ingo Molnar wrote:
>>>
>>> * Jiri Slaby <jirislaby@kernel.org> wrote:
>>>   
>>>> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:  
>>>>> Change return type to bool for better clarity. Update the kernel doc
>>>>> comment accordingly, including fixing "@value" to "@val" and adjusting
>>>>> examples. Also mark the function with __attribute_const__ to allow
>>>>> potential compiler optimizations.
>>>>>
>>>>> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>>>>> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>>>>> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>>>>> ---
>>>>>    include/linux/bitops.h | 10 +++++-----
>>>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>>>>> index c1cb53cf2f0f..44e5765b8bec 100644
>>>>> --- a/include/linux/bitops.h
>>>>> +++ b/include/linux/bitops.h
>>>>> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>>>>>    /**
>>>>>     * parity8 - get the parity of an u8 value
>>>>> - * @value: the value to be examined
>>>>> + * @val: the value to be examined
>>>>>     *
>>>>>     * Determine the parity of the u8 argument.
>>>>>     *
>>>>>     * Returns:
>>>>> - * 0 for even parity, 1 for odd parity
>>>>> + * false for even parity, true for odd parity  
>>>>
>>>> This occurs somehow inverted to me. When something is in parity means that
>>>> it has equal number of 1s and 0s. I.e. return true for even distribution.
>>>> Dunno what others think? Or perhaps this should be dubbed odd_parity() when
>>>> bool is returned? Then you'd return true for odd.  
>>>
>>> OTOH:
>>>
>>>   - '0' is an even number and is returned for even parity,
>>>   - '1' is an odd  number and is returned for odd  parity.  
>>
>> Yes, that used to make sense for me. For bool/true/false, it no longer 
>> does. But as I wrote, it might be only me...
> 
> No me as well, I've made the same comment before.
> When reading code I don't want to have to look up a function definition.
> There is even scope for having parity_odd() and parity_even().
> And, with the version that shifts a constant right you want to invert
> the constant!
> 
> 	David

This is really a question of whether you expect odd or even parity as
the "true" value. I think that would depend on context, and we may not
reach a good consensus.

I do agree that my brain would jump to "true is even, false is odd".
However, I also agree returning the value as 0 for even and 1 for odd
kind of made sense before, and updating this to be a bool and then
requiring to switch all the callers is a bit obnoxious...

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[H. Peter Anvin]

On March 12, 2025 4:56:31 PM PDT, Jacob Keller <jacob.e.keller@intel.com> wrote:
>
>
>On 3/7/2025 11:36 AM, David Laight wrote:
>> On Fri, 7 Mar 2025 12:42:41 +0100
>> Jiri Slaby <jirislaby@kernel.org> wrote:
>> 
>>> On 07. 03. 25, 12:38, Ingo Molnar wrote:
>>>>
>>>> * Jiri Slaby <jirislaby@kernel.org> wrote:
>>>>   
>>>>> On 06. 03. 25, 17:25, Kuan-Wei Chiu wrote:  
>>>>>> Change return type to bool for better clarity. Update the kernel doc
>>>>>> comment accordingly, including fixing "@value" to "@val" and adjusting
>>>>>> examples. Also mark the function with __attribute_const__ to allow
>>>>>> potential compiler optimizations.
>>>>>>
>>>>>> Co-developed-by: Yu-Chun Lin <eleanor15x@gmail.com>
>>>>>> Signed-off-by: Yu-Chun Lin <eleanor15x@gmail.com>
>>>>>> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
>>>>>> ---
>>>>>>    include/linux/bitops.h | 10 +++++-----
>>>>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>>>>>> index c1cb53cf2f0f..44e5765b8bec 100644
>>>>>> --- a/include/linux/bitops.h
>>>>>> +++ b/include/linux/bitops.h
>>>>>> @@ -231,26 +231,26 @@ static inline int get_count_order_long(unsigned long l)
>>>>>>    /**
>>>>>>     * parity8 - get the parity of an u8 value
>>>>>> - * @value: the value to be examined
>>>>>> + * @val: the value to be examined
>>>>>>     *
>>>>>>     * Determine the parity of the u8 argument.
>>>>>>     *
>>>>>>     * Returns:
>>>>>> - * 0 for even parity, 1 for odd parity
>>>>>> + * false for even parity, true for odd parity  
>>>>>
>>>>> This occurs somehow inverted to me. When something is in parity means that
>>>>> it has equal number of 1s and 0s. I.e. return true for even distribution.
>>>>> Dunno what others think? Or perhaps this should be dubbed odd_parity() when
>>>>> bool is returned? Then you'd return true for odd.  
>>>>
>>>> OTOH:
>>>>
>>>>   - '0' is an even number and is returned for even parity,
>>>>   - '1' is an odd  number and is returned for odd  parity.  
>>>
>>> Yes, that used to make sense for me. For bool/true/false, it no longer 
>>> does. But as I wrote, it might be only me...
>> 
>> No me as well, I've made the same comment before.
>> When reading code I don't want to have to look up a function definition.
>> There is even scope for having parity_odd() and parity_even().
>> And, with the version that shifts a constant right you want to invert
>> the constant!
>> 
>> 	David
>
>This is really a question of whether you expect odd or even parity as
>the "true" value. I think that would depend on context, and we may not
>reach a good consensus.
>
>I do agree that my brain would jump to "true is even, false is odd".
>However, I also agree returning the value as 0 for even and 1 for odd
>kind of made sense before, and updating this to be a bool and then
>requiring to switch all the callers is a bit obnoxious...

Odd = 1 = true is the only same definition. It is a bitwise XOR, or sum mod 1.

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Thu, Mar 13, 2025 at 12:29:13AM +0800, Kuan-Wei Chiu wrote:
> On Wed, Mar 12, 2025 at 11:51:12AM -0400, Yury Norov wrote:
> > On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
> > > On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
> > > >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> > > >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> > > >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> > > >> > >On Fri, 07 Mar 2025 11:30:35 -0800
> > > >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> > > >> > >
> > > >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > > >> > >> >> (int)true most definitely is guaranteed to be 1.  
> > > >> > >> >
> > > >> > >> >That's not technically correct any more.
> > > >> > >> >
> > > >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> > > >> > >> >other than 0 and 1.
> > > >> > >> >
> > > >> > >> >https://gcc.gnu.org/gcc-14/changes.html
> > > >> > >> >
> > > >> > >> >~Andrew  
> > > >> > >> 
> > > >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> > > >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> > > >> > >> or anything even remotely like it.
> > > >> > >> 
> > > >> > >
> > > >> > >The whole idea of 'bool' is pretty much broken by design.
> > > >> > >The underlying problem is that values other than 'true' and 'false' can
> > > >> > >always get into 'bool' variables.
> > > >> > >
> > > >> > >Once that has happened it is all fubar.
> > > >> > >
> > > >> > >Trying to sanitise a value with (say):
> > > >> > >int f(bool v)
> > > >> > >{
> > > >> > >	return (int)v & 1;
> > > >> > >}    
> > > >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> > > >> > >
> > > >> > >I really don't see how using (say) 0xaa and 0x55 helps.
> > > >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> > > >> > >from that.
> > > >> > >
> > > >> > >	David
> > > >> > 
> > > >> > Did you just discover GIGO?
> > > >> 
> > > >> Thanks for all the suggestions.
> > > >> 
> > > >> I don't have a strong opinion on the naming or return type. I'm still a
> > > >> bit confused about whether I can assume that casting bool to int always
> > > >> results in 0 or 1.
> > > >> 
> > > >> If that's the case, since most people prefer bool over int as the
> > > >> return type and some are against introducing u1, my current plan is to
> > > >> use the following in the next version:
> > > >> 
> > > >> bool parity_odd(u64 val);
> > > >> 
> > > >> This keeps the bool return type, renames the function for better
> > > >> clarity, and avoids extra maintenance burden by having just one
> > > >> function.
> > > >> 
> > > >> If I can't assume that casting bool to int always results in 0 or 1,
> > > >> would it be acceptable to keep the return type as int?
> > > >> 
> > > >> Would this work for everyone?
> > > >
> > > >Alright, it's clearly a split opinion. So what I would do myself in
> > > >such case is to look at existing code and see what people who really
> > > >need parity invent in their drivers:
> > > >
> > > >                                     bool      parity_odd
> > > >static inline int parity8(u8 val)       -               -
> > > >static u8 calc_parity(u8 val)           -               -
> > > >static int odd_parity(u8 c)             -               +
> > > >static int saa711x_odd_parity           -               +
> > > >static int max3100_do_parity            -               -
> > > >static inline int parity(unsigned x)    -               -
> > > >static int bit_parity(u32 pkt)          -               -
> > > >static int oa_tc6_get_parity(u32 p)     -               -
> > > >static u32 parity32(__le32 data)        -               -
> > > >static u32 parity(u32 sample)           -               -
> > > >static int get_parity(int number,       -               -
> > > >                      int size)
> > > >static bool i2cr_check_parity32(u32 v,  +               -
> > > >                        bool parity)
> > > >static bool i2cr_check_parity64(u64 v)  +               -
> > > >static int sw_parity(__u64 t)           -               -
> > > >static bool parity(u64 value)           +               -
> > > >
> > > >Now you can refer to that table say that int parity(uXX) is what
> > > >people want to see in their drivers.
> > > >
> > > >Whichever interface you choose, please discuss it's pros and cons.
> > > >What bloat-o-meter says for each option? What's maintenance burden?
> > > >Perf test? Look at generated code?
> > > >
> > > >I personally for a macro returning boolean, something like I
> > > >proposed at the very beginning.
> > > >
> > > >Thanks,
> > > >Yury
> > > 
> > > Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.
> > 
> > Yeah. And because linux/bitops.h already includes asm/bitops.h
> > the simplest way would be wrapping generic implementation with
> > the #ifndef parity, similarly to how we handle find_next_bit case.
> > 
> > So:
> > 1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
> > 2. This may, and probably should, be a separate follow-up series,
> >    likely created by corresponding arch experts.
> > 
> I saw discussions in the previous email thread about both
> __builtin_parity and x86-specific implementations. However, from the
> discussion, I learned that before considering any optimization, we
> should first ask: which driver or subsystem actually cares about parity
> efficiency? If someone does, I can help with a micro-benchmark to
> provide performance numbers, but I don't have enough domain knowledge
> to identify hot paths where parity efficiency matters.
> 
IMHO,

If parity is never used in any hot path and we don't care about parity:

Then benchmarking its performance seems meaningless. In this case, a
function with a u64 argument would suffice, and we might not even need
a macro to optimize for different types—especially since the macro
requires special hacks to avoid compiler warnings. Also, I don't think
code size matters here. If it does, we should first consider making
parity a non-inline function in a .c file rather than an inline
function/macro in a header.

If parity is used in a hot path:

We need different handling for different type sizes. As previously
discussed, x86 assembly might use different instructions for u8 and
u16. This may sound stubborn, but I want to ask again: should we
consider using parity8/16/32/64 interfaces? Like in the i3c driver
example, if we only have a single parity macro that selects an
implementation based on type size, users must explicitly cast types.
If future users also need parity in a hot path, they might not be aware
of this requirement and end up generating suboptimal code. Since we
care about efficiency and generated code, why not follow hweight() and
provide separate implementations for different sizes?

Regards,
Kuan-Wei

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Yury Norov]

On Wed, Mar 12, 2025 at 05:09:16PM -0700, H. Peter Anvin wrote:
> On March 12, 2025 4:56:31 PM PDT, Jacob Keller <jacob.e.keller@intel.com> wrote:

[...]

> >This is really a question of whether you expect odd or even parity as
> >the "true" value. I think that would depend on context, and we may not
> >reach a good consensus.
> >
> >I do agree that my brain would jump to "true is even, false is odd".
> >However, I also agree returning the value as 0 for even and 1 for odd
> >kind of made sense before, and updating this to be a bool and then
> >requiring to switch all the callers is a bit obnoxious...
> 
> Odd = 1 = true is the only same definition. It is a bitwise XOR, or sum mod 1.

The x86 implementation will be "popcnt(val) & 1", right? So if we
choose to go with odd == false, we'll have to add an extra negation.
So because it's a purely conventional thing, let's just pick a simpler
one?

Compiler's builtin parity() returns 1 for odd.

Thanks,
Yury

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Yury Norov]

On Fri, Mar 07, 2025 at 11:33:40AM -0800, H. Peter Anvin wrote:
> On March 7, 2025 11:30:08 AM PST, Yury Norov <yury.norov@gmail.com> wrote:

[...]

> >> Instead of "bool" think of it as "bit" and it makes more sense
> >
> >So, to help people thinking that way we can introduce a corresponding
> >type:
> >        typedef unsigned _BitInt(1) u1;
> >
> >It already works for clang, and GCC is going to adopt it with std=c23.
> >We can make u1 an alias to bool for GCC for a while. If you guys like
> >it, I can send a patch.
> >
> >For clang it prints quite a nice overflow warning:
> >
> >tst.c:59:9: warning: implicit conversion from 'int' to 'u1' (aka 'unsigned _BitInt(1)') changes value from 2 to 0 [-Wconstant-conversion]
> >   59 |         u1 r = 2;
> >      |            ~   ^
> >
> >Thanks,
> >Yury
> 
> No, for a whole bunch of reasons.

Can you please elaborate?

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[H. Peter Anvin]

On March 13, 2025 9:24:38 AM PDT, Yury Norov <yury.norov@gmail.com> wrote:
>On Wed, Mar 12, 2025 at 05:09:16PM -0700, H. Peter Anvin wrote:
>> On March 12, 2025 4:56:31 PM PDT, Jacob Keller <jacob.e.keller@intel.com> wrote:
>
>[...]
>
>> >This is really a question of whether you expect odd or even parity as
>> >the "true" value. I think that would depend on context, and we may not
>> >reach a good consensus.
>> >
>> >I do agree that my brain would jump to "true is even, false is odd".
>> >However, I also agree returning the value as 0 for even and 1 for odd
>> >kind of made sense before, and updating this to be a bool and then
>> >requiring to switch all the callers is a bit obnoxious...
>> 
>> Odd = 1 = true is the only same definition. It is a bitwise XOR, or sum mod 1.
>
>The x86 implementation will be "popcnt(val) & 1", right? So if we
>choose to go with odd == false, we'll have to add an extra negation.
>So because it's a purely conventional thing, let's just pick a simpler
>one?
>
>Compiler's builtin parity() returns 1 for odd.
>
>Thanks,
>Yury

The x86 implementation, no, but there will be plenty of others having that exact definition.

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[Jacob Keller]

On 3/13/2025 9:36 AM, H. Peter Anvin wrote:
> On March 13, 2025 9:24:38 AM PDT, Yury Norov <yury.norov@gmail.com> wrote:
>> On Wed, Mar 12, 2025 at 05:09:16PM -0700, H. Peter Anvin wrote:
>>> On March 12, 2025 4:56:31 PM PDT, Jacob Keller <jacob.e.keller@intel.com> wrote:
>>
>> [...]
>>
>>>> This is really a question of whether you expect odd or even parity as
>>>> the "true" value. I think that would depend on context, and we may not
>>>> reach a good consensus.
>>>>
>>>> I do agree that my brain would jump to "true is even, false is odd".
>>>> However, I also agree returning the value as 0 for even and 1 for odd
>>>> kind of made sense before, and updating this to be a bool and then
>>>> requiring to switch all the callers is a bit obnoxious...
>>>
>>> Odd = 1 = true is the only same definition. It is a bitwise XOR, or sum mod 1.
>>
>> The x86 implementation will be "popcnt(val) & 1", right? So if we
>> choose to go with odd == false, we'll have to add an extra negation.
>> So because it's a purely conventional thing, let's just pick a simpler
>> one?
>>
>> Compiler's builtin parity() returns 1 for odd.
>>
>> Thanks,
>> Yury
> 
> The x86 implementation, no, but there will be plenty of others having that exact definition.

Makes sense to stick with that existing convention then. Enough to
convince me.

Thanks,
Jake

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[David Laight]

On Thu, 13 Mar 2025 14:09:24 -0700
Jacob Keller <jacob.e.keller@intel.com> wrote:

> On 3/13/2025 9:36 AM, H. Peter Anvin wrote:
> > On March 13, 2025 9:24:38 AM PDT, Yury Norov <yury.norov@gmail.com> wrote:  
> >> On Wed, Mar 12, 2025 at 05:09:16PM -0700, H. Peter Anvin wrote:  
> >>> On March 12, 2025 4:56:31 PM PDT, Jacob Keller <jacob.e.keller@intel.com> wrote:  
> >>
> >> [...]
> >>  
> >>>> This is really a question of whether you expect odd or even parity as
> >>>> the "true" value. I think that would depend on context, and we may not
> >>>> reach a good consensus.
> >>>>
> >>>> I do agree that my brain would jump to "true is even, false is odd".
> >>>> However, I also agree returning the value as 0 for even and 1 for odd
> >>>> kind of made sense before, and updating this to be a bool and then
> >>>> requiring to switch all the callers is a bit obnoxious...  
> >>>
> >>> Odd = 1 = true is the only same definition. It is a bitwise XOR, or sum mod 1.  
> >>
> >> The x86 implementation will be "popcnt(val) & 1", right? So if we
> >> choose to go with odd == false, we'll have to add an extra negation.
> >> So because it's a purely conventional thing, let's just pick a simpler
> >> one?
> >>
> >> Compiler's builtin parity() returns 1 for odd.
> >>
> >> Thanks,
> >> Yury  
> > 
> > The x86 implementation, no, but there will be plenty of others having that exact definition.  
> 
> Makes sense to stick with that existing convention then. Enough to
> convince me.

There is the possibility that the compiler will treat the builtin as having
an 'int' result without the constraint of it being zero or one.
In which case the conversion to bool will be a compare.
This doesn't happen on x86-64 (gcc or clang) - but who knows elsewhere.

For x86 popcnt(val) & 1 is best (except for parity8) but requires a non-archaic cpu.
(Probably Nehelem or K10 or later - includes Sandy bridge and all the 'earth movers'.)
Since performance isn't critical the generic C code is actually ok.
(The 'parity' flag bit is only set on the low 8 bits.)

	David

Re: [PATCH v3 01/16] bitops: Change parity8() return type to bool

[H. Peter Anvin]

On March 14, 2025 12:06:04 PM PDT, David Laight <david.laight.linux@gmail.com> wrote:
>On Thu, 13 Mar 2025 14:09:24 -0700
>Jacob Keller <jacob.e.keller@intel.com> wrote:
>
>> On 3/13/2025 9:36 AM, H. Peter Anvin wrote:
>> > On March 13, 2025 9:24:38 AM PDT, Yury Norov <yury.norov@gmail.com> wrote:  
>> >> On Wed, Mar 12, 2025 at 05:09:16PM -0700, H. Peter Anvin wrote:  
>> >>> On March 12, 2025 4:56:31 PM PDT, Jacob Keller <jacob.e.keller@intel.com> wrote:  
>> >>
>> >> [...]
>> >>  
>> >>>> This is really a question of whether you expect odd or even parity as
>> >>>> the "true" value. I think that would depend on context, and we may not
>> >>>> reach a good consensus.
>> >>>>
>> >>>> I do agree that my brain would jump to "true is even, false is odd".
>> >>>> However, I also agree returning the value as 0 for even and 1 for odd
>> >>>> kind of made sense before, and updating this to be a bool and then
>> >>>> requiring to switch all the callers is a bit obnoxious...  
>> >>>
>> >>> Odd = 1 = true is the only same definition. It is a bitwise XOR, or sum mod 1.  
>> >>
>> >> The x86 implementation will be "popcnt(val) & 1", right? So if we
>> >> choose to go with odd == false, we'll have to add an extra negation.
>> >> So because it's a purely conventional thing, let's just pick a simpler
>> >> one?
>> >>
>> >> Compiler's builtin parity() returns 1 for odd.
>> >>
>> >> Thanks,
>> >> Yury  
>> > 
>> > The x86 implementation, no, but there will be plenty of others having that exact definition.  
>> 
>> Makes sense to stick with that existing convention then. Enough to
>> convince me.
>
>There is the possibility that the compiler will treat the builtin as having
>an 'int' result without the constraint of it being zero or one.
>In which case the conversion to bool will be a compare.
>This doesn't happen on x86-64 (gcc or clang) - but who knows elsewhere.
>
>For x86 popcnt(val) & 1 is best (except for parity8) but requires a non-archaic cpu.
>(Probably Nehelem or K10 or later - includes Sandy bridge and all the 'earth movers'.)
>Since performance isn't critical the generic C code is actually ok.
>(The 'parity' flag bit is only set on the low 8 bits.)
>
>	David
>
>

You seem confused. We have already established that the built-in didn't currently produce good code on some cpus, but it does on others, with very little in between, so it would make sense to use the builtins on an opt-in basis.

On x86 8- or 16-bit parity is best don't with test or xor respectively; 32- or 64-bit parity may use popcnt; test or by reducing down to a parity16 xor.

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Thu, Mar 13, 2025 at 03:41:49PM +0800, Kuan-Wei Chiu wrote:
> On Thu, Mar 13, 2025 at 12:29:13AM +0800, Kuan-Wei Chiu wrote:
> > On Wed, Mar 12, 2025 at 11:51:12AM -0400, Yury Norov wrote:
> > > On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
> > > > On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
> > > > >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> > > > >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> > > > >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> > > > >> > >On Fri, 07 Mar 2025 11:30:35 -0800
> > > > >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> > > > >> > >
> > > > >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > > > >> > >> >> (int)true most definitely is guaranteed to be 1.  
> > > > >> > >> >
> > > > >> > >> >That's not technically correct any more.
> > > > >> > >> >
> > > > >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> > > > >> > >> >other than 0 and 1.
> > > > >> > >> >
> > > > >> > >> >https://gcc.gnu.org/gcc-14/changes.html
> > > > >> > >> >
> > > > >> > >> >~Andrew  
> > > > >> > >> 
> > > > >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> > > > >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> > > > >> > >> or anything even remotely like it.
> > > > >> > >> 
> > > > >> > >
> > > > >> > >The whole idea of 'bool' is pretty much broken by design.
> > > > >> > >The underlying problem is that values other than 'true' and 'false' can
> > > > >> > >always get into 'bool' variables.
> > > > >> > >
> > > > >> > >Once that has happened it is all fubar.
> > > > >> > >
> > > > >> > >Trying to sanitise a value with (say):
> > > > >> > >int f(bool v)
> > > > >> > >{
> > > > >> > >	return (int)v & 1;
> > > > >> > >}    
> > > > >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> > > > >> > >
> > > > >> > >I really don't see how using (say) 0xaa and 0x55 helps.
> > > > >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> > > > >> > >from that.
> > > > >> > >
> > > > >> > >	David
> > > > >> > 
> > > > >> > Did you just discover GIGO?
> > > > >> 
> > > > >> Thanks for all the suggestions.
> > > > >> 
> > > > >> I don't have a strong opinion on the naming or return type. I'm still a
> > > > >> bit confused about whether I can assume that casting bool to int always
> > > > >> results in 0 or 1.
> > > > >> 
> > > > >> If that's the case, since most people prefer bool over int as the
> > > > >> return type and some are against introducing u1, my current plan is to
> > > > >> use the following in the next version:
> > > > >> 
> > > > >> bool parity_odd(u64 val);
> > > > >> 
> > > > >> This keeps the bool return type, renames the function for better
> > > > >> clarity, and avoids extra maintenance burden by having just one
> > > > >> function.
> > > > >> 
> > > > >> If I can't assume that casting bool to int always results in 0 or 1,
> > > > >> would it be acceptable to keep the return type as int?
> > > > >> 
> > > > >> Would this work for everyone?
> > > > >
> > > > >Alright, it's clearly a split opinion. So what I would do myself in
> > > > >such case is to look at existing code and see what people who really
> > > > >need parity invent in their drivers:
> > > > >
> > > > >                                     bool      parity_odd
> > > > >static inline int parity8(u8 val)       -               -
> > > > >static u8 calc_parity(u8 val)           -               -
> > > > >static int odd_parity(u8 c)             -               +
> > > > >static int saa711x_odd_parity           -               +
> > > > >static int max3100_do_parity            -               -
> > > > >static inline int parity(unsigned x)    -               -
> > > > >static int bit_parity(u32 pkt)          -               -
> > > > >static int oa_tc6_get_parity(u32 p)     -               -
> > > > >static u32 parity32(__le32 data)        -               -
> > > > >static u32 parity(u32 sample)           -               -
> > > > >static int get_parity(int number,       -               -
> > > > >                      int size)
> > > > >static bool i2cr_check_parity32(u32 v,  +               -
> > > > >                        bool parity)
> > > > >static bool i2cr_check_parity64(u64 v)  +               -
> > > > >static int sw_parity(__u64 t)           -               -
> > > > >static bool parity(u64 value)           +               -
> > > > >
> > > > >Now you can refer to that table say that int parity(uXX) is what
> > > > >people want to see in their drivers.
> > > > >
> > > > >Whichever interface you choose, please discuss it's pros and cons.
> > > > >What bloat-o-meter says for each option? What's maintenance burden?
> > > > >Perf test? Look at generated code?
> > > > >
> > > > >I personally for a macro returning boolean, something like I
> > > > >proposed at the very beginning.
> > > > >
> > > > >Thanks,
> > > > >Yury
> > > > 
> > > > Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.
> > > 
> > > Yeah. And because linux/bitops.h already includes asm/bitops.h
> > > the simplest way would be wrapping generic implementation with
> > > the #ifndef parity, similarly to how we handle find_next_bit case.
> > > 
> > > So:
> > > 1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
> > > 2. This may, and probably should, be a separate follow-up series,
> > >    likely created by corresponding arch experts.
> > > 
> > I saw discussions in the previous email thread about both
> > __builtin_parity and x86-specific implementations. However, from the
> > discussion, I learned that before considering any optimization, we
> > should first ask: which driver or subsystem actually cares about parity
> > efficiency? If someone does, I can help with a micro-benchmark to
> > provide performance numbers, but I don't have enough domain knowledge
> > to identify hot paths where parity efficiency matters.
> > 
> IMHO,
> 
> If parity is never used in any hot path and we don't care about parity:
> 
> Then benchmarking its performance seems meaningless. In this case, a
> function with a u64 argument would suffice, and we might not even need
> a macro to optimize for different types—especially since the macro
> requires special hacks to avoid compiler warnings. Also, I don't think
> code size matters here. If it does, we should first consider making
> parity a non-inline function in a .c file rather than an inline
> function/macro in a header.
> 
> If parity is used in a hot path:
> 
> We need different handling for different type sizes. As previously
> discussed, x86 assembly might use different instructions for u8 and
> u16. This may sound stubborn, but I want to ask again: should we
> consider using parity8/16/32/64 interfaces? Like in the i3c driver
> example, if we only have a single parity macro that selects an
> implementation based on type size, users must explicitly cast types.
> If future users also need parity in a hot path, they might not be aware
> of this requirement and end up generating suboptimal code. Since we
> care about efficiency and generated code, why not follow hweight() and
> provide separate implementations for different sizes?
> 
It seems no one will reply to my two emails. So, I have summarized
different interface approaches. If there is a next version, I will send
it after the merge window closes.

Interface 1: Single Function
Description: bool parity_odd(u64)
Pros: Minimal maintenance cost
Cons: Difficult to integrate with architecture-specific implementations
      due to the inability to optimize for different argument sizes
Opinions: Jiri supports this approach

Interface 2: Single Macro
Description: parity_odd() macro
Pros: Allows type-specific implementation
Cons: Requires hacks to avoid warnings; users may need explicit
      casting; potential sub-optimal code on 32-bit x86
Opinions: Yury supports this approach

Interface 3: Multiple Functions
Description: bool parity_odd8/16/32/64()
Pros: No need for explicit casting; easy to integrate
      architecture-specific optimizations; except for parity8(), all
      functions are one-liners with no significant code duplication
Cons: More functions may increase maintenance burden
Opinions: Only I support this approach

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On March 23, 2025 8:16:24 AM PDT, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
>On Thu, Mar 13, 2025 at 03:41:49PM +0800, Kuan-Wei Chiu wrote:
>> On Thu, Mar 13, 2025 at 12:29:13AM +0800, Kuan-Wei Chiu wrote:
>> > On Wed, Mar 12, 2025 at 11:51:12AM -0400, Yury Norov wrote:
>> > > On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
>> > > > On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
>> > > > >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
>> > > > >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
>> > > > >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
>> > > > >> > >On Fri, 07 Mar 2025 11:30:35 -0800
>> > > > >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
>> > > > >> > >
>> > > > >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> > > > >> > >> >> (int)true most definitely is guaranteed to be 1.  
>> > > > >> > >> >
>> > > > >> > >> >That's not technically correct any more.
>> > > > >> > >> >
>> > > > >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
>> > > > >> > >> >other than 0 and 1.
>> > > > >> > >> >
>> > > > >> > >> >https://gcc.gnu.org/gcc-14/changes.html
>> > > > >> > >> >
>> > > > >> > >> >~Andrew  
>> > > > >> > >> 
>> > > > >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
>> > > > >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
>> > > > >> > >> or anything even remotely like it.
>> > > > >> > >> 
>> > > > >> > >
>> > > > >> > >The whole idea of 'bool' is pretty much broken by design.
>> > > > >> > >The underlying problem is that values other than 'true' and 'false' can
>> > > > >> > >always get into 'bool' variables.
>> > > > >> > >
>> > > > >> > >Once that has happened it is all fubar.
>> > > > >> > >
>> > > > >> > >Trying to sanitise a value with (say):
>> > > > >> > >int f(bool v)
>> > > > >> > >{
>> > > > >> > >	return (int)v & 1;
>> > > > >> > >}    
>> > > > >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
>> > > > >> > >
>> > > > >> > >I really don't see how using (say) 0xaa and 0x55 helps.
>> > > > >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
>> > > > >> > >from that.
>> > > > >> > >
>> > > > >> > >	David
>> > > > >> > 
>> > > > >> > Did you just discover GIGO?
>> > > > >> 
>> > > > >> Thanks for all the suggestions.
>> > > > >> 
>> > > > >> I don't have a strong opinion on the naming or return type. I'm still a
>> > > > >> bit confused about whether I can assume that casting bool to int always
>> > > > >> results in 0 or 1.
>> > > > >> 
>> > > > >> If that's the case, since most people prefer bool over int as the
>> > > > >> return type and some are against introducing u1, my current plan is to
>> > > > >> use the following in the next version:
>> > > > >> 
>> > > > >> bool parity_odd(u64 val);
>> > > > >> 
>> > > > >> This keeps the bool return type, renames the function for better
>> > > > >> clarity, and avoids extra maintenance burden by having just one
>> > > > >> function.
>> > > > >> 
>> > > > >> If I can't assume that casting bool to int always results in 0 or 1,
>> > > > >> would it be acceptable to keep the return type as int?
>> > > > >> 
>> > > > >> Would this work for everyone?
>> > > > >
>> > > > >Alright, it's clearly a split opinion. So what I would do myself in
>> > > > >such case is to look at existing code and see what people who really
>> > > > >need parity invent in their drivers:
>> > > > >
>> > > > >                                     bool      parity_odd
>> > > > >static inline int parity8(u8 val)       -               -
>> > > > >static u8 calc_parity(u8 val)           -               -
>> > > > >static int odd_parity(u8 c)             -               +
>> > > > >static int saa711x_odd_parity           -               +
>> > > > >static int max3100_do_parity            -               -
>> > > > >static inline int parity(unsigned x)    -               -
>> > > > >static int bit_parity(u32 pkt)          -               -
>> > > > >static int oa_tc6_get_parity(u32 p)     -               -
>> > > > >static u32 parity32(__le32 data)        -               -
>> > > > >static u32 parity(u32 sample)           -               -
>> > > > >static int get_parity(int number,       -               -
>> > > > >                      int size)
>> > > > >static bool i2cr_check_parity32(u32 v,  +               -
>> > > > >                        bool parity)
>> > > > >static bool i2cr_check_parity64(u64 v)  +               -
>> > > > >static int sw_parity(__u64 t)           -               -
>> > > > >static bool parity(u64 value)           +               -
>> > > > >
>> > > > >Now you can refer to that table say that int parity(uXX) is what
>> > > > >people want to see in their drivers.
>> > > > >
>> > > > >Whichever interface you choose, please discuss it's pros and cons.
>> > > > >What bloat-o-meter says for each option? What's maintenance burden?
>> > > > >Perf test? Look at generated code?
>> > > > >
>> > > > >I personally for a macro returning boolean, something like I
>> > > > >proposed at the very beginning.
>> > > > >
>> > > > >Thanks,
>> > > > >Yury
>> > > > 
>> > > > Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.
>> > > 
>> > > Yeah. And because linux/bitops.h already includes asm/bitops.h
>> > > the simplest way would be wrapping generic implementation with
>> > > the #ifndef parity, similarly to how we handle find_next_bit case.
>> > > 
>> > > So:
>> > > 1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
>> > > 2. This may, and probably should, be a separate follow-up series,
>> > >    likely created by corresponding arch experts.
>> > > 
>> > I saw discussions in the previous email thread about both
>> > __builtin_parity and x86-specific implementations. However, from the
>> > discussion, I learned that before considering any optimization, we
>> > should first ask: which driver or subsystem actually cares about parity
>> > efficiency? If someone does, I can help with a micro-benchmark to
>> > provide performance numbers, but I don't have enough domain knowledge
>> > to identify hot paths where parity efficiency matters.
>> > 
>> IMHO,
>> 
>> If parity is never used in any hot path and we don't care about parity:
>> 
>> Then benchmarking its performance seems meaningless. In this case, a
>> function with a u64 argument would suffice, and we might not even need
>> a macro to optimize for different types—especially since the macro
>> requires special hacks to avoid compiler warnings. Also, I don't think
>> code size matters here. If it does, we should first consider making
>> parity a non-inline function in a .c file rather than an inline
>> function/macro in a header.
>> 
>> If parity is used in a hot path:
>> 
>> We need different handling for different type sizes. As previously
>> discussed, x86 assembly might use different instructions for u8 and
>> u16. This may sound stubborn, but I want to ask again: should we
>> consider using parity8/16/32/64 interfaces? Like in the i3c driver
>> example, if we only have a single parity macro that selects an
>> implementation based on type size, users must explicitly cast types.
>> If future users also need parity in a hot path, they might not be aware
>> of this requirement and end up generating suboptimal code. Since we
>> care about efficiency and generated code, why not follow hweight() and
>> provide separate implementations for different sizes?
>> 
>It seems no one will reply to my two emails. So, I have summarized
>different interface approaches. If there is a next version, I will send
>it after the merge window closes.
>
>Interface 1: Single Function
>Description: bool parity_odd(u64)
>Pros: Minimal maintenance cost
>Cons: Difficult to integrate with architecture-specific implementations
>      due to the inability to optimize for different argument sizes
>Opinions: Jiri supports this approach
>
>Interface 2: Single Macro
>Description: parity_odd() macro
>Pros: Allows type-specific implementation
>Cons: Requires hacks to avoid warnings; users may need explicit
>      casting; potential sub-optimal code on 32-bit x86
>Opinions: Yury supports this approach
>
>Interface 3: Multiple Functions
>Description: bool parity_odd8/16/32/64()
>Pros: No need for explicit casting; easy to integrate
>      architecture-specific optimizations; except for parity8(), all
>      functions are one-liners with no significant code duplication
>Cons: More functions may increase maintenance burden
>Opinions: Only I support this approach
>
>Regards,
>Kuan-Wei

You can add me to the final option. I think it makes most sense

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Yury Norov]

On Sun, Mar 23, 2025 at 03:40:20PM -0700, H. Peter Anvin wrote:
> On March 23, 2025 8:16:24 AM PDT, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> >On Thu, Mar 13, 2025 at 03:41:49PM +0800, Kuan-Wei Chiu wrote:
> >> On Thu, Mar 13, 2025 at 12:29:13AM +0800, Kuan-Wei Chiu wrote:
> >> > On Wed, Mar 12, 2025 at 11:51:12AM -0400, Yury Norov wrote:
> >> > > On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
> >> > > > On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
> >> > > > >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> >> > > > >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> >> > > > >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> >> > > > >> > >On Fri, 07 Mar 2025 11:30:35 -0800
> >> > > > >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> >> > > > >> > >
> >> > > > >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> > > > >> > >> >> (int)true most definitely is guaranteed to be 1.  
> >> > > > >> > >> >
> >> > > > >> > >> >That's not technically correct any more.
> >> > > > >> > >> >
> >> > > > >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> >> > > > >> > >> >other than 0 and 1.
> >> > > > >> > >> >
> >> > > > >> > >> >https://gcc.gnu.org/gcc-14/changes.html
> >> > > > >> > >> >
> >> > > > >> > >> >~Andrew  
> >> > > > >> > >> 
> >> > > > >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> >> > > > >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> >> > > > >> > >> or anything even remotely like it.
> >> > > > >> > >> 
> >> > > > >> > >
> >> > > > >> > >The whole idea of 'bool' is pretty much broken by design.
> >> > > > >> > >The underlying problem is that values other than 'true' and 'false' can
> >> > > > >> > >always get into 'bool' variables.
> >> > > > >> > >
> >> > > > >> > >Once that has happened it is all fubar.
> >> > > > >> > >
> >> > > > >> > >Trying to sanitise a value with (say):
> >> > > > >> > >int f(bool v)
> >> > > > >> > >{
> >> > > > >> > >	return (int)v & 1;
> >> > > > >> > >}    
> >> > > > >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> >> > > > >> > >
> >> > > > >> > >I really don't see how using (say) 0xaa and 0x55 helps.
> >> > > > >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> >> > > > >> > >from that.
> >> > > > >> > >
> >> > > > >> > >	David
> >> > > > >> > 
> >> > > > >> > Did you just discover GIGO?
> >> > > > >> 
> >> > > > >> Thanks for all the suggestions.
> >> > > > >> 
> >> > > > >> I don't have a strong opinion on the naming or return type. I'm still a
> >> > > > >> bit confused about whether I can assume that casting bool to int always
> >> > > > >> results in 0 or 1.
> >> > > > >> 
> >> > > > >> If that's the case, since most people prefer bool over int as the
> >> > > > >> return type and some are against introducing u1, my current plan is to
> >> > > > >> use the following in the next version:
> >> > > > >> 
> >> > > > >> bool parity_odd(u64 val);
> >> > > > >> 
> >> > > > >> This keeps the bool return type, renames the function for better
> >> > > > >> clarity, and avoids extra maintenance burden by having just one
> >> > > > >> function.
> >> > > > >> 
> >> > > > >> If I can't assume that casting bool to int always results in 0 or 1,
> >> > > > >> would it be acceptable to keep the return type as int?
> >> > > > >> 
> >> > > > >> Would this work for everyone?
> >> > > > >
> >> > > > >Alright, it's clearly a split opinion. So what I would do myself in
> >> > > > >such case is to look at existing code and see what people who really
> >> > > > >need parity invent in their drivers:
> >> > > > >
> >> > > > >                                     bool      parity_odd
> >> > > > >static inline int parity8(u8 val)       -               -
> >> > > > >static u8 calc_parity(u8 val)           -               -
> >> > > > >static int odd_parity(u8 c)             -               +
> >> > > > >static int saa711x_odd_parity           -               +
> >> > > > >static int max3100_do_parity            -               -
> >> > > > >static inline int parity(unsigned x)    -               -
> >> > > > >static int bit_parity(u32 pkt)          -               -
> >> > > > >static int oa_tc6_get_parity(u32 p)     -               -
> >> > > > >static u32 parity32(__le32 data)        -               -
> >> > > > >static u32 parity(u32 sample)           -               -
> >> > > > >static int get_parity(int number,       -               -
> >> > > > >                      int size)
> >> > > > >static bool i2cr_check_parity32(u32 v,  +               -
> >> > > > >                        bool parity)
> >> > > > >static bool i2cr_check_parity64(u64 v)  +               -
> >> > > > >static int sw_parity(__u64 t)           -               -
> >> > > > >static bool parity(u64 value)           +               -
> >> > > > >
> >> > > > >Now you can refer to that table say that int parity(uXX) is what
> >> > > > >people want to see in their drivers.
> >> > > > >
> >> > > > >Whichever interface you choose, please discuss it's pros and cons.
> >> > > > >What bloat-o-meter says for each option? What's maintenance burden?
> >> > > > >Perf test? Look at generated code?
> >> > > > >
> >> > > > >I personally for a macro returning boolean, something like I
> >> > > > >proposed at the very beginning.
> >> > > > >
> >> > > > >Thanks,
> >> > > > >Yury
> >> > > > 
> >> > > > Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.
> >> > > 
> >> > > Yeah. And because linux/bitops.h already includes asm/bitops.h
> >> > > the simplest way would be wrapping generic implementation with
> >> > > the #ifndef parity, similarly to how we handle find_next_bit case.
> >> > > 
> >> > > So:
> >> > > 1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
> >> > > 2. This may, and probably should, be a separate follow-up series,
> >> > >    likely created by corresponding arch experts.
> >> > > 
> >> > I saw discussions in the previous email thread about both
> >> > __builtin_parity and x86-specific implementations. However, from the
> >> > discussion, I learned that before considering any optimization, we
> >> > should first ask: which driver or subsystem actually cares about parity
> >> > efficiency? If someone does, I can help with a micro-benchmark to
> >> > provide performance numbers, but I don't have enough domain knowledge
> >> > to identify hot paths where parity efficiency matters.
> >> > 
> >> IMHO,
> >> 
> >> If parity is never used in any hot path and we don't care about parity:
> >> 
> >> Then benchmarking its performance seems meaningless. In this case, a
> >> function with a u64 argument would suffice, and we might not even need
> >> a macro to optimize for different types—especially since the macro
> >> requires special hacks to avoid compiler warnings. Also, I don't think
> >> code size matters here. If it does, we should first consider making
> >> parity a non-inline function in a .c file rather than an inline
> >> function/macro in a header.
> >> 
> >> If parity is used in a hot path:
> >> 
> >> We need different handling for different type sizes. As previously
> >> discussed, x86 assembly might use different instructions for u8 and
> >> u16. This may sound stubborn, but I want to ask again: should we
> >> consider using parity8/16/32/64 interfaces? Like in the i3c driver
> >> example, if we only have a single parity macro that selects an
> >> implementation based on type size, users must explicitly cast types.
> >> If future users also need parity in a hot path, they might not be aware
> >> of this requirement and end up generating suboptimal code. Since we
> >> care about efficiency and generated code, why not follow hweight() and
> >> provide separate implementations for different sizes?
> >> 
> >It seems no one will reply to my two emails. So, I have summarized
> >different interface approaches. If there is a next version, I will send
> >it after the merge window closes.
> >
> >Interface 1: Single Function
> >Description: bool parity_odd(u64)
> >Pros: Minimal maintenance cost
> >Cons: Difficult to integrate with architecture-specific implementations
> >      due to the inability to optimize for different argument sizes

How difficult? It's just as simple as find_next_bit(). I already
pointed that.

> >Opinions: Jiri supports this approach
> >
> >Interface 2: Single Macro
> >Description: parity_odd() macro
> >Pros: Allows type-specific implementation
> >Cons: Requires hacks to avoid warnings; users may need explicit

So if the hack is documented, it's OK. I don't know the other way to
motivate compilers to get better other than pointing them to their
bugs.

> >      casting; potential sub-optimal code on 32-bit x86

Any asm listings? Any real impact?

> >Opinions: Yury supports this approach
> >
> >Interface 3: Multiple Functions
> >Description: bool parity_odd8/16/32/64()
> >Pros: No need for explicit casting; easy to integrate

Explicit castings are sometimes better than implicit ones.

> >      architecture-specific optimizations; except for parity8(), all
> >      functions are one-liners with no significant code duplication
> >Cons: More functions may increase maintenance burden

s/may/will/

> >Opinions: Only I support this approach
> >
> >Regards,
> >Kuan-Wei
> 
> You can add me to the final option. I think it makes most sense

This is not a democracy, and we are not voting here. We are engineers.
We share our expert opinions and choose the best one. I'll be happy to
go with any solution, if we all make sure it's the best.

I'm for #2 because it 
 - generates better code than #1;
 - easier to use than #3; and
 - has less maintenance burden than #3.

Why exactly #3 makes the most sense to you? Most variables are ints
and longs. How are you going to handle those with fixed-types parity()s?

Thanks,
Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[H. Peter Anvin]

On 3/23/25 08:16, Kuan-Wei Chiu wrote:
> 
> Interface 3: Multiple Functions
> Description: bool parity_odd8/16/32/64()
> Pros: No need for explicit casting; easy to integrate
>        architecture-specific optimizations; except for parity8(), all
>        functions are one-liners with no significant code duplication
> Cons: More functions may increase maintenance burden
> Opinions: Only I support this approach
> 

OK, so I responded to this but I can't find my reply or any of the 
followups, so let me go again:

I prefer this option, because:

a. Virtually all uses of parity is done in contexts where the sizes of 
the items for which parity is to be taken are well-defined, but it is 
*really* easy for integer promotion to cause a value to be extended to 
32 bits unnecessarily (sign or zero extend, although for parity it 
doesn't make any difference -- if the compiler realizes it.)

b. It makes it easier to add arch-specific implementations, notably 
using __builtin_parity on architectures where that is known to generate 
good code.

c. For architectures where only *some* parity implementations are 
fast/practical, the generic fallbacks will either naturally synthesize 
them from components via shift-xor, or they can be defined to use a 
larger version; the function prototype acts like a cast.

d. If there is a reason in the future to add a generic version, it is 
really easy to do using the size-specific functions as components; this 
is something we do literally all over the place, using a pattern so 
common that it, itself, probably should be macroized:

#define parity(x) 				\
({						\
	typeof(x) __x = (x);			\
	bool __y;				\
	switch (sizeof(__x)) {			\
		case 1:				\
			__y = parity8(__x);	\
			break;			\
		case 2:				\
			__y = parity16(__x);	\
			break;			\
		case 4:				\
			__y = parity32(__x);	\
			break;			\
		case 8:				\
			__y = parity64(__x);	\
			break;			\
		default:			\
			BUILD_BUG();		\
			break;			\
	}					\
	__y;					\
})
				

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Mon, Mar 24, 2025 at 11:53:35AM -0400, Yury Norov wrote:
> On Sun, Mar 23, 2025 at 03:40:20PM -0700, H. Peter Anvin wrote:
> > On March 23, 2025 8:16:24 AM PDT, Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> > >On Thu, Mar 13, 2025 at 03:41:49PM +0800, Kuan-Wei Chiu wrote:
> > >> On Thu, Mar 13, 2025 at 12:29:13AM +0800, Kuan-Wei Chiu wrote:
> > >> > On Wed, Mar 12, 2025 at 11:51:12AM -0400, Yury Norov wrote:
> > >> > > On Tue, Mar 11, 2025 at 03:24:14PM -0700, H. Peter Anvin wrote:
> > >> > > > On March 11, 2025 3:01:30 PM PDT, Yury Norov <yury.norov@gmail.com> wrote:
> > >> > > > >On Sun, Mar 09, 2025 at 11:48:26PM +0800, Kuan-Wei Chiu wrote:
> > >> > > > >> On Fri, Mar 07, 2025 at 12:07:02PM -0800, H. Peter Anvin wrote:
> > >> > > > >> > On March 7, 2025 11:53:10 AM PST, David Laight <david.laight.linux@gmail.com> wrote:
> > >> > > > >> > >On Fri, 07 Mar 2025 11:30:35 -0800
> > >> > > > >> > >"H. Peter Anvin" <hpa@zytor.com> wrote:
> > >> > > > >> > >
> > >> > > > >> > >> On March 7, 2025 10:49:56 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > >> > > > >> > >> >> (int)true most definitely is guaranteed to be 1.  
> > >> > > > >> > >> >
> > >> > > > >> > >> >That's not technically correct any more.
> > >> > > > >> > >> >
> > >> > > > >> > >> >GCC has introduced hardened bools that intentionally have bit patterns
> > >> > > > >> > >> >other than 0 and 1.
> > >> > > > >> > >> >
> > >> > > > >> > >> >https://gcc.gnu.org/gcc-14/changes.html
> > >> > > > >> > >> >
> > >> > > > >> > >> >~Andrew  
> > >> > > > >> > >> 
> > >> > > > >> > >> Bit patterns in memory maybe (not that I can see the Linux kernel using them) but
> > >> > > > >> > >> for compiler-generated conversations that's still a given, or the manager isn't C
> > >> > > > >> > >> or anything even remotely like it.
> > >> > > > >> > >> 
> > >> > > > >> > >
> > >> > > > >> > >The whole idea of 'bool' is pretty much broken by design.
> > >> > > > >> > >The underlying problem is that values other than 'true' and 'false' can
> > >> > > > >> > >always get into 'bool' variables.
> > >> > > > >> > >
> > >> > > > >> > >Once that has happened it is all fubar.
> > >> > > > >> > >
> > >> > > > >> > >Trying to sanitise a value with (say):
> > >> > > > >> > >int f(bool v)
> > >> > > > >> > >{
> > >> > > > >> > >	return (int)v & 1;
> > >> > > > >> > >}    
> > >> > > > >> > >just doesn't work (see https://www.godbolt.org/z/MEndP3q9j)
> > >> > > > >> > >
> > >> > > > >> > >I really don't see how using (say) 0xaa and 0x55 helps.
> > >> > > > >> > >What happens if the value is wrong? a trap or exception?, good luck recovering
> > >> > > > >> > >from that.
> > >> > > > >> > >
> > >> > > > >> > >	David
> > >> > > > >> > 
> > >> > > > >> > Did you just discover GIGO?
> > >> > > > >> 
> > >> > > > >> Thanks for all the suggestions.
> > >> > > > >> 
> > >> > > > >> I don't have a strong opinion on the naming or return type. I'm still a
> > >> > > > >> bit confused about whether I can assume that casting bool to int always
> > >> > > > >> results in 0 or 1.
> > >> > > > >> 
> > >> > > > >> If that's the case, since most people prefer bool over int as the
> > >> > > > >> return type and some are against introducing u1, my current plan is to
> > >> > > > >> use the following in the next version:
> > >> > > > >> 
> > >> > > > >> bool parity_odd(u64 val);
> > >> > > > >> 
> > >> > > > >> This keeps the bool return type, renames the function for better
> > >> > > > >> clarity, and avoids extra maintenance burden by having just one
> > >> > > > >> function.
> > >> > > > >> 
> > >> > > > >> If I can't assume that casting bool to int always results in 0 or 1,
> > >> > > > >> would it be acceptable to keep the return type as int?
> > >> > > > >> 
> > >> > > > >> Would this work for everyone?
> > >> > > > >
> > >> > > > >Alright, it's clearly a split opinion. So what I would do myself in
> > >> > > > >such case is to look at existing code and see what people who really
> > >> > > > >need parity invent in their drivers:
> > >> > > > >
> > >> > > > >                                     bool      parity_odd
> > >> > > > >static inline int parity8(u8 val)       -               -
> > >> > > > >static u8 calc_parity(u8 val)           -               -
> > >> > > > >static int odd_parity(u8 c)             -               +
> > >> > > > >static int saa711x_odd_parity           -               +
> > >> > > > >static int max3100_do_parity            -               -
> > >> > > > >static inline int parity(unsigned x)    -               -
> > >> > > > >static int bit_parity(u32 pkt)          -               -
> > >> > > > >static int oa_tc6_get_parity(u32 p)     -               -
> > >> > > > >static u32 parity32(__le32 data)        -               -
> > >> > > > >static u32 parity(u32 sample)           -               -
> > >> > > > >static int get_parity(int number,       -               -
> > >> > > > >                      int size)
> > >> > > > >static bool i2cr_check_parity32(u32 v,  +               -
> > >> > > > >                        bool parity)
> > >> > > > >static bool i2cr_check_parity64(u64 v)  +               -
> > >> > > > >static int sw_parity(__u64 t)           -               -
> > >> > > > >static bool parity(u64 value)           +               -
> > >> > > > >
> > >> > > > >Now you can refer to that table say that int parity(uXX) is what
> > >> > > > >people want to see in their drivers.
> > >> > > > >
> > >> > > > >Whichever interface you choose, please discuss it's pros and cons.
> > >> > > > >What bloat-o-meter says for each option? What's maintenance burden?
> > >> > > > >Perf test? Look at generated code?
> > >> > > > >
> > >> > > > >I personally for a macro returning boolean, something like I
> > >> > > > >proposed at the very beginning.
> > >> > > > >
> > >> > > > >Thanks,
> > >> > > > >Yury
> > >> > > > 
> > >> > > > Also, please at least provide a way for an arch to opt in to using the builtins, which seem to produce as good results or better at least on some architectures like x86 and probably with CPU options that imply fast popcnt is available.
> > >> > > 
> > >> > > Yeah. And because linux/bitops.h already includes asm/bitops.h
> > >> > > the simplest way would be wrapping generic implementation with
> > >> > > the #ifndef parity, similarly to how we handle find_next_bit case.
> > >> > > 
> > >> > > So:
> > >> > > 1. Kuan-Wei, please don't invent something like ARCH_HAS_PARITY;
> > >> > > 2. This may, and probably should, be a separate follow-up series,
> > >> > >    likely created by corresponding arch experts.
> > >> > > 
> > >> > I saw discussions in the previous email thread about both
> > >> > __builtin_parity and x86-specific implementations. However, from the
> > >> > discussion, I learned that before considering any optimization, we
> > >> > should first ask: which driver or subsystem actually cares about parity
> > >> > efficiency? If someone does, I can help with a micro-benchmark to
> > >> > provide performance numbers, but I don't have enough domain knowledge
> > >> > to identify hot paths where parity efficiency matters.
> > >> > 
> > >> IMHO,
> > >> 
> > >> If parity is never used in any hot path and we don't care about parity:
> > >> 
> > >> Then benchmarking its performance seems meaningless. In this case, a
> > >> function with a u64 argument would suffice, and we might not even need
> > >> a macro to optimize for different types—especially since the macro
> > >> requires special hacks to avoid compiler warnings. Also, I don't think
> > >> code size matters here. If it does, we should first consider making
> > >> parity a non-inline function in a .c file rather than an inline
> > >> function/macro in a header.
> > >> 
> > >> If parity is used in a hot path:
> > >> 
> > >> We need different handling for different type sizes. As previously
> > >> discussed, x86 assembly might use different instructions for u8 and
> > >> u16. This may sound stubborn, but I want to ask again: should we
> > >> consider using parity8/16/32/64 interfaces? Like in the i3c driver
> > >> example, if we only have a single parity macro that selects an
> > >> implementation based on type size, users must explicitly cast types.
> > >> If future users also need parity in a hot path, they might not be aware
> > >> of this requirement and end up generating suboptimal code. Since we
> > >> care about efficiency and generated code, why not follow hweight() and
> > >> provide separate implementations for different sizes?
> > >> 
> > >It seems no one will reply to my two emails. So, I have summarized
> > >different interface approaches. If there is a next version, I will send
> > >it after the merge window closes.
> > >
> > >Interface 1: Single Function
> > >Description: bool parity_odd(u64)
> > >Pros: Minimal maintenance cost
> > >Cons: Difficult to integrate with architecture-specific implementations
> > >      due to the inability to optimize for different argument sizes
> 
> How difficult? It's just as simple as find_next_bit(). I already
> pointed that.
> 

The architecture-specific implementation may use different approaches
for different bit widths. As previously discussed by Peter and David,
the x86 implementation of parity uses different instructions for u8 and
u16 arguments. Having an interface that only takes u64 makes it
difficult to accommodate these differences.

Are you referring to the #ifndef parity approach when mentioning
find_next_bit()? If so, I'm not sure how that would solve the issue
while keeping different implementations for different bit widths under
an interface that only takes u64.

> > >Opinions: Jiri supports this approach
> > >
> > >Interface 2: Single Macro
> > >Description: parity_odd() macro
> > >Pros: Allows type-specific implementation
> > >Cons: Requires hacks to avoid warnings; users may need explicit
> 
> So if the hack is documented, it's OK. I don't know the other way to
> motivate compilers to get better other than pointing them to their
> bugs.
> 

Perhaps this comes down to taste. IMHO, a solution relying on #if gcc
#else with extensive comments to explain workarounds has a higher
maintenance cost and is uglier compared to simply having three
additional one-liner functions.

That said, you are the maintainer, and you likely have a better
perspective on the maintenance burden than I do.

> > >      casting; potential sub-optimal code on 32-bit x86
> 
> Any asm listings? Any real impact?
> 
I was just referring to the comment previously mentioned by David.
Below is a simple assembly comparison between #2 and #3:

generic_macro:
        movq    xmm0, QWORD PTR [esp+4]
        mov     edx, 27030
        movdqa  xmm2, xmm0
        psrlq   xmm2, 32
        movdqa  xmm1, xmm2
        pxor    xmm1, xmm0
        movdqa  xmm3, xmm1
        psrlq   xmm3, 16
        movdqa  xmm0, xmm3
        pxor    xmm0, xmm1
        movdqa  xmm4, xmm0
        psrlq   xmm4, 8
        movdqa  xmm1, xmm4
        pxor    xmm1, xmm0
        movdqa  xmm5, xmm1
        psrlq   xmm5, 4
        movdqa  xmm0, xmm5
        pxor    xmm0, xmm1
        movd    eax, xmm0
        and     eax, 15
        bt      edx, eax
        setc    al
        ret
fixed_type_parity_function:
        mov     edx, DWORD PTR [esp+8]
        xor     edx, DWORD PTR [esp+4]
        mov     eax, edx
        shr     eax, 16
        xor     eax, edx
        mov     edx, eax
        xor     dl, ah
        mov     eax, edx
        shr     al, 4
        xor     eax, edx
        mov     edx, 27030
        and     eax, 15
        bt      edx, eax
        setc    al
        ret

https://godbolt.org/z/cPjbEYKPP

As I mentioned earlier, I can't point to any specific driver or
subsystem where parity efficiency is critical. However, if performance
is not a concern, then perhaps #1 is the simplest option, and we might
not even need architecture-specific implementations.

> > >Opinions: Yury supports this approach
> > >
> > >Interface 3: Multiple Functions
> > >Description: bool parity_odd8/16/32/64()
> > >Pros: No need for explicit casting; easy to integrate
> 
> Explicit castings are sometimes better than implicit ones.
> 
In #2, users might easily overlook the need for explicit casting. As
Peter pointed out, integer promotion could unintentionally lead to the
32-bit version being used when it wasn't intended.

By making the function names and parameter types explicitly indicate
8/16/32/64 bits, such issues become less likely to occur.

> > >      architecture-specific optimizations; except for parity8(), all
> > >      functions are one-liners with no significant code duplication
> > >Cons: More functions may increase maintenance burden
> 
> s/may/will/
> 
> > >Opinions: Only I support this approach
> > >
> > >Regards,
> > >Kuan-Wei
> > 
> > You can add me to the final option. I think it makes most sense
> 
> This is not a democracy, and we are not voting here. We are engineers.
> We share our expert opinions and choose the best one. I'll be happy to
> go with any solution, if we all make sure it's the best.
> 
I wasn't trying to initiate a vote. I was simply summarizing the
possible approaches discussed so far and the feedback I've received.

> I'm for #2 because it 
>  - generates better code than #1;

Which driver or subsystem would particularly care about parity
efficiency?

>  - easier to use than #3; and

I agree that #2 is easier to use than #3 if users don't need to perform
explicit casting. However, if they do need to cast explicitly to get
correct code generation, then I think #3 is clearer and simpler for
users.

>  - has less maintenance burden than #3.
> 
> Why exactly #3 makes the most sense to you? Most variables are ints
> and longs. How are you going to handle those with fixed-types parity()s?
> 
If explicit casting is required for correct code generation, then both
#2 and #3 face the same issue.

To address this, my first thought was to add a parity_long() function,
similar to hweight_long(). However, I can imagine that you'd find this
even harder to accept due to increased maintenance burden.

This also brings me to another question that isn't directly related to
parity but has me curious. Since you maintain hweight as well, what
makes hweight and parity different enough that hweight follows the
hweight8/16/32/64 and hweight_long() interface, while parity is leaning
toward a macro-based approach? Their functionality seems quite similar
to me.

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Tue, Mar 25, 2025 at 12:43:25PM -0700, H. Peter Anvin wrote:
> On 3/23/25 08:16, Kuan-Wei Chiu wrote:
> > 
> > Interface 3: Multiple Functions
> > Description: bool parity_odd8/16/32/64()
> > Pros: No need for explicit casting; easy to integrate
> >        architecture-specific optimizations; except for parity8(), all
> >        functions are one-liners with no significant code duplication
> > Cons: More functions may increase maintenance burden
> > Opinions: Only I support this approach
> > 
> 
> OK, so I responded to this but I can't find my reply or any of the
> followups, so let me go again:
> 
> I prefer this option, because:
> 
> a. Virtually all uses of parity is done in contexts where the sizes of the
> items for which parity is to be taken are well-defined, but it is *really*
> easy for integer promotion to cause a value to be extended to 32 bits
> unnecessarily (sign or zero extend, although for parity it doesn't make any
> difference -- if the compiler realizes it.)
> 
> b. It makes it easier to add arch-specific implementations, notably using
> __builtin_parity on architectures where that is known to generate good code.
> 
> c. For architectures where only *some* parity implementations are
> fast/practical, the generic fallbacks will either naturally synthesize them
> from components via shift-xor, or they can be defined to use a larger
> version; the function prototype acts like a cast.
> 
> d. If there is a reason in the future to add a generic version, it is really
> easy to do using the size-specific functions as components; this is
> something we do literally all over the place, using a pattern so common that
> it, itself, probably should be macroized:
> 
> #define parity(x) 				\
> ({						\
> 	typeof(x) __x = (x);			\
> 	bool __y;				\
> 	switch (sizeof(__x)) {			\
> 		case 1:				\
> 			__y = parity8(__x);	\
> 			break;			\
> 		case 2:				\
> 			__y = parity16(__x);	\
> 			break;			\
> 		case 4:				\
> 			__y = parity32(__x);	\
> 			break;			\
> 		case 8:				\
> 			__y = parity64(__x);	\
> 			break;			\
> 		default:			\
> 			BUILD_BUG();		\
> 			break;			\
> 	}					\
> 	__y;					\
> })
>
Thank you for your detailed response and for explaining the rationale
behind your preference. The points you outlined in (a)–(d) all seem
quite reasonable to me.

Yury,
do you have any feedback on this?
Thank you.

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Yury Norov]

On Thu, Apr 03, 2025 at 10:39:03PM +0800, Kuan-Wei Chiu wrote:
> On Tue, Mar 25, 2025 at 12:43:25PM -0700, H. Peter Anvin wrote:
> > On 3/23/25 08:16, Kuan-Wei Chiu wrote:
> > > 
> > > Interface 3: Multiple Functions
> > > Description: bool parity_odd8/16/32/64()
> > > Pros: No need for explicit casting; easy to integrate
> > >        architecture-specific optimizations; except for parity8(), all
> > >        functions are one-liners with no significant code duplication
> > > Cons: More functions may increase maintenance burden
> > > Opinions: Only I support this approach
> > > 
> > 
> > OK, so I responded to this but I can't find my reply or any of the
> > followups, so let me go again:
> > 
> > I prefer this option, because:
> > 
> > a. Virtually all uses of parity is done in contexts where the sizes of the
> > items for which parity is to be taken are well-defined, but it is *really*
> > easy for integer promotion to cause a value to be extended to 32 bits
> > unnecessarily (sign or zero extend, although for parity it doesn't make any
> > difference -- if the compiler realizes it.)
> > 
> > b. It makes it easier to add arch-specific implementations, notably using
> > __builtin_parity on architectures where that is known to generate good code.
> > 
> > c. For architectures where only *some* parity implementations are
> > fast/practical, the generic fallbacks will either naturally synthesize them
> > from components via shift-xor, or they can be defined to use a larger
> > version; the function prototype acts like a cast.
> > 
> > d. If there is a reason in the future to add a generic version, it is really
> > easy to do using the size-specific functions as components; this is
> > something we do literally all over the place, using a pattern so common that
> > it, itself, probably should be macroized:
> > 
> > #define parity(x) 				\
> > ({						\
> > 	typeof(x) __x = (x);			\
> > 	bool __y;				\
> > 	switch (sizeof(__x)) {			\
> > 		case 1:				\
> > 			__y = parity8(__x);	\
> > 			break;			\
> > 		case 2:				\
> > 			__y = parity16(__x);	\
> > 			break;			\
> > 		case 4:				\
> > 			__y = parity32(__x);	\
> > 			break;			\
> > 		case 8:				\
> > 			__y = parity64(__x);	\
> > 			break;			\
> > 		default:			\
> > 			BUILD_BUG();		\
> > 			break;			\
> > 	}					\
> > 	__y;					\
> > })
> >
> Thank you for your detailed response and for explaining the rationale
> behind your preference. The points you outlined in (a)–(d) all seem
> quite reasonable to me.
> 
> Yury,
> do you have any feedback on this?
> Thank you.

My feedback to you:

I asked you to share any numbers about each approach. Asm listings,
performance tests, bloat-o-meter. But you did nothing or very little
in that department. You move this series, and it means you should be
very well aware of alternative solutions, their pros and cons.

Instead, you started a poll to pick the best solution. This is not
what I expected, and this is not how the best solution can be found.

To H. Peter and everyone:

Thank you for sharing your opinion on this fixed parity(). Your
arguments may or may not be important, depending on what existing
users actually need. Unfortunately, Kuan-Wei didn't collect
performance numbers and opinions from those proposed users.

I already told that, and I will say again: with the lack of any
evidence that performance and/or code generation is important here,
the best solution is one that minimizes maintainers' (my!) burden.

In other words, bool parity(unsigned long long). I'm OK to maintain
a macro, as well. I understand that more complicated solutions may be
more effective. I will take them only if they will be well advocated.

I hope this will help us to stop moving this discussion back and forth
and save our time, guys.

Thanks,
Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Thu, Apr 03, 2025 at 12:14:04PM -0400, Yury Norov wrote:
> On Thu, Apr 03, 2025 at 10:39:03PM +0800, Kuan-Wei Chiu wrote:
> > On Tue, Mar 25, 2025 at 12:43:25PM -0700, H. Peter Anvin wrote:
> > > On 3/23/25 08:16, Kuan-Wei Chiu wrote:
> > > > 
> > > > Interface 3: Multiple Functions
> > > > Description: bool parity_odd8/16/32/64()
> > > > Pros: No need for explicit casting; easy to integrate
> > > >        architecture-specific optimizations; except for parity8(), all
> > > >        functions are one-liners with no significant code duplication
> > > > Cons: More functions may increase maintenance burden
> > > > Opinions: Only I support this approach
> > > > 
> > > 
> > > OK, so I responded to this but I can't find my reply or any of the
> > > followups, so let me go again:
> > > 
> > > I prefer this option, because:
> > > 
> > > a. Virtually all uses of parity is done in contexts where the sizes of the
> > > items for which parity is to be taken are well-defined, but it is *really*
> > > easy for integer promotion to cause a value to be extended to 32 bits
> > > unnecessarily (sign or zero extend, although for parity it doesn't make any
> > > difference -- if the compiler realizes it.)
> > > 
> > > b. It makes it easier to add arch-specific implementations, notably using
> > > __builtin_parity on architectures where that is known to generate good code.
> > > 
> > > c. For architectures where only *some* parity implementations are
> > > fast/practical, the generic fallbacks will either naturally synthesize them
> > > from components via shift-xor, or they can be defined to use a larger
> > > version; the function prototype acts like a cast.
> > > 
> > > d. If there is a reason in the future to add a generic version, it is really
> > > easy to do using the size-specific functions as components; this is
> > > something we do literally all over the place, using a pattern so common that
> > > it, itself, probably should be macroized:
> > > 
> > > #define parity(x) 				\
> > > ({						\
> > > 	typeof(x) __x = (x);			\
> > > 	bool __y;				\
> > > 	switch (sizeof(__x)) {			\
> > > 		case 1:				\
> > > 			__y = parity8(__x);	\
> > > 			break;			\
> > > 		case 2:				\
> > > 			__y = parity16(__x);	\
> > > 			break;			\
> > > 		case 4:				\
> > > 			__y = parity32(__x);	\
> > > 			break;			\
> > > 		case 8:				\
> > > 			__y = parity64(__x);	\
> > > 			break;			\
> > > 		default:			\
> > > 			BUILD_BUG();		\
> > > 			break;			\
> > > 	}					\
> > > 	__y;					\
> > > })
> > >
> > Thank you for your detailed response and for explaining the rationale
> > behind your preference. The points you outlined in (a)–(d) all seem
> > quite reasonable to me.
> > 
> > Yury,
> > do you have any feedback on this?
> > Thank you.
> 
> My feedback to you:
> 
> I asked you to share any numbers about each approach. Asm listings,
> performance tests, bloat-o-meter. But you did nothing or very little
> in that department. You move this series, and it means you should be
> very well aware of alternative solutions, their pros and cons.
>
I'm willing to run micro-benchmarks, but even with performance data, I
lack the domain knowledge to determine which users care about parity
efficiency. No one in Cc has clarified this either.

> Instead, you started a poll to pick the best solution. This is not
> what I expected, and this is not how the best solution can be found.
> 
> To H. Peter and everyone:
> 
> Thank you for sharing your opinion on this fixed parity(). Your
> arguments may or may not be important, depending on what existing
> users actually need. Unfortunately, Kuan-Wei didn't collect
> performance numbers and opinions from those proposed users.
> 
> I already told that, and I will say again: with the lack of any
> evidence that performance and/or code generation is important here,
> the best solution is one that minimizes maintainers' (my!) burden.
> 
> In other words, bool parity(unsigned long long). I'm OK to maintain
> a macro, as well. I understand that more complicated solutions may be
> more effective. I will take them only if they will be well advocated.
> 
Before Peter suggested an arch-specific implementation, I planned to go
with approach #1, as it minimizes maintenance overhead in the absence
of clear user requirements.

Peter,
Have you identified any users who care about parity efficiency?
If not, do we still need to introduce an arch-specific implementation?

Regards,
Kuan-Wei

> I hope this will help us to stop moving this discussion back and forth
> and save our time, guys.
> 
> Thanks,
> Yury

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Jeremy Kerr]

Hi Yuri & Kuan-Wei:

> Thank you for sharing your opinion on this fixed parity(). Your
> arguments may or may not be important, depending on what existing
> users actually need. Unfortunately, Kuan-Wei didn't collect
> performance numbers and opinions from those proposed users.

For the fsi-i2c side: this isn't a performance-critical path, and any
reasonable common approach would likely perform better that the current
per-bit implementation.

Our common targets for this driver would be arm and powerpc64le. In case
it's useful as a reference, using the kernel compilers I have to hand, a
__builtin_parity() is a library call on the former, and a two-instruction
sequence for the latter.

Cheers,


Jeremy

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

Hi Jeremy,

On Fri, Apr 04, 2025 at 10:51:55AM +0800, Jeremy Kerr wrote:
> Hi Yuri & Kuan-Wei:
> 
> > Thank you for sharing your opinion on this fixed parity(). Your
> > arguments may or may not be important, depending on what existing
> > users actually need. Unfortunately, Kuan-Wei didn't collect
> > performance numbers and opinions from those proposed users.
> 
> For the fsi-i2c side: this isn't a performance-critical path, and any
> reasonable common approach would likely perform better that the current
> per-bit implementation.
> 
> Our common targets for this driver would be arm and powerpc64le. In case
> it's useful as a reference, using the kernel compilers I have to hand, a
> __builtin_parity() is a library call on the former, and a two-instruction
> sequence for the latter.
> 
Thanks for your feedback.

IIUC, from the fsi-i2c perspective, parity efficiency isn't a major
concern, but you still prefer optimizing with methods like
__builtin_parity(). I'm just unsure if this aligns with Yury's point
about needing "evidence that performance and/or code generation is
important here."

Regards,
Kuna-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Kuan-Wei Chiu]

On Thu, Apr 03, 2025 at 12:14:04PM -0400, Yury Norov wrote:
> On Thu, Apr 03, 2025 at 10:39:03PM +0800, Kuan-Wei Chiu wrote:
> > On Tue, Mar 25, 2025 at 12:43:25PM -0700, H. Peter Anvin wrote:
> > > On 3/23/25 08:16, Kuan-Wei Chiu wrote:
> > > > 
> > > > Interface 3: Multiple Functions
> > > > Description: bool parity_odd8/16/32/64()
> > > > Pros: No need for explicit casting; easy to integrate
> > > >        architecture-specific optimizations; except for parity8(), all
> > > >        functions are one-liners with no significant code duplication
> > > > Cons: More functions may increase maintenance burden
> > > > Opinions: Only I support this approach
> > > > 
> > > 
> > > OK, so I responded to this but I can't find my reply or any of the
> > > followups, so let me go again:
> > > 
> > > I prefer this option, because:
> > > 
> > > a. Virtually all uses of parity is done in contexts where the sizes of the
> > > items for which parity is to be taken are well-defined, but it is *really*
> > > easy for integer promotion to cause a value to be extended to 32 bits
> > > unnecessarily (sign or zero extend, although for parity it doesn't make any
> > > difference -- if the compiler realizes it.)
> > > 
> > > b. It makes it easier to add arch-specific implementations, notably using
> > > __builtin_parity on architectures where that is known to generate good code.
> > > 
> > > c. For architectures where only *some* parity implementations are
> > > fast/practical, the generic fallbacks will either naturally synthesize them
> > > from components via shift-xor, or they can be defined to use a larger
> > > version; the function prototype acts like a cast.
> > > 
> > > d. If there is a reason in the future to add a generic version, it is really
> > > easy to do using the size-specific functions as components; this is
> > > something we do literally all over the place, using a pattern so common that
> > > it, itself, probably should be macroized:
> > > 
> > > #define parity(x) 				\
> > > ({						\
> > > 	typeof(x) __x = (x);			\
> > > 	bool __y;				\
> > > 	switch (sizeof(__x)) {			\
> > > 		case 1:				\
> > > 			__y = parity8(__x);	\
> > > 			break;			\
> > > 		case 2:				\
> > > 			__y = parity16(__x);	\
> > > 			break;			\
> > > 		case 4:				\
> > > 			__y = parity32(__x);	\
> > > 			break;			\
> > > 		case 8:				\
> > > 			__y = parity64(__x);	\
> > > 			break;			\
> > > 		default:			\
> > > 			BUILD_BUG();		\
> > > 			break;			\
> > > 	}					\
> > > 	__y;					\
> > > })
> > >
> > Thank you for your detailed response and for explaining the rationale
> > behind your preference. The points you outlined in (a)–(d) all seem
> > quite reasonable to me.
> > 
> > Yury,
> > do you have any feedback on this?
> > Thank you.
> 
> My feedback to you:
> 
> I asked you to share any numbers about each approach. Asm listings,
> performance tests, bloat-o-meter. But you did nothing or very little
> in that department. You move this series, and it means you should be
> very well aware of alternative solutions, their pros and cons.
> 
It seems the concern is that I didn't provide assembly results and
performance numbers. While I believe that listing these numbers alone
cannot prove which users really care about parity efficiency, I have
included the assembly results and my initial observations below. Some
differences, like mov vs movzh, are likely difficult to measure.

Compilation on x86-64 using GCC 14.2 with O2 Optimization:

Link to Godbolt: https://godbolt.org/z/EsqPMz8cq

For u8 Input:
- #2 and #3 generate exactly the same assembly code, while #1 replaces
  one `mov` instruction with `movzh`, which may slightly slow down the
  performance due to zero extension.
- Efficiency: #2 = #3 > #1

For u16 Input:
- As with u8 input, #1 performs an unnecessary zero extension, while #3
  replaces one of the `shr` instructions in #2 with a `mov`, making it
  slightly faster.
- Efficiency: #3 > #2 > #1

For u32 Input:
- #1 has an additional `mov` instruction compared to #2, and #2 has an
  extra `shr` instruction compared to #3.
- Efficiency: #3 > #2 > #1

For u64 Input:
- #1 and #2 generate the same code, but #3 has one less `shr`
  instruction compared to the others.
- Efficiency: #3 > #1 = #2

---

Adding -m32 Flag to View Assembly for 32-bit Machine:

Link to Godbolt: https://godbolt.org/z/GrPa86Eq5

For u8 Input:
- #2 and #3 generate identical assembly code, whereas #1 has additional
  `mov`, `shr`, and `push/pop` instructions.
- Efficiency: #2 = #3 > #1

For u16 Input:
- #1 uses a lot of `xmm` register operations, making it slower than #2
  and #3. Additionally, #2 has an extra `shr` instruction compared to #3.
- Efficiency: #3 > #2 > #1

For u32 Input:
- #1 again uses a lot of `xmm` register operations, so it is slower
  than #2 and #3, and #2 has an additional `shr` instruction compared to #3.
- Efficiency: #3 > #2 > #1

For u64 Input:
- Both #1 and #2 use `xmm` register operations, but #1 has a few extra
  `movdqa` instructions. #3 is more concise, using a few `shr`, `xor`,
  and `mov` instructions to complete the operation.
- Efficiency: #3 > #2 > #1

Regards,
Kuan-Wei

Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper

[Jeremy Kerr]

Hi Kuan-Wei,

> Thanks for your feedback.

No problem!

> IIUC, from the fsi-i2c perspective, parity efficiency isn't a major
> concern,

Yes

> but you still prefer optimizing with methods like __builtin_parity().

No, it's not really about optimisation. In the case of this driver, my
preference would be more directed to using common code (either in the
form of these changes, or __builtin_parity) rather than any performance
considerations.

The implementation details I gave was more a note about the platforms
that are applicable for the driver.

Cheers,


Jeremy