Clean up and unify DefVar/DefLexical/BindVar implementations

RESOLVED FIXED in Firefox 66

Status

()

enhancement
RESOLVED FIXED
6 months ago
4 months ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(9 attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
Assignee

Description

6 months ago
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.
Assignee

Comment 1

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

Comment 9

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

Depends on D13706

Comment 10

4 months ago
Pushed by jdemooij@mozilla.com:
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
Assignee

Updated

4 months ago
Blocks: 1519378

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

You need to log in before you can comment on or make changes to this bug.