On Mon, Jul 17, 2023 at 05:31:44PM +0300, Andy Shevchenko wrote:

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

quoted

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

...

quoted

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.

 
 Yep.

 Alexander, for the next round can you please show disassembly for the
 functions in case of compile-time and run-time defined start and nbits?


 Thanks,
 Yury

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