Closed
Bug 735391
Opened 12 years ago
Closed 12 years ago
IonMonkey: Inline Math.sqrt
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: dvander, Assigned: djvj)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
6.96 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•12 years ago
|
Assignee: general → evilpies
Updated•12 years ago
|
Assignee: evilpies → general
Reporter | ||
Updated•12 years ago
|
Assignee: general → dvander
Status: NEW → ASSIGNED
Assignee | ||
Updated•12 years ago
|
Assignee: dvander → kvijayan
Assignee | ||
Comment 1•12 years ago
|
||
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]); }
Attachment #607339 -
Flags: review?(dvander)
Reporter | ||
Comment 2•12 years ago
|
||
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.
Attachment #607339 -
Flags: review?(dvander) → review+
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 3•12 years ago
|
||
I don't see a fix for this or a link to the mercurial commit. Has this optimization been applied?
You need to log in
before you can comment on or make changes to this bug.
Description
•