1353358 - Support JSOP_OPTIMIZE_SPREADCALL in Ion

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[Ted Campbell [:tcampbell]]

This is a follow-up to bug 1338920.

When we see spreadcalls in source that have a single expanded array, we sometimes try to optimize this by voiding intermediate arguments array construction. See |BytecodeEmitter::emitOptimizeSpread|. We would like to support this in Ion.

The current problem is that IonControlFlow does not recognize the branch pattern emitted into the bytecode and asserts out.

Comment 1

[Ted Campbell [:tcampbell]]

Likely approach will be to ensure we emit proper SrcNotes for the emitOptimizeSpread case and try to make it behave like a normal If-Then branch.

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1353358 - Use IfThenElseEmitter when optimizing spreadcall

Review commit: https://reviewboard.mozilla.org/r/131694/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131694/

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1353358 - Support JSOP_OPTIMIZE_SPREADCALL in Ion

Review commit: https://reviewboard.mozilla.org/r/131696/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131696/

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8859683 [details]
Bug 1353358 - Use IfThenElseEmitter when optimizing spreadcall

https://reviewboard.mozilla.org/r/131694/#review135208

Nice

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8859684 [details]
Bug 1353358 - Support JSOP_OPTIMIZE_SPREADCALL in Ion

https://reviewboard.mozilla.org/r/131696/#review135212

::: js/src/jit/IonBuilder.cpp:2328
(Diff revision 1)
>          return Ok();
>  
> +      case JSOP_OPTIMIZE_SPREADCALL:
> +      {
> +        // Assuming optimization isn't available doesn't affect correctness.
> +        // TODO: Investigate dynamic checks.

Please file a follow-up bug. We could check the shape of Array.prototype etc and branch to the slow path if it changed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd25c2c275fc
Use IfThenElseEmitter when optimizing spreadcall r=jandem
https://hg.mozilla.org/integration/autoland/rev/077dfe082524
Support JSOP_OPTIMIZE_SPREADCALL in Ion r=jandem

Comment 7

[Ted Campbell [:tcampbell]]

Follow-up Bug 1358501

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fd25c2c275fc
https://hg.mozilla.org/mozilla-central/rev/077dfe082524