From b90d87c3828a07060537603ba9c802599276d77b Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Wed, 20 Nov 2013 14:17:47 +0000 Subject: [PATCH] MaterializedLiteral expressions need to cache expression depth. A problem arises in recursive literal expressions due to recent changes that defer allocation of constant literal properties from parse time. We were calculating expression depth as a side-effect of a lazy constant property build, but subsequent calls for the depth always returned 1. Cache the correct depth in the MaterializedLiteral instead. (Related-to/very-partial-revert-of https://codereview.chromium.org/61873003) R=ulan@chromium.org Review URL: https://codereview.chromium.org/78493002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17929 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 10 ++++------ src/ast.cc | 27 ++++++++++++++------------- src/ast.h | 25 ++++++++++++++++++++----- src/ia32/full-codegen-ia32.cc | 10 ++++------ src/mips/full-codegen-mips.cc | 10 ++++------ src/x64/full-codegen-x64.cc | 10 ++++------ 6 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index d95055c9cd..64a9fdf08b 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -1635,8 +1635,7 @@ void FullCodeGenerator::EmitAccessor(Expression* expression) { void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { Comment cmnt(masm_, "[ ObjectLiteral"); - int depth = 1; - expr->BuildConstantProperties(isolate(), &depth); + expr->BuildConstantProperties(isolate()); Handle constant_properties = expr->constant_properties(); __ ldr(r3, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); __ ldr(r3, FieldMemOperand(r3, JSFunction::kLiteralsOffset)); @@ -1651,7 +1650,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ mov(r0, Operand(Smi::FromInt(flags))); int properties_count = constant_properties->length() / 2; if ((FLAG_track_double_fields && expr->may_store_doubles()) || - depth > 1 || Serializer::enabled() || + expr->depth() > 1 || Serializer::enabled() || flags != ObjectLiteral::kFastElements || properties_count > FastCloneShallowObjectStub::kMaximumClonedProperties) { __ Push(r3, r2, r1, r0); @@ -1770,8 +1769,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - int depth = 1; - expr->BuildConstantElements(isolate(), &depth); + expr->BuildConstantElements(isolate()); ZoneList* subexprs = expr->values(); int length = subexprs->length(); Handle constant_elements = expr->constant_elements(); @@ -1795,7 +1793,7 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { __ CallStub(&stub); __ IncrementCounter( isolate()->counters()->cow_arrays_created_stub(), 1, r1, r2); - } else if (depth > 1 || Serializer::enabled() || + } else if (expr->depth() > 1 || Serializer::enabled() || length > FastCloneShallowArrayStub::kMaximumClonedLength) { __ Push(r3, r2, r1); __ CallRuntime(Runtime::kCreateArrayLiteral, 3); diff --git a/src/ast.cc b/src/ast.cc index adf0fb8703..3ca1449409 100644 --- a/src/ast.cc +++ b/src/ast.cc @@ -262,7 +262,7 @@ bool ObjectLiteral::IsBoilerplateProperty(ObjectLiteral::Property* property) { } -void ObjectLiteral::BuildConstantProperties(Isolate* isolate, int* depth) { +void ObjectLiteral::BuildConstantProperties(Isolate* isolate) { if (!constant_properties_.is_null()) return; // Allocate a fixed array to hold all the constant properties. @@ -283,9 +283,8 @@ void ObjectLiteral::BuildConstantProperties(Isolate* isolate, int* depth) { } MaterializedLiteral* m_literal = property->value()->AsMaterializedLiteral(); if (m_literal != NULL) { - int inner_depth = 1; - m_literal->BuildConstants(isolate, &inner_depth); - if (inner_depth >= depth_acc) depth_acc = inner_depth + 1; + m_literal->BuildConstants(isolate); + if (m_literal->depth() >= depth_acc) depth_acc = m_literal->depth() + 1; } // Add CONSTANT and COMPUTED properties to boilerplate. Use undefined @@ -334,11 +333,11 @@ void ObjectLiteral::BuildConstantProperties(Isolate* isolate, int* depth) { fast_elements_ = (max_element_index <= 32) || ((2 * elements) >= max_element_index); set_is_simple(is_simple); - if (depth != NULL) *depth = depth_acc; + set_depth(depth_acc); } -void ArrayLiteral::BuildConstantElements(Isolate* isolate, int* depth) { +void ArrayLiteral::BuildConstantElements(Isolate* isolate) { if (!constant_elements_.is_null()) return; // Allocate a fixed array to hold all the object literals. @@ -355,9 +354,10 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate, int* depth) { Expression* element = values()->at(i); MaterializedLiteral* m_literal = element->AsMaterializedLiteral(); if (m_literal != NULL) { - int inner_depth = 1; - m_literal->BuildConstants(isolate, &inner_depth); - if (inner_depth + 1 > depth_acc) depth_acc = inner_depth + 1; + m_literal->BuildConstants(isolate); + if (m_literal->depth() + 1 > depth_acc) { + depth_acc = m_literal->depth() + 1; + } } Handle boilerplate_value = GetBoilerplateValue(element, isolate); if (boilerplate_value->IsTheHole()) { @@ -392,7 +392,7 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate, int* depth) { constant_elements_ = literals; set_is_simple(is_simple); - if (depth != NULL) *depth = depth_acc; + set_depth(depth_acc); } @@ -408,14 +408,15 @@ Handle MaterializedLiteral::GetBoilerplateValue(Expression* expression, } -void MaterializedLiteral::BuildConstants(Isolate* isolate, int* depth) { +void MaterializedLiteral::BuildConstants(Isolate* isolate) { if (IsArrayLiteral()) { - return AsArrayLiteral()->BuildConstantElements(isolate, depth); + return AsArrayLiteral()->BuildConstantElements(isolate); } if (IsObjectLiteral()) { - return AsObjectLiteral()->BuildConstantProperties(isolate, depth); + return AsObjectLiteral()->BuildConstantProperties(isolate); } ASSERT(IsRegExpLiteral()); + ASSERT(depth() >= 1); // Depth should be initialized. } diff --git a/src/ast.h b/src/ast.h index 2a8669690d..e3fc053d8e 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1409,13 +1409,20 @@ class MaterializedLiteral : public Expression { int literal_index() { return literal_index_; } + int depth() const { + // only callable after initialization. + ASSERT(depth_ >= 1); + return depth_; + } + protected: MaterializedLiteral(Isolate* isolate, int literal_index, int pos) : Expression(isolate, pos), literal_index_(literal_index), - is_simple_(false) {} + is_simple_(false), + depth_(0) {} // A materialized literal is simple if the values consist of only // constants and simple object and array literals. @@ -1423,8 +1430,13 @@ class MaterializedLiteral : public Expression { void set_is_simple(bool is_simple) { is_simple_ = is_simple; } friend class CompileTimeValue; + void set_depth(int depth) { + ASSERT(depth >= 1); + depth_ = depth; + } + // Populate the constant properties/elements fixed array. - void BuildConstants(Isolate* isolate, int* depth); + void BuildConstants(Isolate* isolate); friend class ArrayLiteral; friend class ObjectLiteral; @@ -1438,6 +1450,7 @@ class MaterializedLiteral : public Expression { private: int literal_index_; bool is_simple_; + int depth_; }; @@ -1505,7 +1518,7 @@ class ObjectLiteral V8_FINAL : public MaterializedLiteral { static bool IsBoilerplateProperty(Property* property); // Populate the constant properties fixed array. - void BuildConstantProperties(Isolate* isolate, int* depth = NULL); + void BuildConstantProperties(Isolate* isolate); // Mark all computed expressions that are bound to a key that // is shadowed by a later occurrence of the same key. For the @@ -1564,7 +1577,9 @@ class RegExpLiteral V8_FINAL : public MaterializedLiteral { int pos) : MaterializedLiteral(isolate, literal_index, pos), pattern_(pattern), - flags_(flags) {} + flags_(flags) { + set_depth(1); + } private: Handle pattern_; @@ -1587,7 +1602,7 @@ class ArrayLiteral V8_FINAL : public MaterializedLiteral { } // Populate the constant elements fixed array. - void BuildConstantElements(Isolate* isolate, int* depth = NULL); + void BuildConstantElements(Isolate* isolate); protected: ArrayLiteral(Isolate* isolate, diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index bacfe835db..86c525d7f6 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -1575,8 +1575,7 @@ void FullCodeGenerator::EmitAccessor(Expression* expression) { void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { Comment cmnt(masm_, "[ ObjectLiteral"); - int depth = 1; - expr->BuildConstantProperties(isolate(), &depth); + expr->BuildConstantProperties(isolate()); Handle constant_properties = expr->constant_properties(); int flags = expr->fast_elements() ? ObjectLiteral::kFastElements @@ -1586,7 +1585,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { : ObjectLiteral::kNoFlags; int properties_count = constant_properties->length() / 2; if ((FLAG_track_double_fields && expr->may_store_doubles()) || - depth > 1 || Serializer::enabled() || + expr->depth() > 1 || Serializer::enabled() || flags != ObjectLiteral::kFastElements || properties_count > FastCloneShallowObjectStub::kMaximumClonedProperties) { __ mov(edi, Operand(ebp, JavaScriptFrameConstants::kFunctionOffset)); @@ -1705,8 +1704,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - int depth = 1; - expr->BuildConstantElements(isolate(), &depth); + expr->BuildConstantElements(isolate()); ZoneList* subexprs = expr->values(); int length = subexprs->length(); Handle constant_elements = expr->constant_elements(); @@ -1733,7 +1731,7 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { DONT_TRACK_ALLOCATION_SITE, length); __ CallStub(&stub); - } else if (depth > 1 || Serializer::enabled() || + } else if (expr->depth() > 1 || Serializer::enabled() || length > FastCloneShallowArrayStub::kMaximumClonedLength) { __ mov(ebx, Operand(ebp, JavaScriptFrameConstants::kFunctionOffset)); __ push(FieldOperand(ebx, JSFunction::kLiteralsOffset)); diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 9d93aafb9d..944a5e7cd0 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -1645,8 +1645,7 @@ void FullCodeGenerator::EmitAccessor(Expression* expression) { void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { Comment cmnt(masm_, "[ ObjectLiteral"); - int depth = 1; - expr->BuildConstantProperties(isolate(), &depth); + expr->BuildConstantProperties(isolate()); Handle constant_properties = expr->constant_properties(); __ lw(a3, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); __ lw(a3, FieldMemOperand(a3, JSFunction::kLiteralsOffset)); @@ -1661,7 +1660,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ li(a0, Operand(Smi::FromInt(flags))); int properties_count = constant_properties->length() / 2; if ((FLAG_track_double_fields && expr->may_store_doubles()) || - depth > 1 || Serializer::enabled() || + expr->depth() > 1 || Serializer::enabled() || flags != ObjectLiteral::kFastElements || properties_count > FastCloneShallowObjectStub::kMaximumClonedProperties) { __ Push(a3, a2, a1, a0); @@ -1780,8 +1779,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - int depth = 1; - expr->BuildConstantElements(isolate(), &depth); + expr->BuildConstantElements(isolate()); ZoneList* subexprs = expr->values(); int length = subexprs->length(); @@ -1808,7 +1806,7 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { __ CallStub(&stub); __ IncrementCounter(isolate()->counters()->cow_arrays_created_stub(), 1, a1, a2); - } else if (depth > 1 || Serializer::enabled() || + } else if (expr->depth() > 1 || Serializer::enabled() || length > FastCloneShallowArrayStub::kMaximumClonedLength) { __ Push(a3, a2, a1); __ CallRuntime(Runtime::kCreateArrayLiteral, 3); diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 8947e2f84c..71b54680f2 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -1596,8 +1596,7 @@ void FullCodeGenerator::EmitAccessor(Expression* expression) { void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { Comment cmnt(masm_, "[ ObjectLiteral"); - int depth = 1; - expr->BuildConstantProperties(isolate(), &depth); + expr->BuildConstantProperties(isolate()); Handle constant_properties = expr->constant_properties(); int flags = expr->fast_elements() ? ObjectLiteral::kFastElements @@ -1607,7 +1606,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { : ObjectLiteral::kNoFlags; int properties_count = constant_properties->length() / 2; if ((FLAG_track_double_fields && expr->may_store_doubles()) || - depth > 1 || Serializer::enabled() || + expr->depth() > 1 || Serializer::enabled() || flags != ObjectLiteral::kFastElements || properties_count > FastCloneShallowObjectStub::kMaximumClonedProperties) { __ movq(rdi, Operand(rbp, JavaScriptFrameConstants::kFunctionOffset)); @@ -1726,8 +1725,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { Comment cmnt(masm_, "[ ArrayLiteral"); - int depth = 1; - expr->BuildConstantElements(isolate(), &depth); + expr->BuildConstantElements(isolate()); ZoneList* subexprs = expr->values(); int length = subexprs->length(); Handle constant_elements = expr->constant_elements(); @@ -1754,7 +1752,7 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { DONT_TRACK_ALLOCATION_SITE, length); __ CallStub(&stub); - } else if (depth > 1 || Serializer::enabled() || + } else if (expr->depth() > 1 || Serializer::enabled() || length > FastCloneShallowArrayStub::kMaximumClonedLength) { __ movq(rbx, Operand(rbp, JavaScriptFrameConstants::kFunctionOffset)); __ push(FieldOperand(rbx, JSFunction::kLiteralsOffset));