All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: Kuan-Wei Chiu <visitorckw@gmail.com>,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	dave.hansen@linux.intel.com, x86@kernel.org, jk@ozlabs.org,
	joel@jms.id.au, eajames@linux.ibm.com, andrzej.hajda@intel.com,
	neil.armstrong@linaro.org, rfoss@kernel.org,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch,
	dmitry.torokhov@gmail.com, mchehab@kernel.org,
	awalls@md.metrocast.net, hverkuil@xs4all.nl,
	miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
	louis.peens@corigine.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	parthiban.veerasooran@microchip.com,
	arend.vanspriel@broadcom.com, johannes@sipsolutions.net,
	gregkh@linuxfoundation.org, jirislaby@kernel.org,
	akpm@linux-foundation.org, hpa@zytor.com, alistair@popple.id.au,
	linux@rasmusvillemoes.dk, Laurent.pinchart@ideasonboard.com,
	jonas@kwiboo.se, jernej.skrabec@gmail.com, kuba@kernel.org,
	linux-kernel@vger.kernel.org, linux-fsi@lists.ozlabs.org,
	dri-devel@lists.freedesktop.org, linux-input@vger.kernel.org,
	linux-media@vger.kernel.org, linux-mtd@lists.infradead.org,
	oss-drivers@corigine.com, netdev@vger.kernel.org,
	linux-wireless@vger.kernel.org, brcm80211@lists.linux.dev,
	brcm80211-dev-list.pdl@broadcom.com,
	linux-serial@vger.kernel.org, bpf@vger.kernel.org,
	jserv@ccns.ncku.edu.tw, Yu-Chun Lin <eleanor15x@gmail.com>
Subject: Re: [PATCH 02/17] bitops: Add generic parity calculation for u64
Date: Wed, 26 Feb 2025 22:29:11 +0000	[thread overview]
Message-ID: <20250226222911.22cb0c18@pumpkin> (raw)
In-Reply-To: <Z7zIBwH4aUA7G9MY@thinkpad>

On Mon, 24 Feb 2025 14:27:03 -0500
Yury Norov <yury.norov@gmail.com> wrote:
....
> +#define parity(val)					\
> +({							\
> +	u64 __v = (val);				\
> +	int __ret;					\
> +	switch (BITS_PER_TYPE(val)) {			\
> +	case 64:					\
> +		__v ^= __v >> 32;			\
> +		fallthrough;				\
> +	case 32:					\
> +		__v ^= __v >> 16;			\
> +		fallthrough;				\
> +	case 16:					\
> +		__v ^= __v >> 8;			\
> +		fallthrough;				\
> +	case 8:						\
> +		__v ^= __v >> 4;			\
> +		__ret =  (0x6996 >> (__v & 0xf)) & 1;	\
> +		break;					\
> +	default:					\
> +		BUILD_BUG();				\
> +	}						\
> +	__ret;						\
> +})
> +

You really don't want to do that!
gcc makes a right hash of it for x86 (32bit).
See https://www.godbolt.org/z/jG8dv3cvs

You do better using a __v32 after the 64bit xor.

Even the 64bit version is probably sub-optimal (both gcc and clang).
The whole lot ends up being a bit single register dependency chain.
You want to do:
	mov %eax, %edx
	shrl $n, %eax
	xor %edx, %eax
so that the 'mov' and 'shrl' can happen in the same clock
(without relying on the register-register move being optimised out).

I dropped in the arm64 for an example of where the magic shift of 6996
just adds an extra instruction.

	David



WARNING: multiple messages have this Message-ID (diff)
From: David Laight <david.laight.linux@gmail.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: Kuan-Wei Chiu <visitorckw@gmail.com>,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	dave.hansen@linux.intel.com, x86@kernel.org, jk@ozlabs.org,
	joel@jms.id.au, eajames@linux.ibm.com, andrzej.hajda@intel.com,
	neil.armstrong@linaro.org, rfoss@kernel.org,
	maarten.lankhorst@linux.intel.com, mripard@kernel.org,
	tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch,
	dmitry.torokhov@gmail.com, mchehab@kernel.org,
	awalls@md.metrocast.net, hverkuil@xs4all.nl,
	miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
	louis.peens@corigine.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	parthiban.veerasooran@microchip.com,
	arend.vanspriel@broadcom.com, johannes@sipsolutions.net,
	gregkh@linuxfoundation.org, jirislaby@kernel.org,
	akpm@linux-foundation.org, hpa@zytor.com, alistair@popple.id.au,
	linux@rasmusvillemoes.dk, Laurent.pinchart@ideasonboard.com,
	jonas@kwiboo.se, jernej.skrabec@gmail.com, kuba@kernel.org,
	linux-kernel@vger.kernel.org, linux-fsi@lists.ozlabs.org,
	dri-devel@lists.freedesktop.org, linux-input@vger.kernel.org,
	linux-media@vger.kernel.org, linux-mtd@lists.infradead.org,
	oss-drivers@corigine.com, netdev@vger.kernel.org,
	linux-wireless@vger.kernel.org, brcm80211@lists.linux.dev,
	brcm80211-dev-list.pdl@broadcom.com,
	linux-serial@vger.kernel.org, bpf@vger.kernel.org,
	jserv@ccns.ncku.edu.tw, Yu-Chun Lin <eleanor15x@gmail.com>
Subject: Re: [PATCH 02/17] bitops: Add generic parity calculation for u64
Date: Wed, 26 Feb 2025 22:29:11 +0000	[thread overview]
Message-ID: <20250226222911.22cb0c18@pumpkin> (raw)
In-Reply-To: <Z7zIBwH4aUA7G9MY@thinkpad>

On Mon, 24 Feb 2025 14:27:03 -0500
Yury Norov <yury.norov@gmail.com> wrote:
....
> +#define parity(val)					\
> +({							\
> +	u64 __v = (val);				\
> +	int __ret;					\
> +	switch (BITS_PER_TYPE(val)) {			\
> +	case 64:					\
> +		__v ^= __v >> 32;			\
> +		fallthrough;				\
> +	case 32:					\
> +		__v ^= __v >> 16;			\
> +		fallthrough;				\
> +	case 16:					\
> +		__v ^= __v >> 8;			\
> +		fallthrough;				\
> +	case 8:						\
> +		__v ^= __v >> 4;			\
> +		__ret =  (0x6996 >> (__v & 0xf)) & 1;	\
> +		break;					\
> +	default:					\
> +		BUILD_BUG();				\
> +	}						\
> +	__ret;						\
> +})
> +

You really don't want to do that!
gcc makes a right hash of it for x86 (32bit).
See https://www.godbolt.org/z/jG8dv3cvs

You do better using a __v32 after the 64bit xor.

Even the 64bit version is probably sub-optimal (both gcc and clang).
The whole lot ends up being a bit single register dependency chain.
You want to do:
	mov %eax, %edx
	shrl $n, %eax
	xor %edx, %eax
so that the 'mov' and 'shrl' can happen in the same clock
(without relying on the register-register move being optimised out).

I dropped in the arm64 for an example of where the magic shift of 6996
just adds an extra instruction.

	David



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2025-02-26 22:29 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-23 16:42 [PATCH 00/17] Introduce and use generic parity32/64 helper Kuan-Wei Chiu
2025-02-23 16:42 ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 01/17] bitops: Add generic parity calculation for u32 Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 02/17] bitops: Add generic parity calculation for u64 Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-24  7:09   ` Jiri Slaby
2025-02-24  7:09     ` Jiri Slaby
2025-02-24 13:34     ` David Laight
2025-02-24 13:34       ` David Laight
2025-02-24 16:56       ` Yu-Chun Lin
2025-02-24 16:56         ` Yu-Chun Lin
2025-02-25 15:21       ` H. Peter Anvin
2025-02-25 15:21         ` H. Peter Anvin
2025-02-25 15:24       ` H. Peter Anvin
2025-02-25 15:24         ` H. Peter Anvin
2025-02-25 21:43         ` Andrew Cooper
2025-02-25 21:43           ` Andrew Cooper
2025-02-26  1:35           ` H. Peter Anvin
2025-02-26  1:35             ` H. Peter Anvin
2025-02-24 19:27   ` Yury Norov
2025-02-24 19:27     ` Yury Norov
2025-02-25 13:29     ` Kuan-Wei Chiu
2025-02-25 13:29       ` Kuan-Wei Chiu
2025-02-25 14:20       ` Kuan-Wei Chiu
2025-02-25 14:20         ` Kuan-Wei Chiu
2025-02-26  7:14       ` Jiri Slaby
2025-02-26  7:14         ` Jiri Slaby
2025-02-26 17:59         ` Kuan-Wei Chiu
2025-02-26 17:59           ` Kuan-Wei Chiu
2025-02-26 18:33         ` Yury Norov
2025-02-26 18:33           ` Yury Norov
2025-02-27  6:38           ` Jiri Slaby
2025-02-27  6:38             ` Jiri Slaby
2025-02-27 17:37             ` Yury Norov
2025-02-27 17:37               ` Yury Norov
2025-02-26 22:29     ` David Laight [this message]
2025-02-26 22:29       ` David Laight
2025-02-27 18:05       ` Yury Norov
2025-02-27 18:05         ` Yury Norov
2025-02-27 21:57         ` David Laight
2025-02-27 21:57           ` David Laight
2025-02-28  1:50           ` H. Peter Anvin
2025-02-28  1:50             ` H. Peter Anvin
2025-03-02 15:47           ` Yury Norov
2025-03-02 15:47             ` Yury Norov
2025-02-23 16:42 ` [PATCH 03/17] x86: Replace open-coded parity calculation with parity8() Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-24 15:24   ` Uros Bizjak
2025-02-24 15:24     ` Uros Bizjak
2025-02-24 21:55     ` H. Peter Anvin
2025-02-24 21:55       ` H. Peter Anvin
2025-02-24 22:08       ` Uros Bizjak
2025-02-24 22:08         ` Uros Bizjak
2025-02-24 22:18         ` H. Peter Anvin
2025-02-24 22:18           ` H. Peter Anvin
2025-02-25  3:36         ` H. Peter Anvin
2025-02-25  3:36           ` H. Peter Anvin
2025-02-24 22:17       ` Yury Norov
2025-02-24 22:17         ` Yury Norov
2025-02-24 22:21         ` H. Peter Anvin
2025-02-24 22:21           ` H. Peter Anvin
2025-02-24 22:30           ` Yury Norov
2025-02-24 22:30             ` Yury Norov
2025-02-25 22:46       ` David Laight
2025-02-25 22:46         ` David Laight
2025-02-26  0:26         ` H. Peter Anvin
2025-02-26  0:26           ` H. Peter Anvin
2025-02-23 16:42 ` [PATCH 04/17] media: media/test_drivers: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 05/17] media: pci: cx18-av-vbi: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 06/17] media: saa7115: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 07/17] serial: max3100: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-24  7:25   ` Jiri Slaby
2025-02-24  7:25     ` Jiri Slaby
2025-02-23 16:42 ` [PATCH 08/17] lib/bch: Replace open-coded parity calculation with parity32() Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 09/17] Input: joystick - " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 10/17] net: ethernet: oa_tc6: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 11/17] wifi: brcm80211: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-25  6:29   ` Arend Van Spriel
2025-02-25  6:29     ` Arend Van Spriel
2025-02-23 16:42 ` [PATCH 12/17] drm/bridge: dw-hdmi: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 13/17] mtd: ssfdc: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 14/17] fsi: i2cr: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 15/17] fsi: i2cr: Replace open-coded parity calculation with parity64() Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 16/17] Input: joystick - " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 16:42 ` [PATCH 17/17] nfp: bpf: " Kuan-Wei Chiu
2025-02-23 16:42   ` Kuan-Wei Chiu
2025-02-23 20:25 ` [PATCH 00/17] Introduce and use generic parity32/64 helper Uros Bizjak
2025-02-23 20:25   ` Uros Bizjak
2025-02-24 15:27   ` Yu-Chun Lin
2025-02-24 15:27     ` Yu-Chun Lin
2025-02-24  7:58 ` Jeremy Kerr
2025-02-24  7:58   ` Jeremy Kerr
2025-02-24 15:35   ` Yu-Chun Lin
2025-02-24 15:35     ` Yu-Chun Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250226222911.22cb0c18@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=alistair@popple.id.au \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrzej.hajda@intel.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=awalls@md.metrocast.net \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=brcm80211@lists.linux.dev \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eajames@linux.ibm.com \
    --cc=edumazet@google.com \
    --cc=eleanor15x@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jernej.skrabec@gmail.com \
    --cc=jirislaby@kernel.org \
    --cc=jk@ozlabs.org \
    --cc=joel@jms.id.au \
    --cc=johannes@sipsolutions.net \
    --cc=jonas@kwiboo.se \
    --cc=jserv@ccns.ncku.edu.tw \
    --cc=kuba@kernel.org \
    --cc=linux-fsi@lists.ozlabs.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=louis.peens@corigine.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mchehab@kernel.org \
    --cc=mingo@redhat.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@corigine.com \
    --cc=pabeni@redhat.com \
    --cc=parthiban.veerasooran@microchip.com \
    --cc=rfoss@kernel.org \
    --cc=richard@nod.at \
    --cc=simona@ffwll.ch \
    --cc=tglx@linutronix.de \
    --cc=tzimmermann@suse.de \
    --cc=vigneshr@ti.com \
    --cc=visitorckw@gmail.com \
    --cc=x86@kernel.org \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.