Re: [PATCH 03/17] x86: Replace open-coded parity calculation with parity8()

Yury Norov <[email protected]> Mon, 24 Feb 2025 17:17:29 -0500

Newsgroups	dev.linux.lists.brcm80211,org.freedesktop.lists.dri-devel,org.infradead.lists.linux-mtd,org.kernel.vger.bpf,org.kernel.vger.linux-input,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media,org.kernel.vger.linux-serial,org.kernel.vger.linux-wireless,org.kernel.vger.netdev
Message-ID	<Z7zv-c4A76jeMAKf@thinkpad>

On Mon, Feb 24, 2025 at 01:55:28PM -0800, H. Peter Anvin wrote:
> On 2/24/25 07:24, Uros Bizjak wrote:
> > 
> > 
> > On 23. 02. 25 17:42, Kuan-Wei Chiu wrote:
> > > Refactor parity calculations to use the standard parity8() helper. This
> > > change eliminates redundant implementations and improves code
> > > efficiency.
> > 
> > The patch improves parity assembly code in bootflag.o from:
> > 
> >  Â  58:Â Â Â  89 deÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  movÂ Â Â  %ebx,%esi
> >  Â  5a:Â Â Â  b9 08 00 00 00Â Â Â Â Â Â Â Â Â Â  movÂ Â Â  $0x8,%ecx
> >  Â  5f:Â Â Â  31 d2Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  xorÂ Â Â  %edx,%edx
> >  Â  61:Â Â Â  89 f0Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  movÂ Â Â  %esi,%eax
> >  Â  63:Â Â Â  89 d7Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  movÂ Â Â  %edx,%edi
> >  Â  65:Â Â Â  40 d0 eeÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  shrÂ Â Â  %sil
> >  Â  68:Â Â Â  83 e0 01Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  andÂ Â Â  $0x1,%eax
> >  Â  6b:Â Â Â  31 c2Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  xorÂ Â Â  %eax,%edx
> >  Â  6d:Â Â Â  83 e9 01Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  subÂ Â Â  $0x1,%ecx
> >  Â  70:Â Â Â  75 efÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  jneÂ Â Â  61 <sbf_init+0x51>
> >  Â  72:Â Â Â  39 c7Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  cmpÂ Â Â  %eax,%edi
> >  Â  74:Â Â Â  74 7fÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  jeÂ Â Â Â  f5 <sbf_init+0xe5>
> >  Â  76:
> > 
> > to:
> > 
> >  Â  54:Â Â Â  89 d8Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  movÂ Â Â  %ebx,%eax
> >  Â  56:Â Â Â  ba 96 69 00 00Â Â Â Â Â Â Â Â Â Â  movÂ Â Â  $0x6996,%edx
> >  Â  5b:Â Â Â  c0 e8 04Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  shrÂ Â Â  $0x4,%al
> >  Â  5e:Â Â Â  31 d8Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  xorÂ Â Â  %ebx,%eax
> >  Â  60:Â Â Â  83 e0 0fÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  andÂ Â Â  $0xf,%eax
> >  Â  63:Â Â Â  0f a3 c2Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  btÂ Â Â Â  %eax,%edx
> >  Â  66:Â Â Â  73 64Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  jaeÂ Â Â  cc <sbf_init+0xbc>
> >  Â  68:
> > 
> > which is faster and smaller (-10 bytes) code.
> > 
> 
> Of course, on x86, parity8() and parity16() can be implemented very simply:
> 
> (Also, the parity functions really ought to return bool, and be flagged
> __attribute_const__.)

There was a discussion regarding return type when parity8() was added.
The integer type was taken over bool with a sort of consideration that
bool should be returned as an answer to some question, like parity_odd().

To me it's not a big deal. We can switch to boolean and describe in
comment what the 'true' means for the parity() function.