Note: There are a few cases of duplicates in user autocompletion which are being worked on.

IonMonkey: Inline Math.sqrt

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: djvj)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
Assignee: general → evilpies
Assignee: evilpies → general
(Reporter)

Updated

5 years ago
Assignee: general → dvander
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Assignee: dvander → kvijayan
(Assignee)

Comment 1

5 years ago
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]);
}
Attachment #607339 - Flags: review?(dvander)
(Reporter)

Comment 2

5 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

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 3

5 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.