Messages in this thread |  | | | Date | Tue, 13 May 2008 09:11:11 -0700 | | From | Andrew Morton <> | | Subject | Re: [PATCH] asm-generic/bitops/fls64.h |
| |
On Tue, 13 May 2008 16:35:25 +0200 Andreas Schwab <schwab@suse.de> wrote:
> Russell King <rmk+lkml@arm.linux.org.uk> writes: > > > On Tue, May 13, 2008 at 03:24:13PM +0200, Alexander van Heukelum wrote: > >> On Tue, 13 May 2008 16:29:04 +0400, "Nickolay Vinogradov" > >> <nickolay@protei.ru> said: > >> > Alexander van Heukelum пишет: > >> > > >> > > Hi Nickolay, > >> > > > >> > > The change is ok, I guess, but the cast should be a no-op (fls > >> > > takes an int, which is always 32 bit in linux). What is the problem > >> > > you are seeing? Does fls64() return a wrong value in some cases? If > >> > > so, what cpu? Which values? > >> > > > >> > > Why would this be a bug on big endian systems only? There is no > >> > > pointer magic involved, so the compiler should take care of the > >> > > casts in a correct way. > >> > > > >> > > Maybe you see a compiler warning? Which compiler version? > >> > > > >> > > (also note that current (development) kernels now have separate > >> > > versions for 32-bit and 64-bit environments.) > >> > > >> > Because fls() is a macro for asm-arm: > >> > > >> > #define fls(x) \ > >> > ( __builtin_constant_p(x) ? constant_fls(x) : \ > >> > ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); > >> > 32-__r; }) ) > >> > > >> > We can fix it right here: > > > > No. "fls" is for finding the last set bit in an _int_. It is not > > supposed to have random crap passed to it, such as types longer than > > sizeof(int). > > If you write fls as an (inline) function then the argument is implicitly > converted. >
Yes, and that's what other architectures do.
It's not pretty, but I do think the arm implementation should zap the upper 32 bits as other architectures do, and as one would expect from the usual C type conversion rules.
Converting it to a C implementation would be a suitable means...
|  |