[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VfeZVjv_3r4e1uib0BOfxaKTr6Sx59xiUZr=oQ+z_2Fig@mail.gmail.com>
Date: Wed, 24 Aug 2022 20:54:46 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Yury Norov <yury.norov@...il.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Guenter Roeck <linux@...ck-us.net>,
Dennis Zhou <dennis@...nel.org>,
Russell King <linux@...linux.org.uk>,
Catalin Marinas <catalin.marinas@....com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Alexey Klimov <aklimov@...hat.com>,
Kees Cook <keescook@...omium.org>,
Andy Whitcroft <apw@...onical.com>
Subject: Re: [PATCH v2 3/3] lib/find_bit: optimize find_next_bit() functions
On Wed, Aug 24, 2022 at 4:53 PM Yury Norov <yury.norov@...il.com> wrote:
> On Wed, Aug 24, 2022 at 12:19:05PM +0300, Andy Shevchenko wrote:
> > On Wed, Aug 24, 2022 at 4:56 AM Yury Norov <yury.norov@...il.com> wrote:
...
> > > +#define FIND_NEXT_BIT(EXPRESSION, size, start) \
> > > +({ \
> > > + unsigned long mask, idx, tmp, sz = (size), __start = (start); \
> > > + \
> > > + if (unlikely(__start >= sz)) \
> > > + goto out; \
> > > + \
> > > + mask = word_op(BITMAP_FIRST_WORD_MASK(__start)); \
> > > + idx = __start / BITS_PER_LONG; \
> > > + \
> > > + for (tmp = (EXPRESSION) & mask; !tmp; tmp = (EXPRESSION)) { \
> >
> > for (unsigned long tmp ...;
> > But hey, why not loop over idx (which probably should be named as
> > offset)
>
> Offset in structure, index in array, isn't?
>
> > as I proposed in the first patch? You will drop a lot of
> > divisions / multiplications, no?
>
> Those divisions and multiplications are optimized away, and
> what you suggested blows up the EXPRESSION.
>
> I tried like this:
> mask = word_op(BITMAP_FIRST_WORD_MASK(__start));
> idx = __start / BITS_PER_LONG;
> tmp = (EXPRESSION);
>
> while (1) {
> if (tmp) {
> sz = min(idx * BITS_PER_LONG + __ffs(word_op(tmp)), sz);
> break;
> }
>
> if (++idx > sz)
> break;
>
> tmp = (EXPRESSION);
> }
>
> And it generated the same code, but looks less expressive to me.
> If you have some elegant approach in mind - can you please share
> it, and how the generated code looks?
for (unsigned long idx = 0; idx < sz; idx++) {
unsigned long tmp;
tmp = (EXPRESSION);
if (tmp) {
...
}
}
No?
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists