From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754637AbcEPTUj (ORCPT <rfc822;w@1wt.eu>);
	Mon, 16 May 2016 15:20:39 -0400
Received: from terminus.zytor.com ([198.137.202.10]:49364 "EHLO mail.zytor.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754371AbcEPTUh (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Mon, 16 May 2016 15:20:37 -0400
User-Agent: K-9 Mail for Android
In-Reply-To: <20160511093139.GA3206@twins.programming.kicks-ass.net>
References: <1462955158-28394-1-git-send-email-zengzhaoxiu@163.com> <1462958205-24890-1-git-send-email-zengzhaoxiu@163.com> <20160511093139.GA3206@twins.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain;
 charset=UTF-8
Subject: Re: [patch V4 09/31] bitops: Add x86-specific parity functions
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Mon, 16 May 2016 12:19:15 -0700
To: Peter Zijlstra <peterz@infradead.org>, zengzhaoxiu@163.com
CC: linux-kernel@vger.kernel.org, bp@suse.de, gnomes@lxorguk.ukuu.org.uk,
        andi@firstfloor.org, dvyukov@google.com,
        Zhaoxiu Zeng <zhaoxiu.zeng@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
        x86@kernel.org, Denys Vlasenko <dvlasenk@redhat.com>
Message-ID: <30BAB98B-698D-4C90-A7C2-F6A92AFFD302@zytor.com>
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On May 11, 2016 2:31:39 AM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Wed, May 11, 2016 at 05:16:38PM +0800, zengzhaoxiu@163.com wrote:
>
>> +static inline unsigned int __arch_parity4(unsigned int w)
>> +{
>> +	unsigned int res = 0;
>> +
>> +	asm("test $0xf, %1; setpo %b0"
>> +		: "+q" (res)
>> +		: "r" (w)
>> +		: "cc");
>> +
>> +	return res;
>> +}
>> +
>> +static inline unsigned int __arch_parity8(unsigned int w)
>> +{
>> +	unsigned int res = 0;
>> +
>> +	asm("test %1, %1; setpo %b0"
>> +		: "+q" (res)
>> +		: "r" (w)
>> +		: "cc");
>> +
>> +	return res;
>> +}
>> +
>> +static inline unsigned int __arch_parity16(unsigned int w)
>> +{
>> +	unsigned int res = 0;
>> +
>> +	asm("xor %h1, %b1; setpo %b0"
>> +		: "+q" (res), "+q" (w)
>> +		: : "cc");
>> +
>> +	return res;
>> +}
>
>Please use the GEN_*_RMWcc() stuff to avoid the setpo where possible.
>
>> +
>> +#ifdef CONFIG_64BIT
>> +/* popcnt %eax, %eax -- redundant REX prefix for alignment */
>> +#define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc0"
>> +/* popcnt %rax, %rax */
>> +#define POPCNT64 ".byte 0xf3,0x48,0x0f,0xb8,0xc0"
>> +#else
>> +/* popcnt %eax, %eax */
>> +#define POPCNT32 ".byte 0xf3,0x0f,0xb8,0xc0"
>> +#endif
>
>Yuck, please don't duplicate stuff like this.
>
>> +
>> +static __always_inline unsigned int __arch_parity32(unsigned int w)
>> +{
>> +	unsigned int res;
>> +	unsigned int tmp;
>> +
>> +	asm(ALTERNATIVE(
>> +		"	mov	%%eax, %1	\n"
>> +		"	shr	$16, %%eax	\n"
>> +		"	xor	%1, %%eax	\n"
>> +		"	xor	%%ah, %%al	\n"
>> +		"	mov	$0, %%eax	\n"
>> +		"	setpo	%%al	\n",
>> +		POPCNT32 "			\n"
>> +		"	and	$1, %%eax	\n",
>> +		X86_FEATURE_POPCNT)
>> +		: "=a" (res), "=&r" (tmp)
>> +		: "a" (w)
>> +		: "cc");
>> +
>> +	return res;
>> +}
>
>How many bytes does that end up being? Should we make it a call?

Also, setxx is used with a "bool" or "u8", not " unsigned int"!!!!
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.