Bug 88314 – Improve Math.round and Math.floor intrinsic

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yuqiang Xian]

Currently we call a native function from the JIT code to complete the "round" and "floor" operations. We could inline some fast paths especially for those positive values on the platforms where floating point truncation is supported.

[Yuqiang Xian]

Created attachment 145729 [details]
patch

[Yuqiang Xian]

Created attachment 145730 [details]
Performance result

~3% improvement on Kraken (32% on audio-oscillator), and nearly 1% on SunSpider.

[Yuqiang Xian]

(In reply to comment #2)
> Created an attachment (id=145730) [details]
> Performance result
> 
> ~3% improvement on Kraken (32% on audio-oscillator), and nearly 1% on SunSpider.

o... for clarification, overall SS improvement is not obvious, but definitely 9% on one case.

[Filip Pizlo]

Comment on attachment 145729 [details]
patch

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

This looks right.  I'm curious what your thoughts are about the comparing against neg zero.  Maybe this code would work the same if you just used zero instead of neg zero, which would be faster, since regular zero is easier to load in SSE?

Not marking cq+ just because I want to give you an opportunity to respond.  But I think the code is correct so if there's nothing that needs changing then I'm fine with this being landed as-is.

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:197
> +        doubleResult.append(jit.branchDouble(MacroAssembler::DoubleEqual, SpecializedThunkJIT::fpRegT0, SpecializedThunkJIT::fpRegT1));

Is this right?  Double equality has strange handling of zero.  0 == -0 for example.

[Yuqiang Xian]

(In reply to comment #4)
> (From update of attachment 145729 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145729&action=review
> 
> This looks right.  I'm curious what your thoughts are about the comparing against neg zero.  Maybe this code would work the same if you just used zero instead of neg zero, which would be faster, since regular zero is easier to load in SSE?
> 
> Not marking cq+ just because I want to give you an opportunity to respond.  But I think the code is correct so if there's nothing that needs changing then I'm fine with this being landed as-is.
> 
> > Source/JavaScriptCore/jit/ThunkGenerators.cpp:197
> > +        doubleResult.append(jit.branchDouble(MacroAssembler::DoubleEqual, SpecializedThunkJIT::fpRegT0, SpecializedThunkJIT::fpRegT1));
> 
> Is this right?  Double equality has strange handling of zero.  0 == -0 for example.

You're right. It should be 0.

[Yuqiang Xian]

Landed with Filip's comments addressed: http://trac.webkit.org/changeset/119558