[parisc-linux-cvs] Re: 2.6.0-test4-pa12 __fls()
Sun, 7 Sep 2003 21:21:21 -0600
On Sat, Sep 06, 2003 at 08:31:00PM +0100, Matthew Wilcox wrote:
> Sorry, but it's clearly broken.
> > +#if BITS_PER_LONG > 32
> > + " extrd,u,*<> %0,63,32,%%r0\n"
> if any of the bottom 32 bits are set ...
This line of code was clearly wrong.
Should have read "%0,31,32"
> > + " depd,*TR %0,31,32,%0\n"
> move the bottom 32 bits up into the top 32 bits
This moves the top 32-bits to the bottom 32-bits.
> > + " addi 32,%1,%1\n"
> otherwise add 32
> > +#endif
> ... and then do things that can't see the top 32 bits.
right. the intent was to make the upper 32-bits visible if anything
was set. narrow/wide code could share the rest of the __asm__().
This doesn't seem feasible though.
> > + " extru,<> %0,15,16,%%r0\n"
> > + " zdep,TR %0,15,16,%0\n"
> > + " addi 16,%1,%1\n"
> think you could put in comments similar to the endian swapping?
Yes - I will if they are not there already.
> seems to me this whole thing should be done as
> if (any top N bits set)
> shift N bits left
> subtract N
This doesn't sound right. The shift left seems to be clobbering the
(N-1) set of bits we want to test next.
I think it should be:
if (any top N bits set)
shift N right
where top "N bits" is bits 16-31 for N=16, 8-15 for N=8, ...
> > + " extru,<> %0,7,8,%%r19\n"
> r19? That's not mentioned as being clobbered. I think you mean r0.
Yes - this is complete crap - I'm embarrassed that I overlooked it. /o\
I recall checking the first 3 instructions and decided the rest
should be correct (NOT!).
> we don't need an __fls, so implementing an fls that works directly would
> be better.
well, my hope was the __fls() would be useful for resource bitmaps.
But I'm still not exactly sure how. The current code in sba has
constraints on alignment so it may not be useful.
I won't miss it you take it out.
Anyway, thanks for pointing out the bloody obvious (and the less
obvious) and fixing it.