Reviewers: Benedikt Meurer, mvstanton,
Message:
Benedikt: PTAL.
Michael: FYI.
https://codereview.chromium.org/1198263004/diff/1/src/compiler/ast-graph-builder.h
File src/compiler/ast-graph-builder.h (right):
https://codereview.chromium.org/1198263004/diff/1/src/compiler/ast-graph-builder.h#newcode101
src/compiler/ast-graph-builder.h:101: // TODO(mstarzinger): Can we let
GVN take care of that?
Is there particular reason why machine loads are not marked as
kIdempotent?
Description:
[turbofan] Avoid embedding type feedback vector into code.
[email protected]
Please review this at https://codereview.chromium.org/1198263004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+48, -35 lines):
M src/compiler/ast-graph-builder.h
M src/compiler/ast-graph-builder.cc
M test/cctest/compiler/test-run-jscalls.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
44f2276ee6513231af2bcdd29fa6023323fbcd52..4b6e243e3e8abe0e2f526d7f0ee2bf40f07baa0b
100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -479,16 +479,6 @@ Node* AstGraphBuilder::GetFunctionClosure() {
}
-Node* AstGraphBuilder::GetFeedbackVector() {
- if (!feedback_vector_.is_set()) {
- Node* vector =
-
jsgraph()->Constant(handle(info()->shared_info()->feedback_vector()));
- feedback_vector_.set(vector);
- }
- return feedback_vector_.get();
-}
-
-
void AstGraphBuilder::CreateFunctionContext(bool constant_context) {
function_context_.set(constant_context
? jsgraph()->HeapConstant(info()->context())
@@ -3293,7 +3283,7 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable*
variable,
uint32_t check_bitset = ComputeBitsetForDynamicGlobal(variable);
const Operator* op = javascript()->LoadDynamicGlobal(
name, check_bitset, feedback, contextual_mode);
- value = NewNode(op, GetFeedbackVector(), current_context());
+ value = NewNode(op, BuildLoadFeedbackVector(), current_context());
states.AddToNode(value, bailout_id, combine);
} else if (mode == DYNAMIC_LOCAL) {
Variable* local = variable->local_if_not_shadowed();
@@ -3317,7 +3307,7 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable*
variable,
uint32_t check_bitset = DynamicGlobalAccess::kFullCheckRequired;
const Operator* op = javascript()->LoadDynamicGlobal(
name, check_bitset, feedback, contextual_mode);
- value = NewNode(op, GetFeedbackVector(), current_context());
+ value = NewNode(op, BuildLoadFeedbackVector(), current_context());
states.AddToNode(value, bailout_id, combine);
}
return value;
@@ -3491,8 +3481,8 @@ static inline Node* Record(JSTypeFeedbackTable*
js_type_feedback, Node* node,
Node* AstGraphBuilder::BuildKeyedLoad(Node* object, Node* key,
const VectorSlotPair& feedback) {
const Operator* op = javascript()->LoadProperty(feedback);
- return Record(js_type_feedback_,
- NewNode(op, object, key, GetFeedbackVector()),
feedback.slot());
+ Node* node = NewNode(op, object, key, BuildLoadFeedbackVector());
+ return Record(js_type_feedback_, node, feedback.slot());
}
@@ -3501,8 +3491,8 @@ Node* AstGraphBuilder::BuildNamedLoad(Node* object,
Handle<Name> name,
ContextualMode mode) {
const Operator* op =
javascript()->LoadNamed(MakeUnique(name), feedback, mode);
- return Record(js_type_feedback_, NewNode(op, object,
GetFeedbackVector()),
- feedback.slot());
+ Node* node = NewNode(op, object, BuildLoadFeedbackVector());
+ return Record(js_type_feedback_, node, feedback.slot());
}
@@ -3510,7 +3500,8 @@ Node* AstGraphBuilder::BuildKeyedStore(Node* object,
Node* key, Node* value,
const VectorSlotPair& feedback,
TypeFeedbackId id) {
const Operator* op = javascript()->StoreProperty(language_mode(),
feedback);
- return Record(js_type_feedback_, NewNode(op, object, key, value), id);
+ Node* node = NewNode(op, object, key, value);
+ return Record(js_type_feedback_, node, id);
}
@@ -3520,7 +3511,8 @@ Node* AstGraphBuilder::BuildNamedStore(Node* object,
Handle<Name> name,
TypeFeedbackId id) {
const Operator* op =
javascript()->StoreNamed(language_mode(), MakeUnique(name),
feedback);
- return Record(js_type_feedback_, NewNode(op, object, value), id);
+ Node* node = NewNode(op, object, value);
+ return Record(js_type_feedback_, node, id);
}
@@ -3529,8 +3521,8 @@ Node* AstGraphBuilder::BuildNamedSuperLoad(Node*
receiver, Node* home_object,
const VectorSlotPair& feedback)
{
Node* name_node = jsgraph()->Constant(name);
const Operator* op = javascript()->CallRuntime(Runtime::kLoadFromSuper,
3);
- Node* value = NewNode(op, receiver, home_object, name_node);
- return Record(js_type_feedback_, value, feedback.slot());
+ Node* node = NewNode(op, receiver, home_object, name_node);
+ return Record(js_type_feedback_, node, feedback.slot());
}
@@ -3539,8 +3531,8 @@ Node* AstGraphBuilder::BuildKeyedSuperLoad(Node*
receiver, Node* home_object,
const VectorSlotPair& feedback)
{
const Operator* op =
javascript()->CallRuntime(Runtime::kLoadKeyedFromSuper, 3);
- Node* value = NewNode(op, receiver, home_object, key);
- return Record(js_type_feedback_, value, feedback.slot());
+ Node* node = NewNode(op, receiver, home_object, key);
+ return Record(js_type_feedback_, node, feedback.slot());
}
@@ -3551,8 +3543,8 @@ Node* AstGraphBuilder::BuildKeyedSuperStore(Node*
receiver, Node* home_object,
?
Runtime::kStoreKeyedToSuper_Strict
:
Runtime::kStoreKeyedToSuper_Sloppy;
const Operator* op = javascript()->CallRuntime(function_id, 4);
- Node* result = NewNode(op, receiver, home_object, key, value);
- return Record(js_type_feedback_, result, id);
+ Node* node = NewNode(op, receiver, home_object, key, value);
+ return Record(js_type_feedback_, node, id);
}
@@ -3564,14 +3556,18 @@ Node* AstGraphBuilder::BuildNamedSuperStore(Node*
receiver, Node* home_object,
? Runtime::kStoreToSuper_Strict
: Runtime::kStoreToSuper_Sloppy;
const Operator* op = javascript()->CallRuntime(function_id, 4);
- Node* result = NewNode(op, receiver, home_object, name_node, value);
- return Record(js_type_feedback_, result, id);
+ Node* node = NewNode(op, receiver, home_object, name_node, value);
+ return Record(js_type_feedback_, node, id);
}
-Node* AstGraphBuilder::BuildLoadObjectField(Node* object, int offset) {
- return NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object,
- jsgraph()->IntPtrConstant(offset - kHeapObjectTag));
+Node* AstGraphBuilder::BuildLoadObjectField(Node* object, int offset,
+ bool immutable) {
+ Node* off = jsgraph()->IntPtrConstant(offset - kHeapObjectTag);
+ return immutable
+ ? graph()->NewNode(jsgraph()->machine()->Load(kMachAnyTagged),
+ object, off, graph()->start(),
graph()->start())
+ : NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object,
off);
}
@@ -3598,6 +3594,19 @@ Node* AstGraphBuilder::BuildLoadGlobalProxy() {
}
+Node* AstGraphBuilder::BuildLoadFeedbackVector() {
+ if (!feedback_vector_.is_set()) {
+ Node* closure = GetFunctionClosure();
+ Node* shared = BuildLoadObjectField(
+ closure, JSFunction::kSharedFunctionInfoOffset, true);
+ Node* vector = BuildLoadObjectField(
+ shared, SharedFunctionInfo::kFeedbackVectorOffset, true);
+ feedback_vector_.set(vector);
+ }
+ return feedback_vector_.get();
+}
+
+
Node* AstGraphBuilder::BuildLoadExternal(ExternalReference reference,
MachineType type) {
return NewNode(jsgraph()->machine()->Load(type),
Index: src/compiler/ast-graph-builder.h
diff --git a/src/compiler/ast-graph-builder.h
b/src/compiler/ast-graph-builder.h
index
2c9f1be4c9d903b2c74ef95e947293cac986054c..ce4ec8ea17cfeaf76fc9c842319d6c45de217692
100644
--- a/src/compiler/ast-graph-builder.h
+++ b/src/compiler/ast-graph-builder.h
@@ -88,7 +88,6 @@ class AstGraphBuilder : public AstVisitor {
// Nodes representing values in the activation record.
SetOncePointer<Node> function_closure_;
SetOncePointer<Node> function_context_;
- SetOncePointer<Node> feedback_vector_;
// Tracks how many try-blocks are currently entered.
int try_catch_nesting_level_;
@@ -98,6 +97,10 @@ class AstGraphBuilder : public AstVisitor {
int input_buffer_size_;
Node** input_buffer_;
+ // Optimization to cache loaded feedback vector.
+ // TODO(mstarzinger): Can we let GVN take care of that?
+ SetOncePointer<Node> feedback_vector_;
+
// Control nodes that exit the function body.
ZoneVector<Node*> exit_controls_;
@@ -149,9 +152,6 @@ class AstGraphBuilder : public AstVisitor {
Node* GetFunctionClosureForContext();
Node* GetFunctionClosure();
- // Get or create the node that represents the functions type feedback
vector.
- Node* GetFeedbackVector();
-
// Node creation helpers.
Node* NewNode(const Operator* op, bool incomplete = false) {
return MakeNode(op, 0, static_cast<Node**>(NULL), incomplete);
@@ -306,8 +306,10 @@ class AstGraphBuilder : public AstVisitor {
Node* BuildLoadBuiltinsObject();
Node* BuildLoadGlobalObject();
Node* BuildLoadGlobalProxy();
- Node* BuildLoadClosure();
- Node* BuildLoadObjectField(Node* object, int offset);
+ Node* BuildLoadFeedbackVector();
+
+ // Builder for accessing a (potentially immutable) object field.
+ Node* BuildLoadObjectField(Node* object, int offset, bool immutable =
false);
// Builders for accessing external references.
Node* BuildLoadExternal(ExternalReference ref, MachineType type);
Index: test/cctest/compiler/test-run-jscalls.cc
diff --git a/test/cctest/compiler/test-run-jscalls.cc
b/test/cctest/compiler/test-run-jscalls.cc
index
a622af8995bda44cacde9f173859de1b6fa06e92..8de2d7a2142d516d781618bd552ed1aa492dbc91
100644
--- a/test/cctest/compiler/test-run-jscalls.cc
+++ b/test/cctest/compiler/test-run-jscalls.cc
@@ -242,6 +242,7 @@ TEST(ContextLoadedFromActivation) {
i::Handle<i::Object> ofun = v8::Utils::OpenHandle(*value);
i::Handle<i::JSFunction> jsfun = Handle<JSFunction>::cast(ofun);
jsfun->set_code(T.function->code());
+ jsfun->set_shared(T.function->shared());
context->Global()->Set(v8_str("foo"), v8::Utils::ToLocal(jsfun));
CompileRun("var x = 24;");
ExpectInt32("foo();", 24);
@@ -263,6 +264,7 @@ TEST(BuiltinLoadedFromActivation) {
i::Handle<i::Object> ofun = v8::Utils::OpenHandle(*value);
i::Handle<i::JSFunction> jsfun = Handle<JSFunction>::cast(ofun);
jsfun->set_code(T.function->code());
+ jsfun->set_shared(T.function->shared());
context->Global()->Set(v8_str("foo"), v8::Utils::ToLocal(jsfun));
CompileRun("var x = 24;");
ExpectObject("foo()", context->Global());
--
--
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.
