public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@kernel.org>
To: Glauber Costa <gcosta@redhat.com>
Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@elte.hu,
	x86@kernel.org
Subject: Re: [PATCH 08/39] don't use word-size specifiers
Date: Fri, 27 Jun 2008 16:17:56 -0700	[thread overview]
Message-ID: <48657524.4020307@kernel.org> (raw)
In-Reply-To: <1214602486-17080-9-git-send-email-gcosta@redhat.com>

Glauber Costa wrote:
> since the instructions refer to registers, they'll be able
> to figure it out.
> 
> Signed-off-by: Glauber Costa <gcosta@redhat.com>

> diff --git a/arch/x86/lib/getuser_32.S b/arch/x86/lib/getuser_32.S
> index 6d84b53..8200fde 100644
> --- a/arch/x86/lib/getuser_32.S
> +++ b/arch/x86/lib/getuser_32.S
> @@ -29,44 +29,44 @@
>  ENTRY(__get_user_1)
>  	CFI_STARTPROC
>  	GET_THREAD_INFO(%edx)
> -	cmpl TI_addr_limit(%edx),%eax
> +	cmp TI_addr_limit(%edx),%eax
>  	jae bad_get_user
> -1:	movzbl (%eax),%edx
> -	xorl %eax,%eax
> +1:	movzb (%eax),%edx
> +	xor %eax,%eax
>  	ret
>  	CFI_ENDPROC

I hate to say it, but I really think this is a step backwards in 
readability.  Consistency is a good thing, and with the suffixes in 
place we are consistent between instructions that refer to memory and 
instructions that refer to registers.  We also get one more check on 
things, where the assembler can tell the programmer he probably typoed.

So I would prefer if we *didn't* go down this route, except for explicit 
unification, but that's not the case here (since the size is still 
explicit in the register names.)

	-hpa

  parent reply	other threads:[~2008-06-27 23:19 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-27 21:34 [PATCH 0/39] Merge files at x86/lib Glauber Costa
2008-06-27 21:34 ` [PATCH 01/39] Don't use size specifiers Glauber Costa
2008-06-27 21:34   ` [PATCH 02/39] provide delay loop for x86_64 Glauber Costa
2008-06-27 21:34     ` [PATCH 03/39] use rdtscll in read_current_timer for i386 Glauber Costa
2008-06-27 21:34       ` [PATCH 04/39] explicitly use edx in const delay function Glauber Costa
2008-06-27 21:34         ` [PATCH 05/39] integrate delay functions Glauber Costa
2008-06-27 21:34           ` [PATCH 06/39] use something common for both architectures Glauber Costa
2008-06-27 21:34             ` [PATCH 07/39] don't clobber r8 nor use rcx Glauber Costa
2008-06-27 21:34               ` [PATCH 08/39] don't use word-size specifiers Glauber Costa
2008-06-27 21:34                 ` [PATCH 09/39] adapt x86_64 getuser functions Glauber Costa
2008-06-27 21:34                   ` [PATCH 10/39] rename threadinfo to TI Glauber Costa
2008-06-27 21:34                     ` [PATCH 11/39] Don't use word-size specifiers on getuser_64 Glauber Costa
2008-06-27 21:34                       ` [PATCH 12/39] introduce __ASM_REG macro Glauber Costa
2008-06-27 21:34                         ` [PATCH 13/39] use _ASM_PTR instead of explicit word-size pointers Glauber Costa
2008-06-27 21:34                           ` [PATCH 14/39] merge getuser asm functions Glauber Costa
2008-06-27 21:34                             ` [PATCH 15/39] don't save ebx in putuser_32.S Glauber Costa
2008-06-27 21:34                               ` [PATCH 16/39] user put_user_x instead of all variants Glauber Costa
2008-06-27 21:34                                 ` [PATCH 17/39] clobber rbx in putuser_64.S Glauber Costa
2008-06-27 21:34                                   ` [PATCH 18/39] pass argument to putuser_64 functions in ax register Glauber Costa
2008-06-27 21:34                                     ` [PATCH 19/39] change testing logic in putuser_64.S Glauber Costa
2008-06-27 21:34                                       ` [PATCH 20/39] replace function headers by macros Glauber Costa
2008-06-27 21:34                                         ` [PATCH 21/39] don't use word-size specifiers in putuser files Glauber Costa
2008-06-27 21:34                                           ` [PATCH 22/39] use macros from asm.h Glauber Costa
2008-06-27 21:34                                             ` [PATCH 23/39] merge putuser asm functions Glauber Costa
2008-06-27 21:34                                               ` [PATCH 24/39] commonize __range_not_ok Glauber Costa
2008-06-27 21:34                                                 ` [PATCH 25/39] merge common parts of uaccess Glauber Costa
2008-06-27 21:34                                                   ` [PATCH 26/39] merge getuser Glauber Costa
2008-06-27 21:34                                                     ` [PATCH 27/39] move __addr_ok to uaccess.h Glauber Costa
2008-06-27 21:34                                                       ` [PATCH 28/39] use k modifier for 4-byte access Glauber Costa
2008-06-27 21:34                                                         ` [PATCH 29/39] mark x86_64 as having a working WP Glauber Costa
2008-06-27 21:34                                                           ` [PATCH 30/39] don't always use EFAULT on __put_user_size Glauber Costa
2008-06-27 21:34                                                             ` [PATCH 31/39] merge __put_user_asm and its user Glauber Costa
2008-06-27 21:34                                                               ` [PATCH 32/39] don't always use EFAULT on __get_user_size Glauber Costa
2008-06-27 21:34                                                                 ` [PATCH 33/39] merge __get_user_asm and its users Glauber Costa
2008-06-27 21:34                                                                   ` [PATCH 34/39] Be more explicit in __put_user_x Glauber Costa
2008-06-27 21:34                                                                     ` [PATCH 35/39] turn __put_user_check directly into put_user Glauber Costa
2008-06-27 21:34                                                                       ` [PATCH 36/39] merge put_user Glauber Costa
2008-06-27 21:34                                                                         ` [PATCH 37/39] move __get_user and __put_user into uaccess.h Glauber Costa
2008-06-27 21:34                                                                           ` [PATCH 38/39] put movsl_mask " Glauber Costa
2008-06-27 21:34                                                                             ` [PATCH 39/39] define architectural characteristics in uaccess.h Glauber Costa
2008-06-30  6:30                                                   ` [PATCH 25/39] merge common parts of uaccess Ingo Molnar
2008-06-30  6:31                                                     ` Ingo Molnar
2008-06-30 16:31                                                       ` H. Peter Anvin
2008-06-30 18:48                                                     ` Glauber Costa
2008-06-30 18:53                                                       ` Ingo Molnar
2008-06-30 19:00                                                         ` Glauber Costa
2008-06-30 19:29                                                           ` Ingo Molnar
2008-06-28 12:12                                   ` [PATCH 17/39] clobber rbx in putuser_64.S Andi Kleen
2008-06-30 21:00                                     ` Glauber Costa
2008-06-30 23:45                                       ` Andi Kleen
2008-07-01  2:46                                         ` Glauber Costa
2008-07-01 15:11                                         ` Glauber Costa
2008-06-30 21:40                                     ` Glauber Costa
2008-06-28 12:10                               ` [PATCH 15/39] don't save ebx in putuser_32.S Andi Kleen
2008-06-30 20:43                                 ` Glauber Costa
2008-06-28 12:09                         ` [PATCH 12/39] introduce __ASM_REG macro Andi Kleen
2008-06-27 23:17                 ` H. Peter Anvin [this message]
2008-06-27 23:22                   ` [PATCH 08/39] don't use word-size specifiers H. Peter Anvin
2008-06-28  1:41                     ` Glauber Costa
2008-06-28  5:09                       ` H. Peter Anvin
2008-06-30  5:10 ` [PATCH 0/39] Merge files at x86/lib H. Peter Anvin

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=48657524.4020307@kernel.org \
    --to=hpa@kernel.org \
    --cc=gcosta@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox