Reviewers: titzer,
Message:
Improves performance of loads from immutable context variables. Assistance
with
this would be appreciated, or perhaps someone would be able to work on it.
Description:
Avoid initialization checks when loading from an immutable context variable.
This patch tries to address the issue by copying some code from the context
specialization reduction. Sorry I am not yet familiar enough with the code
to
really write this, but it does avoid the initialization checks and a phi
that
frustrates further optimization, and it passes the checks, and looks
plausible.
This can make a significant performance improvement to asm.js style code
that
frequently references immutable module variables, and should help JS code in
general. The Odin asm.js implementation optimizes a 'const' module
variable, but
with this patch v8 appears to also optimize an immutable 'var' well.
Further,
Odin compiles before the module variables have been initialized, but with
this
patch V8 can compile knowing the initialized variable values which is a very
interesting feature. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1138132
for a request to add the later support to Odin and asm.js and some
discussion.
BUG=
Please review this at https://codereview.chromium.org/967093002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+23, -3 lines):
M src/compiler/ast-graph-builder.cc
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc
b/src/compiler/ast-graph-builder.cc
index
b6c6e0fb3f5bc1a0f396d178d09931f978332bda..6661b4025cd495e1d3022a5908b1af04c4a2d45f
100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -2595,9 +2595,29 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable*
variable,
const Operator* op =
javascript()->LoadContext(depth, variable->index(), immutable);
Node* value = NewNode(op, current_context());
- // TODO(titzer): initialization checks are redundant for already
- // initialized immutable context loads, but only specialization
knows.
- // Maybe specializer should be a parameter to the graph builder?
+
+ if (immutable) {
+ // TODO(titzer): initialization checks are redundant for already
+ // initialized immutable context loads, but only specialization
knows.
+ // Below copied from context specialization reduction - refactor?
+ // Maybe specializer should be a parameter to the graph builder?
+ HeapObjectMatcher<Context> m(NodeProperties::GetValueInput(value,
0));
+ // If the context is not constant, no reduction can occur.
+ if (m.HasValue()) {
+ // Find the right parent context.
+ Context* context = *m.Value().handle();
+ for (size_t i = depth; i > 0; --i) {
+ context = context->previous();
+ }
+ Handle<Object> value2 = Handle<Object>(
+ context->get(static_cast<int>(variable->index())),
+ jsgraph_->isolate());
+ if (!value2->IsUndefined() && !value2->IsTheHole()) {
+ return value;
+ }
+ }
+ }
+
if (mode == CONST_LEGACY) {
// Perform check for uninitialized legacy const variables.
Node* undefined = jsgraph()->UndefinedConstant();
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
