911834 - OdinMonkey: create phis with a type directly

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Since the type of a slot can't change in asm.js and the types are known up-front, we can avoid the analysis in ApplyTypeInformation and just create the phis with the correct resultType immediately (just like what happens with all the other MIR nodes).  This saves a bit more than half a second of total asm.js compile time on a certain large asm.js app on a Nexus 4.

Comment 1

[Luke Wagner [:luke]]

Attachment: remove dead store

While I was looking, I noticed that this setSlot shouldn't be necessary.

Comment 2

[Luke Wagner [:luke]]

Attachment: create phis with types

This patch simply propagates the types of predecessor MDefinitions into the MPhi.  With this change, no MIR nodes created by Odin should have MIRType_Value.  The main question is whether to add extra parameters/branches to the existing paths or whether to clone.  In two cases where I felt the extra branches might have affected perf and made the methods harder to read, I cloned and in the third case I added a branch.  Let me know if you think they'd be better the other way, though.

Comment 3

[Luke Wagner [:luke]]

Attachment: disable ApplyTypeInformation when compilingAsmJS

Comment 4

[Luke Wagner [:luke]]

Comment on attachment 798611 [details] [diff] [review]
remove dead store

Oops, this is totally not a dead store.

Comment 5

[Luke Wagner [:luke]]

Attachment: create phis with types

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 798613 [details] [diff] [review]
disable ApplyTypeInformation when compilingAsmJS

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

::: js/src/jit/Ion.cpp
@@ +1228,5 @@
>      if (mir->shouldCancel("Phi reverse mapping"))
>          return false;
>  
> +    if (!mir->compilingAsmJS()) {
> +        // This pass also removes copies.

Nit: can you remove this comment while you're here? Copies (MCopy) were removed a long time ago (bug 732862).

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 798622 [details] [diff] [review]
create phis with types

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

Looks good.

Comment 8

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45e1927c9136
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed2a7239fc36

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/45e1927c9136
https://hg.mozilla.org/mozilla-central/rev/ed2a7239fc36