Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1401587 > unrolled thread

Re: [patch V4 09/31] bitops: Add x86-specific parity functions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [patch V4 09/31] bitops: Add x86-specific parity functions Zhaoxiu Zeng <zengzhaoxiu@163.com> - 2016-05-16 18:00 +0200
    Re: [patch V4 09/31] bitops: Add x86-specific parity functions Peter Zijlstra <peterz@infradead.org> - 2016-05-16 19:10 +0200
      Re: [patch V4 09/31] bitops: Add x86-specific parity functions "H. Peter Anvin" <hpa@zytor.com> - 2016-05-16 21:30 +0200
        Re: [patch V4 09/31] bitops: Add x86-specific parity functions Peter Zijlstra <peterz@infradead.org> - 2016-05-17 13:30 +0200

#1401587 — Re: [patch V4 09/31] bitops: Add x86-specific parity functions

On 2016/5/11 17:31, Peter Zijlstra wrote:
> Please use the GEN_*_RMWcc() stuff to avoid the setpo where possible.

Setpo is better.
In most cases, we need to store the parity, or compare it with other variables.

For example, in drivers/net/ethernet/broadcom/tg3.c,

static int tg3_test_nvram(struct tg3 *tp)
{
	......
	if (parity8(data[i]) == !!parity[i])
		goto out;
	......
}

If use GEN_BINARY_RMWcc stuff，

static inline unsigned int __arch_parity8(unsigned int w)
{
	GEN_BINARY_RMWcc("testb", w, "er", 0xff, "%0", "po");
}

gcc's output:
   1c2fe:	0f b6 54 05 a0       	movzbl -0x60(%rbp,%rax,1),%edx
   1c303:	89 55 9c             	mov    %edx,-0x64(%rbp)
   1c306:	f6 45 9c ff          	testb  $0xff,-0x64(%rbp)
   1c30a:	7b 2c                	jnp    1c338 <tg3_self_test+0xf98>
   1c30c:	31 c9                	xor    %ecx,%ecx
   1c30e:	31 d2                	xor    %edx,%edx
   1c310:	80 7c 05 bc 00       	cmpb   $0x0,-0x44(%rbp,%rax,1)
   1c315:	0f 95 c2             	setne  %dl
   1c318:	39 ca                	cmp    %ecx,%edx
   1c31a:	75 d8                	jne    1c2f4 <tg3_self_test+0xf54>
   ......
   1c338:	b9 01 00 00 00       	mov    $0x1,%ecx
   1c33d:	eb cf                	jmp    1c30e <tg3_self_test+0xf6e>


Else if use setpo,

static inline unsigned int __arch_parity8(unsigned int w)
{
	u8 res;
	asm("test $0xff, %1; setpo %0" : "=qm" (res) : "rm" (w) : "memory");
	return res;
}

gcc's output:
   1c2fe:	31 c9                	xor    %ecx,%ecx
   1c300:	0f b6 44 15 a0       	movzbl -0x60(%rbp,%rdx,1),%eax
   1c305:	a9 ff 00 00 00       	test   $0xff,%eax
   1c30a:	0f 9b c0             	setnp  %al
   1c30d:	80 7c 15 bc 00       	cmpb   $0x0,-0x44(%rbp,%rdx,1)
   1c312:	0f b6 c0             	movzbl %al,%eax
   1c315:	0f 95 c1             	setne  %cl
   1c318:	39 c8                	cmp    %ecx,%eax
   1c31a:	75 d8                	jne    1c2f4 <tg3_self_test+0xf54>

[toc] | [next] | [standalone]

#1401641

On Mon, May 16, 2016 at 11:49:05PM +0800, Zhaoxiu Zeng wrote:
> On 2016/5/11 17:31, Peter Zijlstra wrote:
> > Please use the GEN_*_RMWcc() stuff to avoid the setpo where possible.
> 
> Setpo is better.
> In most cases, we need to store the parity, or compare it with other variables.
> 
> For example, in drivers/net/ethernet/broadcom/tg3.c,
> 
> static int tg3_test_nvram(struct tg3 *tp)
> {
> 	......
> 	if (parity8(data[i]) == !!parity[i])
> 		goto out;
> 	......
> }
> 
> If use GEN_BINARY_RMWcc stuff，
> 
> static inline unsigned int __arch_parity8(unsigned int w)
> {
> 	GEN_BINARY_RMWcc("testb", w, "er", 0xff, "%0", "po");
> }

blergh; GCC does indeed make a mess of that. It looks we'll need the
cc-output stuff for this in order for GCC to generates sane code for
that :/

[toc] | [prev] | [next] | [standalone]

#1401722

On May 16, 2016 10:06:08 AM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Mon, May 16, 2016 at 11:49:05PM +0800, Zhaoxiu Zeng wrote:
>> On 2016/5/11 17:31, Peter Zijlstra wrote:
>> > Please use the GEN_*_RMWcc() stuff to avoid the setpo where
>possible.
>> 
>> Setpo is better.
>> In most cases, we need to store the parity, or compare it with other
>variables.
>> 
>> For example, in drivers/net/ethernet/broadcom/tg3.c,
>> 
>> static int tg3_test_nvram(struct tg3 *tp)
>> {
>> 	......
>> 	if (parity8(data[i]) == !!parity[i])
>> 		goto out;
>> 	......
>> }
>> 
>> If use GEN_BINARY_RMWcc stuff，
>> 
>> static inline unsigned int __arch_parity8(unsigned int w)
>> {
>> 	GEN_BINARY_RMWcc("testb", w, "er", 0xff, "%0", "po");
>> }
>
>blergh; GCC does indeed make a mess of that. It looks we'll need the
>cc-output stuff for this in order for GCC to generates sane code for
>that :/

For what it's worth, I have a patchset for cc out just about ready to post.  It is in gcc 6.1.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

#1402298

On Mon, May 16, 2016 at 12:22:05PM -0700, H. Peter Anvin wrote:
> On May 16, 2016 10:06:08 AM PDT, Peter Zijlstra <peterz@infradead.org> wrote:

> >blergh; GCC does indeed make a mess of that. It looks we'll need the
> >cc-output stuff for this in order for GCC to generates sane code for
> >that :/
> 
> For what it's worth, I have a patchset for cc out just about ready to post.  It is in gcc 6.1.

Nice, looking fwd to play with that. Thanks!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web