917441 - Remove dependence of IonBuilder on ScriptAnalysis

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

With bug 916914 done, removing the remaining dependencies that IonBuilder has on ScriptAnalysis, and using BytecodeAnalysis instead, is fairly straightforward.

Comment 1

[Jan de Mooij [:jandem]]

Comment on attachment 806152 [details] [diff] [review]
patch

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

Very nice, now we only need to port the arguments analysis to IonBuilder and we can rm ScriptAnalysis.

However, Till tried to remove the ScriptAnalysis::arrayWriteHole bit in bug 894948 and it regressed Octane-crypto. See bug 894948 comment 8 for what I *think* caused this. The easiest fix is to add a "write hole" flag to the baseline SetElem IC. We should do that and make sure it doesn't regress Octane-crypto.

::: js/src/jit/IonBuilder.cpp
@@ +7379,2 @@
>          SetElemICInspector icInspect(inspector->setElemICInspector(pc));
>          writeHole |= icInspect.sawOOBDenseWrite();

Nit: s/|=/=

::: js/src/jit/MIRGraph.h
@@ +9,5 @@
>  
>  // This file declares the data structures used to build a control-flow graph
>  // containing MIR.
>  
> +#include "jit/BytecodeAnalysis.h"

Nit: you can forward-declare BytecodeAnalysis since we only use a pointer to it.

::: js/src/vm/Interpreter.cpp
@@ +1225,5 @@
>  static JS_ALWAYS_INLINE bool
>  SetObjectElementOperation(JSContext *cx, Handle<JSObject*> obj, HandleId id, const Value &value,
>                            bool strict, JSScript *maybeScript = NULL, jsbytecode *pc = NULL)
>  {
>      RootedScript script(cx, maybeScript);

Also remove this root + the maybeScript/pc arguments.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: updated

Good catch, that crypto regression was in place and your suggested fix fixes it.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: updated

This time without an unrelated jit-tests change.

Comment 4

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed91189f940e

Comment 5

[Brian Hackett [Laid off!]]

Followup to unbreak --disable-ion builds:

https://hg.mozilla.org/integration/mozilla-inbound/rev/38d8c6c2c223

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/ed91189f940e
https://hg.mozilla.org/mozilla-central/rev/38d8c6c2c223