1271977 - Lift bit shifting and bit counting into the macro-assembler

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Lars T Hansen [:lth]]

This is code that the wasm baseline will need, I'm just factoring existing code and naming it.

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1271977-bit-stuff.patch

Right now this is defined for x86_shared only because that's where the code comes from and that's what I have the ability to test, but the idea will be to evolve this to cover all platforms.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8751265 [details] [diff] [review]
bug1271977-bit-stuff.patch

Review of attachment 8751265 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you! I'm about to do the same for x64...

::: js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp
@@ +797,5 @@
>  CodeGeneratorX86Shared::visitPopcntI(LPopcntI* ins)
>  {
>      Register input = ToRegister(ins->input());
>      Register output = ToRegister(ins->output());
> +    Register temp = ins->temp()->isBogusTemp() ? InvalidReg : ToRegister(ins->temp());

I think you also need to change Lowering.cpp (and extract the lowering of MPopcnt to the platform specific ones).

::: js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h
@@ +86,5 @@
> +    if (!knownNotZero) {
> +        // If the source is zero then bsrl leaves garbage in the destination.
> +        Label nonzero;
> +        j(Assembler::NonZero, &nonzero);
> +        movl(Imm32(0), dest);

xor(0, 0x1F) is 0x1f, but Math.clz32(0) == 32 == 0x20, so this immediate should be 0x3f.

Comment 3

[Lars T Hansen [:lth]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #2)

> ::: js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h
> @@ +86,5 @@
> > +    if (!knownNotZero) {
> > +        // If the source is zero then bsrl leaves garbage in the destination.
> > +        Label nonzero;
> > +        j(Assembler::NonZero, &nonzero);
> > +        movl(Imm32(0), dest);
> 
> xor(0, 0x1F) is 0x1f, but Math.clz32(0) == 32 == 0x20, so this immediate
> should be 0x3f.

And I am adding some more test cases since the test suite does not cover this...

Comment 4

[Lars T Hansen [:lth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ccf7dc442e1fea2b7ae1c1f28a6c75cbd54a3102
Bug 1271977 - bit shifting, bit counting. r=bbouvier

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ccf7dc442e1f