AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[turbofan] Further harden the JSCreateClosure nodes.

Browse Source 
The CreateClosureMode introduced with 2ece046c5 is still not 100%
fail-safe and doesn't scale. What we really need instead, especially
when we might start removing the SharedFunctionInfo::code field
eventually, is to tell the JSCreateClosure node which code object to
use. So instead of adding magic around it, let's just pass it to the
node.

Bug: v8:2206, v8:7253, v8:7310
Change-Id: Iedb6ae468a763643617975f47d96854d1aeafbe9
Reviewed-on: https://chromium-review.googlesource.com/937121
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51548}
This commit is contained in:
Benedikt Meurer 2018-02-26 07:54:59 +01:00 committed by Commit Bot
parent 2ece046c5b
commit 597852f815
6 changed files with 22 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/compiler/bytecode-graph-builder.cc
View File

@ -1444,9 +1444,10 @@ void BytecodeGraphBuilder::VisitCreateClosure() {
bytecode_iterator().GetFlagOperand(2))
? TENURED
: NOT_TENURED;
const Operator* op =
javascript()->CreateClosure(shared_info, nexus.GetFeedbackCell(),
CreateClosureMode::kRegular, tenured);
const Operator* op = javascript()->CreateClosure(
shared_info, nexus.GetFeedbackCell(),
handle(jsgraph()->isolate()->builtins()->builtin(Builtins::kCompileLazy)),
tenured);
Node* closure = NewNode(op);
environment()->BindAccumulator(closure);
}

8
src/compiler/js-call-reducer.cc
View File

@ -4657,7 +4657,7 @@ Reduction JSCallReducer::ReducePromiseConstructor(Node* node) {
Node* resolve = effect =
graph()->NewNode(javascript()->CreateClosure(
resolve_shared, factory()->many_closures_cell(),
CreateClosureMode::kBuiltin),
handle(resolve_shared->code(), isolate())),
promise_context, effect, control);
// Allocate the closure for the reject case.
@ -4667,7 +4667,7 @@ Reduction JSCallReducer::ReducePromiseConstructor(Node* node) {
Node* reject = effect =
graph()->NewNode(javascript()->CreateClosure(
reject_shared, factory()->many_closures_cell(),
CreateClosureMode::kBuiltin),
handle(reject_shared->code(), isolate())),
promise_context, effect, control);
// Re-use the params from above, but actually set the promise parameter now.
@ -4962,7 +4962,7 @@ Reduction JSCallReducer::ReducePromisePrototypeFinally(Node* node) {
catch_true = etrue =
graph()->NewNode(javascript()->CreateClosure(
catch_finally, factory()->many_closures_cell(),
CreateClosureMode::kBuiltin),
handle(catch_finally->code(), isolate())),
context, etrue, if_true);
// Allocate the closure for the fulfill case.
@ -4971,7 +4971,7 @@ Reduction JSCallReducer::ReducePromisePrototypeFinally(Node* node) {
then_true = etrue =
graph()->NewNode(javascript()->CreateClosure(
then_finally, factory()->many_closures_cell(),
CreateClosureMode::kBuiltin),
handle(then_finally->code(), isolate())),
context, etrue, if_true);
}

11
src/compiler/js-create-lowering.cc
View File

@ -901,7 +901,7 @@ Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) {
CreateClosureParameters const& p = CreateClosureParametersOf(node->op());
Handle<SharedFunctionInfo> shared = p.shared_info();
Handle<FeedbackCell> feedback_cell = p.feedback_cell();
CreateClosureMode const mode = p.mode();
Handle<Code> code = p.code();
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node);
Node* context = NodeProperties::GetContextInput(node);
@ -910,16 +910,13 @@ Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) {
// seen more than one instantiation, this simplifies the generated code and
// also serves as a heuristic of which allocation sites benefit from it.
if (feedback_cell->map() != isolate()->heap()->many_closures_cell_map()) {
// The generic path can only create closures for user functions.
DCHECK_EQ(isolate()->builtins()->builtin(Builtins::kCompileLazy), *code);
return NoChange();
}
Handle<Map> function_map(
Map::cast(native_context()->get(shared->function_map_index())));
Node* lazy_compile_builtin = jsgraph()->HeapConstant(
handle(mode == CreateClosureMode::kBuiltin
? shared->code()
: isolate()->builtins()->builtin(Builtins::kCompileLazy),
isolate()));
DCHECK(!function_map->IsInobjectSlackTrackingInProgress());
DCHECK(!function_map->is_dictionary_map());
@ -946,7 +943,7 @@ Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) {
a.Store(AccessBuilder::ForJSFunctionSharedFunctionInfo(), shared);
a.Store(AccessBuilder::ForJSFunctionContext(), context);
a.Store(AccessBuilder::ForJSFunctionFeedbackCell(), feedback_cell);
a.Store(AccessBuilder::ForJSFunctionCode(), lazy_compile_builtin);
a.Store(AccessBuilder::ForJSFunctionCode(), code);
STATIC_ASSERT(JSFunction::kSizeWithoutPrototype == 7 * kPointerSize);
if (function_map->has_prototype_slot()) {
a.Store(AccessBuilder::ForJSFunctionPrototypeOrInitialMap(),

3
src/compiler/js-generic-lowering.cc
View File

@ -376,9 +376,6 @@ void JSGenericLowering::LowerJSCreateClosure(Node* node) {
node->InsertInput(zone(), 1, jsgraph()->HeapConstant(p.feedback_cell()));
node->RemoveInput(4); // control
// We cannot deal with closures for builtins here.
DCHECK_EQ(CreateClosureMode::kRegular, p.mode());
// Use the FastNewClosure builtin only for functions allocated in new space.
if (p.pretenure() == NOT_TENURED) {
Callable callable =

19
src/compiler/js-operator.cc
View File

@ -445,21 +445,10 @@ const CreateBoundFunctionParameters& CreateBoundFunctionParametersOf(
return OpParameter<CreateBoundFunctionParameters>(op);
}
size_t hash_value(CreateClosureMode mode) { return static_cast<uint8_t>(mode); }
std::ostream& operator<<(std::ostream& os, CreateClosureMode mode) {
switch (mode) {
case CreateClosureMode::kBuiltin:
return os << "Builtin";
case CreateClosureMode::kRegular:
return os << "Regular";
}
UNREACHABLE();
}
bool operator==(CreateClosureParameters const& lhs,
CreateClosureParameters const& rhs) {
return lhs.pretenure() == rhs.pretenure() &&
lhs.code().location() == rhs.code().location() &&
lhs.feedback_cell().location() == rhs.feedback_cell().location() &&
lhs.shared_info().location() == rhs.shared_info().location();
}
@ -479,7 +468,7 @@ size_t hash_value(CreateClosureParameters const& p) {
std::ostream& operator<<(std::ostream& os, CreateClosureParameters const& p) {
return os << p.pretenure() << ", " << Brief(*p.shared_info()) << ", "
<< Brief(*p.feedback_cell());
<< Brief(*p.feedback_cell()) << ", " << Brief(*p.code());
}
@ -1074,8 +1063,8 @@ const Operator* JSOperatorBuilder::CreateBoundFunction(size_t arity,
const Operator* JSOperatorBuilder::CreateClosure(
Handle<SharedFunctionInfo> shared_info, Handle<FeedbackCell> feedback_cell,
CreateClosureMode mode, PretenureFlag pretenure) {
CreateClosureParameters parameters(shared_info, feedback_cell, mode,
Handle<Code> code, PretenureFlag pretenure) {
CreateClosureParameters parameters(shared_info, feedback_cell, code,
pretenure);
return new (zone()) Operator1<CreateClosureParameters>( // --
IrOpcode::kJSCreateClosure, Operator::kEliminatable, // opcode

22
src/compiler/js-operator.h
View File

@ -529,37 +529,27 @@ std::ostream& operator<<(std::ostream&, CreateBoundFunctionParameters const&);
const CreateBoundFunctionParameters& CreateBoundFunctionParametersOf(
const Operator* op);
// The mode identifies the kind of closure to create.
enum class CreateClosureMode : uint8_t {
kBuiltin, // Closure for a builtin, use SharedFunctionInfo::code().
kRegular // Regular closure, use CompileLazy for code.
};
size_t hash_value(CreateClosureMode);
std::ostream& operator<<(std::ostream&, CreateClosureMode);
// Defines shared information for the closure that should be created. This is
// used as a parameter by JSCreateClosure operators.
class CreateClosureParameters final {
public:
CreateClosureParameters(Handle<SharedFunctionInfo> shared_info,
Handle<FeedbackCell> feedback_cell,
CreateClosureMode mode, PretenureFlag pretenure)
Handle<FeedbackCell> feedback_cell, Handle<Code> code,
PretenureFlag pretenure)
: shared_info_(shared_info),
feedback_cell_(feedback_cell),
mode_(mode),
code_(code),
pretenure_(pretenure) {}
Handle<SharedFunctionInfo> shared_info() const { return shared_info_; }
Handle<FeedbackCell> feedback_cell() const { return feedback_cell_; }
CreateClosureMode mode() const { return mode_; }
Handle<Code> code() const { return code_; }
PretenureFlag pretenure() const { return pretenure_; }
private:
Handle<SharedFunctionInfo> const shared_info_;
Handle<FeedbackCell> const feedback_cell_;
CreateClosureMode const mode_;
Handle<Code> const code_;
PretenureFlag const pretenure_;
};
@ -669,7 +659,7 @@ class V8_EXPORT_PRIVATE JSOperatorBuilder final
const Operator* CreateBoundFunction(size_t arity, Handle<Map> map);
const Operator* CreateClosure(Handle<SharedFunctionInfo> shared_info,
Handle<FeedbackCell> feedback_cell,
CreateClosureMode mode,
Handle<Code> code,
PretenureFlag pretenure = NOT_TENURED);
const Operator* CreateIterResultObject();
const Operator* CreateStringIterator();