[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220615140030.1265068-1-alexandr.lobakin@intel.com>
Date: Wed, 15 Jun 2022 16:00:30 +0200
From: Alexander Lobakin <alexandr.lobakin@...el.com>
To: Yury Norov <yury.norov@...il.com>
Cc: Alexander Lobakin <alexandr.lobakin@...el.com>,
Arnd Bergmann <arnd@...db.de>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Mark Rutland <mark.rutland@....com>,
Matt Turner <mattst88@...il.com>,
Brian Cain <bcain@...cinc.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Yoshinori Sato <ysato@...rs.sourceforge.jp>,
Rich Felker <dalias@...c.org>,
"David S. Miller" <davem@...emloft.net>,
Kees Cook <keescook@...omium.org>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
Marco Elver <elver@...gle.com>, Borislav Petkov <bp@...e.de>,
Tony Luck <tony.luck@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-alpha@...r.kernel.org, linux-hexagon@...r.kernel.org,
linux-ia64@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/6] bitops: let optimize out non-atomic bitops on compile-time constants
From: Yury Norov <yury.norov@...il.com>
Date: Tue, 14 Jun 2022 20:26:54 -0700
> Hi Alexander,
>
> On Fri, Jun 10, 2022 at 01:34:27PM +0200, Alexander Lobakin wrote:
> > Currently, many architecture-specific non-atomic bitop
> > implementations use inline asm or other hacks which are faster or
> > more robust when working with "real" variables (i.e. fields from
> > the structures etc.), but the compilers have no clue how to optimize
> > them out when called on compile-time constants. That said, the
> > following code:
> >
> > DECLARE_BITMAP(foo, BITS_PER_LONG) = { }; // -> unsigned long foo[1];
> > unsigned long bar = BIT(BAR_BIT);
> > unsigned long baz = 0;
> >
> > __set_bit(FOO_BIT, foo);
> > baz |= BIT(BAZ_BIT);
> >
> > BUILD_BUG_ON(!__builtin_constant_p(test_bit(FOO_BIT, foo));
> > BUILD_BUG_ON(!__builtin_constant_p(bar & BAR_BIT));
> > BUILD_BUG_ON(!__builtin_constant_p(baz & BAZ_BIT));
>
> Can you put this snippet into lib/test_bitops.c?
Great idea, sure!
>
> Thanks,
> Yury
>
> > triggers the first assertion on x86_64, which means that the
> > compiler is unable to evaluate it to a compile-time initializer
> > when the architecture-specific bitop is used even if it's obvious.
> > In order to let the compiler optimize out such cases, expand the
> > bitop() macro to use the "constant" C non-atomic bitop
> > implementations when all of the arguments passed are compile-time
> > constants, which means that the result will be a compile-time
> > constant as well, so that it produces more efficient and simple
> > code in 100% cases, comparing to the architecture-specific
> > counterparts.
> >
> > The savings are architecture, compiler and compiler flags dependent,
> > for example, on x86_64 -O2:
> >
> > GCC 12: add/remove: 78/29 grow/shrink: 332/525 up/down: 31325/-61560 (-30235)
> > LLVM 13: add/remove: 79/76 grow/shrink: 184/537 up/down: 55076/-141892 (-86816)
> > LLVM 14: add/remove: 10/3 grow/shrink: 93/138 up/down: 3705/-6992 (-3287)
> >
> > and ARM64 (courtesy of Mark):
> >
> > GCC 11: add/remove: 92/29 grow/shrink: 933/2766 up/down: 39340/-82580 (-43240)
> > LLVM 14: add/remove: 21/11 grow/shrink: 620/651 up/down: 12060/-15824 (-3764)
> >
> > Cc: Mark Rutland <mark.rutland@....com>
> > Signed-off-by: Alexander Lobakin <alexandr.lobakin@...el.com>
> > ---
> > include/linux/bitops.h | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index 753f98e0dcf5..364bdc3606b4 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -33,8 +33,24 @@ extern unsigned long __sw_hweight64(__u64 w);
> >
> > #include <asm-generic/bitops/generic-non-atomic.h>
> >
> > +/*
> > + * Many architecture-specific non-atomic bitops contain inline asm code and due
> > + * to that the compiler can't optimize them to compile-time expressions or
> > + * constants. In contrary, gen_*() helpers are defined in pure C and compilers
> > + * optimize them just well.
> > + * Therefore, to make `unsigned long foo = 0; __set_bit(BAR, &foo)` effectively
> > + * equal to `unsigned long foo = BIT(BAR)`, pick the generic C alternative when
> > + * the arguments can be resolved at compile time. That expression itself is a
> > + * constant and doesn't bring any functional changes to the rest of cases.
> > + * The casts to `uintptr_t` are needed to mitigate `-Waddress` warnings when
> > + * passing a bitmap from .bss or .data (-> `!!addr` is always true).
> > + */
> > #define bitop(op, nr, addr) \
> > - op(nr, addr)
> > + ((__builtin_constant_p(nr) && \
> > + __builtin_constant_p((uintptr_t)(addr) != (uintptr_t)NULL) && \
> > + (uintptr_t)(addr) != (uintptr_t)NULL && \
> > + __builtin_constant_p(*(const unsigned long *)(addr))) ? \
> > + const##op(nr, addr) : op(nr, addr))
> >
> > #define __set_bit(nr, addr) bitop(___set_bit, nr, addr)
> > #define __clear_bit(nr, addr) bitop(___clear_bit, nr, addr)
> > --
> 2.36.1
Thanks,
Olek
Powered by blists - more mailing lists