Last Comment Bug 735391 - IonMonkey: Inline Math.sqrt
: IonMonkey: Inline Math.sqrt
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Kannan Vijayan [:djvj]
:
Mentors:
Depends on:
Blocks: IonSpeed
  Show dependency treegraph
 
Reported: 2012-03-13 12:33 PDT by David Anderson [:dvander]
Modified: 2012-03-28 13:24 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (6.96 KB, patch)
2012-03-19 15:11 PDT, Kannan Vijayan [:djvj]
dvander: review+
Details | Diff | Splinter Review

Description David Anderson [:dvander] 2012-03-13 12:33:19 PDT
Should be straightforward, we can re-use the native inlining infrastructure for Math.abs and friends. Math.sqrt can be implemented using sqrtsd on x86/x64 and vsqrt on ARM.
Comment 1 Kannan Vijayan [:djvj] 2012-03-19 15:11:31 PDT
Created attachment 607339 [details] [diff] [review]
Patch v1

About a 27% speed improvement (avg across 10 runs) on the following
microbench on x86-64:

function foo(j) {
    return Math.sqrt(j) + Math.sqrt(j + 2);
}
var arr = [];
for(var i = 0; i < 50000000; i++) {
    arr.push(foo(i * 0.95));
}
for(var i = 1; i < arr.length; i *= 2) {
    print(arr[i]);
}
Comment 2 David Anderson [:dvander] 2012-03-21 10:35:10 PDT
Comment on attachment 607339 [details] [diff] [review]
Patch v1

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

::: js/src/ion/MCallOptimize.cpp
@@ +148,5 @@
> +                if (arg1Type == MIRType_Int32) {
> +                    MToDouble *conv = MToDouble::New(input);
> +                    current->add(conv);
> +                    current->push(conv);
> +                    input = conv;

Here, I would not do this conversion. Instead just pass the input wholesale and the type conversion process will do the rest.

::: js/src/ion/MIR.h
@@ +1912,5 @@
> +{
> +    MSqrt(MDefinition *num)
> +      : MUnaryInstruction(num)
> +    {
> +        JS_ASSERT(num->type() == MIRType_Double);

Here, I would set specialization_ = MIRType_Double instead of this assert. Then ArithPolicy will guarantee conversion for you.
Comment 3 Reece H. Dunn 2012-03-28 13:24:54 PDT
I don't see a fix for this or a link to the mercurial commit. Has this optimization been applied?

Note You need to log in before you can comment on or make changes to this bug.