public inbox for [email protected]
 help / color / mirror / Atom feed
From: Willy Tarreau <[email protected]>
To: Alviro Iskandar Setiawan <[email protected]>
Cc: Ammar Faizi <[email protected]>,
	"Paul E. McKenney" <[email protected]>,
	Nugraha <[email protected]>,
	Linux Kernel Mailing List <[email protected]>,
	GNU/Weeb Mailing List <[email protected]>,
	David Laight <[email protected]>
Subject: Re: [PATCH v1 04/11] tools/nolibc: x86-64: Use appropriate register constraints if exist
Date: Thu, 24 Mar 2022 16:42:10 +0100	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAOG64qMwKYHLrUVro1gFhYqHvm8wq5DUdO7QfK5gG2TKhfnNhA@mail.gmail.com>

On Thu, Mar 24, 2022 at 03:33:57PM +0700, Alviro Iskandar Setiawan wrote:
> On Thu, Mar 24, 2022 at 2:57 PM Willy Tarreau <[email protected]> wrote:
> > On Thu, Mar 24, 2022 at 02:30:32PM +0700, Ammar Faizi wrote:
> > > Use appropriate register constraints if exist. Don't use register
> > > variables for all inputs.
> > >
> > > Register variables with "r" constraint should be used when we need to
> > > pass data through a specific register to extended inline assembly that
> > > doesn't have a specific register constraint associated with it (anything
> > > outside %rax, %rbx, %rcx, %rdx, %rsi, %rdi).
> > >
> > > It also simplifies the macro definition.
> >
> > I'm a bit bothered by this one because I went the exact opposite route
> > in the early design precisely because I found that the current one was
> > simpler. [...]
> [...]
> > I'd say that if there is any technical benefit in doing this (occasional
> > code improvement or better support for older or exotic compilers), I'd say
> > "ok go for it", but if it's only a matter of taste, I'm not convinced at
> > all and am rather seeing this as a regression. Now if there's rough
> > consensus around this approach I'll abide, but then I'd request that other
> > archs are adapted as well so that we don't keep a different approach only
> > for these two ones.
> 
> I don't see any technical benefit for x86-64, so I don't think there
> is a need in doing this. Though I personally prefer to use register
> constraints if they exist instead of register variables for everything
> (oh yeah, matter of taste since I don't have any technical argument to
> say it's better respecting the resulting codegen). The only real issue
> is for the syscall6() implementation on i386 as we've been bitten by a
> real compiler issue. In short, I am neutral on this change.

Just to be clear, I usually only use register constraints as well but I
changed this for the syscalls since they were not sufficient, and found
that the mix of the two was really not great to deal with.

Thanks,
Willy

  parent reply	other threads:[~2022-03-24 15:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24  7:30 [PATCH v1 00/11] Add dynamic memory allocator support for nolibc Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 01/11] tools/nolibc: x86-64: Update System V ABI document link Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 02/11] tools/nolibc: Remove .global _start from the entry point code Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 03/11] tools/nolibc: Replace `asm` with `__asm__` Ammar Faizi
2022-03-24  7:41   ` Willy Tarreau
2022-03-24  9:03     ` Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 04/11] tools/nolibc: x86-64: Use appropriate register constraints if exist Ammar Faizi
2022-03-24  7:57   ` Willy Tarreau
2022-03-24  8:33     ` Alviro Iskandar Setiawan
2022-03-24  9:00       ` Ammar Faizi
2022-03-24 15:42       ` Willy Tarreau [this message]
2022-03-24  7:30 ` [PATCH v1 05/11] tools/nolibc: i386: " Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 06/11] tools/nolibc: i386: Implement syscall with 6 arguments Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 07/11] tools/nolibc/sys: Implement `mmap()` and `munmap()` Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 08/11] tools/nolibc/types: Implement `offsetof()` and `container_of()` macro Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 09/11] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()` Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 10/11] tools/nolibc/string: Implement `strnlen()` Ammar Faizi
2022-03-24  7:30 ` [PATCH v1 11/11] tools/include/string: Implement `strdup()` and `strndup()` Ammar Faizi

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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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