[parisc-linux-cvs] Re: 2.6.0-test4-pa12 __fls()

[Grant Grundler]

On Sat, Sep 06, 2003 at 08:31:00PM +0100, Matthew Wilcox wrote:
> Sorry, but it's clearly broken.

yes

...
> > +#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

ouch.

> > +#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
> else
> 	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
	else 
		subtract N

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.

grant