Change https://golang.org/cl/229127 mentions this issue: cmd/compile: use cheaper implementation of oneBit

I guess you don't like doing it earlier in the compiler before it reaches SSA? Probably cost outweighs the benefits, and clutters earlier phases?

cost outweighs the benefits, and clutters earlier phases?

Exactly.

Change https://golang.org/cl/229197 mentions this issue: cmd/compile: use cheaper implementation of oneBit

As a side, but related note, at least with GOAMD64=v2 and above there is no CPU flag check, so we end up with this code instead:

XORL	CX, CX
POPCNTQ	AX, CX
CMPQ	CX, $1
SETEQ	AL

No call to math/bits.OnesCount(SB) is emited.
For the completeness, the proposed replacement pattern x&(x-1) != 0 && x != 0 is compiled as:

LEAQ	-1(AX), CX
TESTQ	CX, AX
JEQ	17
TESTQ	AX, AX
SETNE	CL
JMP	19
XORL	CX, CX
MOVL	CX, AX

Tested on Go tip (well, not the latest one, but close enough: fad67f8).

That replacement compiled code should have no jumps in it. We must be missing a rewrite rule or three.

OK, sorry for bumping this up again, but I was staring at the originally proposed replacement and it doesn't look 100% correct.
Or maybe I miscalculated something.

Perhaps it should look like this?

- ((x&(x-1)) != 0) && x != 0
+ ((x&(x-1)) == 0) && x != 0

With that in mind, here is a comparison with different compilers output.
Clang (https://godbolt.org/z/o9PzM49TG):

        lea     eax, [rdi - 1]
        test    edi, eax
        sete    cl
        test    edi, edi
        setne   al
        and     al, cl
        ret

GCC:

        lea     eax, [rdi-1]
        test    eax, edi
        sete    al
        test    edi, edi
        setne   dl
        and     eax, edx
        ret

Go (https://godbolt.org/z/nfKjKe93a):

        LEAQ    -1(AX), CX
        TESTQ   CX, AX
        JEQ     f_pc17
        TESTQ   AX, AX
        SETNE   CL
        JMP     f_pc19
f_pc17:
        XORL    CX, CX
f_pc19:
        MOVL    CX, AX
        RET

It looks like Go should be able to remove some branches here too.