Bug 144205

Summary: [JSC] Implement Math.clz32(), remove Number.clz()
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, msaboff
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Benjamin Poulain 2015-04-25 19:35:54 PDT
[JSC] Implement Math.clz32(), remove Number.clz()
Comment 1 Benjamin Poulain 2015-04-25 19:47:49 PDT
Created attachment 251664 [details]
Patch
Comment 2 Michael Saboff 2015-04-26 08:15:41 PDT
Comment on attachment 251664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251664&action=review

r=me with one question.

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:194
> +        xor32(TrustedImm32(0x1f), dst);

Why is the xor needed?  Since you protect for the zero case and bsr is spec'ed to return an unsigned value from 0..31, you shouldn't need this xor.
Comment 3 Benjamin Poulain 2015-04-26 11:15:58 PDT
(In reply to comment #2)
> > Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:194
> > +        xor32(TrustedImm32(0x1f), dst);
> 
> Why is the xor needed?  Since you protect for the zero case and bsr is
> spec'ed to return an unsigned value from 0..31, you shouldn't need this xor.

BSR returns the position of the most significant bit, the opposite of what we need.
The xor32 is there to inverse the result.
Comment 4 Benjamin Poulain 2015-04-26 12:56:28 PDT
Comment on attachment 251664 [details]
Patch

Clearing flags on attachment: 251664

Committed r183358: <http://trac.webkit.org/changeset/183358>
Comment 5 Benjamin Poulain 2015-04-26 12:56:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Michael Saboff 2015-04-26 22:04:16 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > > Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:194
> > > +        xor32(TrustedImm32(0x1f), dst);
> > 
> > Why is the xor needed?  Since you protect for the zero case and bsr is
> > spec'ed to return an unsigned value from 0..31, you shouldn't need this xor.
> 
> BSR returns the position of the most significant bit, the opposite of what
> we need.
> The xor32 is there to inverse the result.

Got it.