Re: [RFC PATCH 2/2] treewide, bits: use ffs_val() where it is open-coded

[David Laight]

On Sun, 11 Jan 2026 03:15:22 +0000 (GMT)
"Maciej W. Rozycki" <macro@xxxxxxxxxxx> wrote:

> On Sat, 10 Jan 2026, David Laight wrote:
> 
> > > > > diff --git a/arch/mips/dec/ecc-berr.c b/arch/mips/dec/ecc-berr.c
> > > > > index 1eb356fdd8323..8934b8b1cf375 100644
> > > > > --- a/arch/mips/dec/ecc-berr.c
> > > > > +++ b/arch/mips/dec/ecc-berr.c
> > > > > @@ -153,7 +153,7 @@ static int dec_ecc_be_backend(struct pt_regs *regs, int is_fixup, int invoker)
> > > > >  			/* Ack now, now we've rewritten (or not). */
> > > > >  			dec_ecc_be_ack();
> > > > >  
> > > > > -			if (syn && syn == (syn & -syn)) {
> > > > > +			if (syn && syn == ffs_val(syn)) {    
> > > > 
> > > > It opencodes is_power_of_2().  
> > 
> > Badly...  
> 
>  It's 4 MIPS instructions and exactly the same machine code produced as 
> compiler output from `is_power_of_2'.

The compiler must be detecting that x == (x & -x) is the same as
(x & (x - 1)) == 0.
Although for MIPS the former is negate, and, beq(x,y) - so 3 instructions.
On x86 it is negate, and, cmp, beq(zero flag) - one extra instruction.
(The 4th MIPS instruction will be beq(syn,0).)

So maybe s/Badly/In an unusual way/

> If you know of a better alternative

Doing is_power_of_2() with only one conditional branch would be a gain.
But I've not thought about it hard enough to find one.
I suspect there may not be one, but all these 'tricks' come from the 1970s
(or earlier) and don't allow for the behaviour of modern cpu with multiple
execution units and long pipelines.
(Although MIPS is likely to be relatively sane.)

	David

> I'll be happy to get enlightened, however it's academic anyway, as it's 
> the ECC memory error handler.  If you get here, then you have a bigger 
> issue than a couple of extra instructions executed per kernel log entry 
> produced.  If you indeed can get rid of them in the first place, that is.
> 
>   Maciej
>