Improve types at MTableSwitch

RESOLVED FIXED in mozilla43

Status

()

Core
JavaScript Engine: JIT
P5
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: h4writer, Assigned: h4writer, Mentored)

Tracking

unspecified
mozilla43
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Same as "bug 994016" but for switches.
(Assignee)

Comment 1

3 years ago
In our JS Engine we have 3 engines. A basic interpreter, a faster Baseline Compiler and our most advanced compiler IonMonkey.
Executing JS code always starts in the lower engine and when a script gets hot it will go to the Baseline Compiler after which it will go to IonMonkey. This bug is about IonMonkey.

In IonMonkey we get the script (in an intermediate format jsbytecode) and we transform that to another intermediate format (MIR). The last format has some nice properties (SSA) that enables us to easily to optimizations. The step from jsbytecode to MIR happens in something we call "IonBuilder". This bug is about an improvement in the transformation.

The actual improvement is about switches in JS.
We often see code like:

> switch(e) {
>   case 1:
>     foo(e+1);
>   case 2:
>     foo(e+1);
>   case 3:
>     foo(e+1);

But as one can see, we can specialize the e and insert the correct value:

> switch(e) {
>   case 1:
>     foo(1+1);
>   case 2:
>     foo(2+1);
>   case 3:
>     foo(3+1);

As a result IonMonkey will be able to compute the additions already, instead of needing to do that addition every time.

We already have a start of an infrastructure that allows to do some things.

ImproveTypesAtTest
- https://dxr.mozilla.org/mozilla-central/source/js/src/jit/IonBuilder.cpp#3674
This is similar to what is needed here. Instead of changing actual values it changes types:
> if (undefined === b) {
>      // b is now definitely 'undefined'
> }
This specific functions test does some checks on what is inside the "if" and computes the type.


replaceTypeSet
- https://dxr.mozilla.org/mozilla-central/source/js/src/jit/IonBuilder.cpp#3383
This function actually replaces the subject value with a given "type" (TemporaryTypeSet).


For this change we need to do something similar as those two functions, but instead of working on types, it should work with actual values. The switch we are currently targeting is the "tableswitch" or MTableSwitch. (which is a switch statements where all cases are integer and consequent).

If there are questions/suggestions. I'm on #jsapi or #ionmonkey as h4writer
(Assignee)

Updated

3 years ago
Mentor: hv1989
(Assignee)

Comment 2

3 years ago
Created attachment 8660490 [details]
WIP
Assignee: nobody → hv1989
(Assignee)

Comment 3

3 years ago
Created attachment 8661771 [details] [diff] [review]
Optimize the input to the MTableSwitch to a constant in case statement
Attachment #8661771 - Flags: review?(jdemooij)
Comment on attachment 8661771 [details] [diff] [review]
Optimize the input to the MTableSwitch to a constant in case statement

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

Nice.

::: js/src/jit/IonBuilder.cpp
@@ +3387,3 @@
>              tableswitch->addBlock(caseblock);
>  
> +            // Add constant to indicate which case this is.

Nit: add "for use by processNextTableSwitchCase." to the end of this sentence.
Attachment #8661771 - Flags: review?(jdemooij) → review+
Can we land the patch?
Flags: needinfo?(hv1989)
Priority: -- → P5
(Assignee)

Comment 6

2 years ago
This actually already landed. Used the wrong bug number apparently:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef74a17b0963
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(hv1989)
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.