1511891 - Clean up and unify DefVar/DefLexical/BindVar implementations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

In jit/VMFunctions.* we have BindVar/DefVar/DefLexical functions that behave like the interpreter versions, but there's no good reason to have the logic duplicated there.

I have a stack of patches to clean this up and unify as BindVarOperation, DefVarOperation, DefLexicalOperation that we call from interpreter, Baseline, Ion.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 1 - Add BindVarOperation and use it for JSOP_BINDVAR in interpreter and JITs. r?tcampbell

This also adds a GetVariablesObject helper so we don't have to duplicate the
logic there.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 2 - Move DefVarOperation from Interpreter-inl.h to Interpreter.cpp. r?tcampbell

No change in behavior.

Depends on D13698

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 3 - Refactor DefVarOperation so interpreter and JITs can call it directly. r?tcampbell

Depends on D13700

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 4 - Move DefLexicalOperation from Interpreter-inl.h to Interpreter.cpp. r?tcampbell

Depends on D13701

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 5 - Refactor DefLexicalOperation to make it easier to call directly from JIT code in the next patch. r?tcampbell

Depends on D13702

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 6 - Merge two DefLexicalOperation functions into one and have the JITs call it directly. r?tcampbell

Depends on D13703

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 7 - Change jit::CheckGlobalOrEvalDeclarationConflicts signature to make it less Baseline-specific. r?tcampbell

Depends on D13704

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 8 - Move CheckGlobalOrEvalDeclarationConflicts to EnvironmentObject.cpp and call it also in the interpreter. r?tcampbell

Depends on D13705

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1511891 part 9 - Call recordreplay::AdvanceExecutionProgressCounter also for eval frames in InterpreterFrame::prologue. r?bhackett

I think this is the right thing to do but I'm not sure.

Depends on D13706

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/582477c043dd
part 1 - Add BindVarOperation and use it for JSOP_BINDVAR in interpreter and JITs. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/3523e71282fd
part 2 - Move DefVarOperation from Interpreter-inl.h to Interpreter.cpp. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/452034cea923
part 3 - Refactor DefVarOperation so interpreter and JITs can call it directly. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8fccd1861a22
part 4 - Move DefLexicalOperation from Interpreter-inl.h to Interpreter.cpp. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/c6e9f5fd0ee7
part 5 - Refactor DefLexicalOperation to make it easier to call directly from JIT code in the next patch. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/745cbd9ae616
part 6 - Merge two DefLexicalOperation functions into one and have the JITs call it directly. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/0df13a6c64be
part 7 - Change jit::CheckGlobalOrEvalDeclarationConflicts signature to make it less Baseline-specific. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/4e1a2cc33577
part 8 - Move CheckGlobalOrEvalDeclarationConflicts to EnvironmentObject.cpp and call it also in the interpreter. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d7c3229eb734
part 9 - Call recordreplay::AdvanceExecutionProgressCounter also for eval frames in InterpreterFrame::prologue. r=bhackett

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/582477c043dd
https://hg.mozilla.org/mozilla-central/rev/3523e71282fd
https://hg.mozilla.org/mozilla-central/rev/452034cea923
https://hg.mozilla.org/mozilla-central/rev/8fccd1861a22
https://hg.mozilla.org/mozilla-central/rev/c6e9f5fd0ee7
https://hg.mozilla.org/mozilla-central/rev/745cbd9ae616
https://hg.mozilla.org/mozilla-central/rev/0df13a6c64be
https://hg.mozilla.org/mozilla-central/rev/4e1a2cc33577
https://hg.mozilla.org/mozilla-central/rev/d7c3229eb734

Comment 12

[Ionuț Goldan [:igoldan]]

Perf improvement noticed! \0/

== Change summary for alert #18702 (as of Fri, 11 Jan 2019 11:22:36 GMT) ==

Improvements:

3% raptor-assorted-dom-firefox linux64 opt 25.69 -> 24.88

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=18702