Re: [PATCH v3 1/5] lib/bitmap: add bitmap_{set,get}_value()

On Mon, Jul 17, 2023 at 05:29:12PM +0300, Andy Shevchenko wrote:

On Mon, Jul 17, 2023 at 04:14:57PM +0200, Alexander Potapenko wrote:

...

quoted

quoted

quoted

+             map[index] &= ~(GENMASK(nbits + offset - 1, offset));

I remember that this construction may bring horrible code on some architectures
with some version(s) of the compiler (*).

Wow, even the trunk Clang and GCC seem to generate better code for
your version of this line: https://godbolt.org/z/36Kqxhe6j

Wow, indeed! Perhaps time to report to clang and GCC people. I believe the root
cause is that in the original version compiler can't prove that l is constant
for GENMASK().

quoted

quoted

To fix that I found an easy refactoring:

                map[index] &= ~(GENMASK(nbits, 0) << offset));

nbits - 1 it should be, btw. In any case it seems the code is still better.

quoted

I'll take this one.

quoted

(*) don't remember the actual versions, though, but anyway...

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel