The default bug view has changed. See this FAQ.

IonMonkey: Inline more unary math functions

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
For Kraken darkroom we need Math.log to be fast. On a typical micro-benchmark we're slower than V8.

As a first step we should inline a call to a "double log(MathCache *, double)" helper (and sin, cos, tan). This should be pretty straight-forward and if it does not help much we can try to inline more.
(Assignee)

Comment 1

5 years ago
Created attachment 633179 [details] [diff] [review]
Patch

Inlines Math.{log, sin, cos, tan} by calling a stub. Flagging Marty for the ARM changes.

This patch itself is a small win on Kraken darkroom, but combined with some inlining patches this will allow us to hoist a loop-invariant Math.log and that will help a lot more.
Attachment #633179 - Flags: review?(mrosenberg)
Attachment #633179 - Flags: review?(dvander)
Attachment #633179 - Flags: review?(mrosenberg) → review+
Comment on attachment 633179 [details] [diff] [review]
Patch

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

::: js/src/ion/CodeGenerator.cpp
@@ +1249,5 @@
> +      default:
> +        JS_NOT_REACHED("Unknown math function");
> +    }
> +
> +    masm.callWithABI(funptr, masm.DOUBLE);

Interesting, this doesn't have to be MacroAssembler::DOUBLE?
Attachment #633179 - Flags: review?(dvander) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/c980045c4077

(In reply to David Anderson [:dvander] from comment #2)
> 
> Interesting, this doesn't have to be MacroAssembler::DOUBLE?

Both Clang and GCC 4.2 didn't complain. I changed it to MacroAssembler::DOUBLE though, makes more sense indeed.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.