Date: Mon, 2 Mar 2020 09:13:53 -0800 From: Ryan Libby <rlibby@freebsd.org> To: "Alexander V. Chernikov" <melifaro@ipfw.ru> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r358439 - head/sys/amd64/include Message-ID: <CAHgpiFzg1Co6KY2twE3Pa7-ozQti4kzMr432R_7w9hukMQU00A@mail.gmail.com> In-Reply-To: <5767791583138727@sas1-c7aad230fe87.qloud-c.yandex.net> References: <202002281832.01SIWaEL071685@repo.freebsd.org> <5767791583138727@sas1-c7aad230fe87.qloud-c.yandex.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Mar 2, 2020 at 12:45 AM Alexander V. Chernikov <melifaro@ipfw.ru> wrote: > > 28.02.2020, 18:32, "Ryan Libby" <rlibby@freebsd.org>: > > Author: rlibby > > Date: Fri Feb 28 18:32:36 2020 > > New Revision: 358439 > > URL: https://svnweb.freebsd.org/changeset/base/358439 > > > > Log: > > amd64 atomic.h: minor codegen optimization in flag access > > > > Previously the pattern to extract status flags from inline assembly > > blocks was to use setcc in the block to write the flag to a register. > > This was suboptimal in a few ways: > > - It would lead to code like: sete %cl; test %cl; jne, i.e. a flag > > would just be loaded into a register and then reloaded to a flag. > > - The setcc would force the block to use an additional register. > > - If the client code didn't care for the flag value then the setcc > > would be entirely pointless but could not be eliminated by the > > optimizer. > > > > A more modern inline asm construct (since gcc 6 and clang 9) allows for > This effectively restricts kernel builds by all older compilers. > Is there any chance of making it conditional depending on the compiler version/features? Yes, it is possible to test for __GCC_ASM_FLAG_OUTPUTS__. It is more maintenance effort going forward. If building current with an old cross compiler is an important scenario, we can either revert this and the following revision or work up a patch to make it conditional. I'll see what that might look like. > > "flag output operands", where a C variable can be written directly from > > a flag. The optimizer can then use this to produce direct code where > > the flag does not take a trip through a register. > > > > In practice this makes each affected operation sequence shorter by five > > bytes of instructions. It's unlikely this has a measurable performance > > impact. > > > > Reviewed by: kib, markj, mjg > > Sponsored by: Dell EMC Isilon > > Differential Revision: https://reviews.freebsd.org/D23869 > > > > Modified: > > head/sys/amd64/include/atomic.h > > > > Modified: head/sys/amd64/include/atomic.h > > ============================================================================== > > --- head/sys/amd64/include/atomic.h Fri Feb 28 17:41:46 2020 (r358438) > > +++ head/sys/amd64/include/atomic.h Fri Feb 28 18:32:36 2020 (r358439) > > @@ -201,9 +201,8 @@ atomic_cmpset_##TYPE(volatile u_##TYPE *dst, u_##TYPE > > __asm __volatile( \ > > " " MPLOCKED " " \ > > " cmpxchg %3,%1 ; " \ > > - " sete %0 ; " \ > > "# atomic_cmpset_" #TYPE " " \ > > - : "=q" (res), /* 0 */ \ > > + : "=@cce" (res), /* 0 */ \ > > "+m" (*dst), /* 1 */ \ > > "+a" (expect) /* 2 */ \ > > : "r" (src) /* 3 */ \ > > @@ -219,9 +218,8 @@ atomic_fcmpset_##TYPE(volatile u_##TYPE *dst, u_##TYPE > > __asm __volatile( \ > > " " MPLOCKED " " \ > > " cmpxchg %3,%1 ; " \ > > - " sete %0 ; " \ > > "# atomic_fcmpset_" #TYPE " " \ > > - : "=q" (res), /* 0 */ \ > > + : "=@cce" (res), /* 0 */ \ > > "+m" (*dst), /* 1 */ \ > > "+a" (*expect) /* 2 */ \ > > : "r" (src) /* 3 */ \ > > @@ -278,9 +276,8 @@ atomic_testandset_int(volatile u_int *p, u_int v) > > __asm __volatile( > > " " MPLOCKED " " > > " btsl %2,%1 ; " > > - " setc %0 ; " > > "# atomic_testandset_int" > > - : "=q" (res), /* 0 */ > > + : "=@ccc" (res), /* 0 */ > > "+m" (*p) /* 1 */ > > : "Ir" (v & 0x1f) /* 2 */ > > : "cc"); > > @@ -295,9 +292,8 @@ atomic_testandset_long(volatile u_long *p, u_int v) > > __asm __volatile( > > " " MPLOCKED " " > > " btsq %2,%1 ; " > > - " setc %0 ; " > > "# atomic_testandset_long" > > - : "=q" (res), /* 0 */ > > + : "=@ccc" (res), /* 0 */ > > "+m" (*p) /* 1 */ > > : "Jr" ((u_long)(v & 0x3f)) /* 2 */ > > : "cc"); > > @@ -312,9 +308,8 @@ atomic_testandclear_int(volatile u_int *p, u_int v) > > __asm __volatile( > > " " MPLOCKED " " > > " btrl %2,%1 ; " > > - " setc %0 ; " > > "# atomic_testandclear_int" > > - : "=q" (res), /* 0 */ > > + : "=@ccc" (res), /* 0 */ > > "+m" (*p) /* 1 */ > > : "Ir" (v & 0x1f) /* 2 */ > > : "cc"); > > @@ -329,9 +324,8 @@ atomic_testandclear_long(volatile u_long *p, u_int v) > > __asm __volatile( > > " " MPLOCKED " " > > " btrq %2,%1 ; " > > - " setc %0 ; " > > "# atomic_testandclear_long" > > - : "=q" (res), /* 0 */ > > + : "=@ccc" (res), /* 0 */ > > "+m" (*p) /* 1 */ > > : "Jr" ((u_long)(v & 0x3f)) /* 2 */ > > : "cc");
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHgpiFzg1Co6KY2twE3Pa7-ozQti4kzMr432R_7w9hukMQU00A>