Revision: 11051
Author: [email protected]
Date: Thu Mar 15 00:13:46 2012
Log: Ensure that generated code for object literals will call
Runtime_DefineOrRedefineAccessorProperty only once per accessor property.
To do this, we collect all accessor properties in a first pass and emit
code for
defining those properties afterwards in a second pass.
As a finger exercise, the table used for collecting accessors has a (subset
of
an) STL-like iterator interface, including STL-like names and operators.
Although C++ is quite verbose here (as usual, but partly this is caused by
our
current slightly clumsy classes/templates), things work out quite nicely
and it
cleans up some confusion, e.g. a table entry is not an iterator etc.
Everything compiles into very efficient code, e.g. the loop condition 'it !=
accessor_table.end()' compiles into a single 'testl' instruction on ia32.
+1 for using standard APIs!
Review URL: https://chromiumcodereview.appspot.com/9691040
http://code.google.com/p/v8/source/detail?r=11051
Modified:
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/full-codegen.h
/branches/bleeding_edge/src/hashmap.h
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/mips/full-codegen-mips.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
/branches/bleeding_edge/src/zone.h
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Fri Mar 9 04:07:29
2012
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Thu Mar 15 00:13:46
2012
@@ -1408,6 +1408,16 @@
__ CopyFields(r0, r5, r2.bit(), size / kPointerSize);
context()->Plug(r0);
}
+
+
+void FullCodeGenerator::EmitAccessor(Expression* expression) {
+ if (expression == NULL) {
+ __ LoadRoot(r1, Heap::kNullValueRootIndex);
+ __ push(r1);
+ } else {
+ VisitForStackValue(expression);
+ }
+}
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
@@ -1445,6 +1455,7 @@
// marked expressions, no store code is emitted.
expr->CalculateEmitStore();
+ AccessorTable accessor_table(isolate()->zone());
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1493,26 +1504,28 @@
}
break;
case ObjectLiteral::Property::GETTER:
+ accessor_table.find(key)->second->getter = value;
+ break;
case ObjectLiteral::Property::SETTER:
- // Duplicate receiver on stack.
- __ ldr(r0, MemOperand(sp));
- __ push(r0);
- VisitForStackValue(key);
- if (property->kind() == ObjectLiteral::Property::GETTER) {
- VisitForStackValue(value);
- __ LoadRoot(r1, Heap::kNullValueRootIndex);
- __ push(r1);
- } else {
- __ LoadRoot(r1, Heap::kNullValueRootIndex);
- __ push(r1);
- VisitForStackValue(value);
- }
- __ mov(r0, Operand(Smi::FromInt(NONE)));
- __ push(r0);
- __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ accessor_table.find(key)->second->setter = value;
break;
}
}
+
+ // Emit code to define accessors, using only a single call to the
runtime for
+ // each pair of corresponding getters and setters.
+ for (AccessorTable::Iterator it = accessor_table.begin();
+ it != accessor_table.end();
+ ++it) {
+ __ ldr(r0, MemOperand(sp)); // Duplicate receiver.
+ __ push(r0);
+ VisitForStackValue(it->first);
+ EmitAccessor(it->second->getter);
+ EmitAccessor(it->second->setter);
+ __ mov(r0, Operand(Smi::FromInt(NONE)));
+ __ push(r0);
+ __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ }
if (expr->has_function()) {
ASSERT(result_saved);
=======================================
--- /branches/bleeding_edge/src/ast.h Tue Mar 13 05:11:46 2012
+++ /branches/bleeding_edge/src/ast.h Thu Mar 15 00:13:46 2012
@@ -1366,6 +1366,12 @@
kHasFunction = 1 << 1
};
+ struct Accessors: public ZoneObject {
+ Accessors() : getter(NULL), setter(NULL) { }
+ Expression* getter;
+ Expression* setter;
+ };
+
protected:
template<class> friend class AstNodeFactory;
=======================================
--- /branches/bleeding_edge/src/full-codegen.h Fri Mar 9 08:23:06 2012
+++ /branches/bleeding_edge/src/full-codegen.h Thu Mar 15 00:13:46 2012
@@ -470,6 +470,8 @@
Label* done);
void EmitVariableLoad(VariableProxy* proxy);
+ void EmitAccessor(Expression* expression);
+
// Expects the arguments and the function already pushed.
void EmitResolvePossiblyDirectEval(int arg_count);
@@ -804,6 +806,28 @@
};
+// A map from property names to getter/setter pairs allocated in the zone.
+class AccessorTable: public TemplateHashMap<Literal,
+ ObjectLiteral::Accessors,
+ ZoneListAllocationPolicy> {
+ public:
+ explicit AccessorTable(Zone* zone) :
+ TemplateHashMap<Literal,
+ ObjectLiteral::Accessors,
+ ZoneListAllocationPolicy>(Literal::Match),
+ zone_(zone) { }
+
+ Iterator lookup(Literal* literal) {
+ Iterator it = find(literal, true);
+ if (it->second == NULL) it->second = new(zone_)
ObjectLiteral::Accessors();
+ return it;
+ }
+
+ private:
+ Zone* zone_;
+};
+
+
} } // namespace v8::internal
#endif // V8_FULL_CODEGEN_H_
=======================================
--- /branches/bleeding_edge/src/hashmap.h Thu Feb 23 02:12:50 2012
+++ /branches/bleeding_edge/src/hashmap.h Thu Mar 15 00:13:46 2012
@@ -36,15 +36,15 @@
namespace internal {
template<class AllocationPolicy>
-class TemplateHashMap {
+class TemplateHashMapImpl {
public:
typedef bool (*MatchFun) (void* key1, void* key2);
// initial_capacity is the size of the initial hash map;
// it must be a power of 2 (and thus must not be 0).
- TemplateHashMap(MatchFun match, uint32_t initial_capacity = 8);
-
- ~TemplateHashMap();
+ TemplateHashMapImpl(MatchFun match, uint32_t initial_capacity = 8);
+
+ ~TemplateHashMapImpl();
// HashMap entries are (key, value, hash) triplets.
// Some clients may not need to use the value slot
@@ -99,10 +99,10 @@
void Resize();
};
-typedef TemplateHashMap<FreeStoreAllocationPolicy> HashMap;
+typedef TemplateHashMapImpl<FreeStoreAllocationPolicy> HashMap;
template<class P>
-TemplateHashMap<P>::TemplateHashMap(MatchFun match,
+TemplateHashMapImpl<P>::TemplateHashMapImpl(MatchFun match,
uint32_t initial_capacity) {
match_ = match;
Initialize(initial_capacity);
@@ -110,13 +110,13 @@
template<class P>
-TemplateHashMap<P>::~TemplateHashMap() {
+TemplateHashMapImpl<P>::~TemplateHashMapImpl() {
P::Delete(map_);
}
template<class P>
-typename TemplateHashMap<P>::Entry* TemplateHashMap<P>::Lookup(
+typename TemplateHashMapImpl<P>::Entry* TemplateHashMapImpl<P>::Lookup(
void* key, uint32_t hash, bool insert) {
// Find a matching entry.
Entry* p = Probe(key, hash);
@@ -146,7 +146,7 @@
template<class P>
-void TemplateHashMap<P>::Remove(void* key, uint32_t hash) {
+void TemplateHashMapImpl<P>::Remove(void* key, uint32_t hash) {
// Lookup the entry for the key to remove.
Entry* p = Probe(key, hash);
if (p->key == NULL) {
@@ -206,7 +206,7 @@
template<class P>
-void TemplateHashMap<P>::Clear() {
+void TemplateHashMapImpl<P>::Clear() {
// Mark all entries as empty.
const Entry* end = map_end();
for (Entry* p = map_; p < end; p++) {
@@ -217,13 +217,14 @@
template<class P>
-typename TemplateHashMap<P>::Entry* TemplateHashMap<P>::Start() const {
+typename TemplateHashMapImpl<P>::Entry* TemplateHashMapImpl<P>::Start()
const {
return Next(map_ - 1);
}
template<class P>
-typename TemplateHashMap<P>::Entry* TemplateHashMap<P>::Next(Entry* p)
const {
+typename TemplateHashMapImpl<P>::Entry*
TemplateHashMapImpl<P>::Next(Entry* p)
+ const {
const Entry* end = map_end();
ASSERT(map_ - 1 <= p && p < end);
for (p++; p < end; p++) {
@@ -236,7 +237,7 @@
template<class P>
-typename TemplateHashMap<P>::Entry* TemplateHashMap<P>::Probe(void* key,
+typename TemplateHashMapImpl<P>::Entry*
TemplateHashMapImpl<P>::Probe(void* key,
uint32_t hash)
{
ASSERT(key != NULL);
@@ -258,7 +259,7 @@
template<class P>
-void TemplateHashMap<P>::Initialize(uint32_t capacity) {
+void TemplateHashMapImpl<P>::Initialize(uint32_t capacity) {
ASSERT(IsPowerOf2(capacity));
map_ = reinterpret_cast<Entry*>(P::New(capacity * sizeof(Entry)));
if (map_ == NULL) {
@@ -271,7 +272,7 @@
template<class P>
-void TemplateHashMap<P>::Resize() {
+void TemplateHashMapImpl<P>::Resize() {
Entry* map = map_;
uint32_t n = occupancy_;
@@ -289,6 +290,50 @@
// Delete old map.
P::Delete(map);
}
+
+
+// A hash map for pointer keys and values with an STL-like interface.
+template<class Key, class Value, class AllocationPolicy>
+class TemplateHashMap: private TemplateHashMapImpl<AllocationPolicy> {
+ public:
+ STATIC_ASSERT(sizeof(Key*) == sizeof(void*)); // NOLINT
+ STATIC_ASSERT(sizeof(Value*) == sizeof(void*)); // NOLINT
+ struct value_type {
+ Key* first;
+ Value* second;
+ };
+
+ class Iterator {
+ public:
+ Iterator& operator++() {
+ entry_ = map_->Next(entry_);
+ return *this;
+ }
+
+ value_type* operator->() { return
reinterpret_cast<value_type*>(entry_); }
+ bool operator!=(const Iterator& other) { return entry_ !=
other.entry_; }
+
+ private:
+ Iterator(const TemplateHashMapImpl<AllocationPolicy>* map,
+ typename TemplateHashMapImpl<AllocationPolicy>::Entry*
entry) :
+ map_(map), entry_(entry) { }
+
+ const TemplateHashMapImpl<AllocationPolicy>* map_;
+ typename TemplateHashMapImpl<AllocationPolicy>::Entry* entry_;
+
+ friend class TemplateHashMap;
+ };
+
+ TemplateHashMap(
+ typename TemplateHashMapImpl<AllocationPolicy>::MatchFun match)
+ : TemplateHashMapImpl<AllocationPolicy>(match) { }
+
+ Iterator begin() const { return Iterator(this, this->Start()); }
+ Iterator end() const { return Iterator(this, NULL); }
+ Iterator find(Key* key, bool insert = false) {
+ return Iterator(this, Lookup(key, key->Hash(), insert));
+ }
+};
} } // namespace v8::internal
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Mar 9
08:23:06 2012
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Thu Mar 15
00:13:46 2012
@@ -1409,6 +1409,15 @@
}
context()->Plug(eax);
}
+
+
+void FullCodeGenerator::EmitAccessor(Expression* expression) {
+ if (expression == NULL) {
+ __ push(Immediate(isolate()->factory()->null_value()));
+ } else {
+ VisitForStackValue(expression);
+ }
+}
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
@@ -1445,6 +1454,7 @@
// marked expressions, no store code is emitted.
expr->CalculateEmitStore();
+ AccessorTable accessor_table(isolate()->zone());
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1456,6 +1466,8 @@
result_saved = true;
}
switch (property->kind()) {
+ case ObjectLiteral::Property::CONSTANT:
+ UNREACHABLE();
case ObjectLiteral::Property::MATERIALIZED_LITERAL:
ASSERT(!CompileTimeValue::IsCompileTimeValue(value));
// Fall through.
@@ -1487,23 +1499,27 @@
__ Drop(3);
}
break;
- case ObjectLiteral::Property::SETTER:
case ObjectLiteral::Property::GETTER:
- __ push(Operand(esp, 0)); // Duplicate receiver.
- VisitForStackValue(key);
- if (property->kind() == ObjectLiteral::Property::GETTER) {
- VisitForStackValue(value);
- __ push(Immediate(isolate()->factory()->null_value()));
- } else {
- __ push(Immediate(isolate()->factory()->null_value()));
- VisitForStackValue(value);
- }
- __ push(Immediate(Smi::FromInt(NONE)));
- __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ accessor_table.lookup(key)->second->getter = value;
break;
- default: UNREACHABLE();
+ case ObjectLiteral::Property::SETTER:
+ accessor_table.lookup(key)->second->setter = value;
+ break;
}
}
+
+ // Emit code to define accessors, using only a single call to the
runtime for
+ // each pair of corresponding getters and setters.
+ for (AccessorTable::Iterator it = accessor_table.begin();
+ it != accessor_table.end();
+ ++it) {
+ __ push(Operand(esp, 0)); // Duplicate receiver.
+ VisitForStackValue(it->first);
+ EmitAccessor(it->second->getter);
+ EmitAccessor(it->second->setter);
+ __ push(Immediate(Smi::FromInt(NONE)));
+ __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ }
if (expr->has_function()) {
ASSERT(result_saved);
=======================================
--- /branches/bleeding_edge/src/mips/full-codegen-mips.cc Tue Mar 13
09:18:30 2012
+++ /branches/bleeding_edge/src/mips/full-codegen-mips.cc Thu Mar 15
00:13:46 2012
@@ -1419,6 +1419,16 @@
__ CopyFields(v0, t1, a2.bit(), size / kPointerSize);
context()->Plug(v0);
}
+
+
+void FullCodeGenerator::EmitAccessor(Expression* expression) {
+ if (expression == NULL) {
+ __ LoadRoot(a1, Heap::kNullValueRootIndex);
+ __ push(a1);
+ } else {
+ VisitForStackValue(expression);
+ }
+}
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
@@ -1456,6 +1466,7 @@
// marked expressions, no store code is emitted.
expr->CalculateEmitStore();
+ AccessorTable accessor_table(isolate()->zone());
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1505,26 +1516,28 @@
}
break;
case ObjectLiteral::Property::GETTER:
+ accessor_table.find(key)->second->getter = value;
+ break;
case ObjectLiteral::Property::SETTER:
- // Duplicate receiver on stack.
- __ lw(a0, MemOperand(sp));
- __ push(a0);
- VisitForStackValue(key);
- if (property->kind() == ObjectLiteral::Property::GETTER) {
- VisitForStackValue(value);
- __ LoadRoot(a1, Heap::kNullValueRootIndex);
- __ push(a1);
- } else {
- __ LoadRoot(a1, Heap::kNullValueRootIndex);
- __ push(a1);
- VisitForStackValue(value);
- }
- __ li(a0, Operand(Smi::FromInt(NONE)));
- __ push(a0);
- __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ accessor_table.find(key)->second->setter = value;
break;
}
}
+
+ // Emit code to define accessors, using only a single call to the
runtime for
+ // each pair of corresponding getters and setters.
+ for (AccessorTable::Iterator it = accessor_table.begin();
+ it != accessor_table.end();
+ ++it) {
+ __ lw(a0, MemOperand(sp)); // Duplicate receiver.
+ __ push(a0);
+ VisitForStackValue(it->first);
+ EmitAccessor(it->second->getter);
+ EmitAccessor(it->second->setter);
+ __ li(a0, Operand(Smi::FromInt(NONE)));
+ __ push(a0);
+ __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ }
if (expr->has_function()) {
ASSERT(result_saved);
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Fri Mar 9 04:07:29
2012
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Thu Mar 15 00:13:46
2012
@@ -1375,6 +1375,15 @@
}
context()->Plug(rax);
}
+
+
+void FullCodeGenerator::EmitAccessor(Expression* expression) {
+ if (expression == NULL) {
+ __ PushRoot(Heap::kNullValueRootIndex);
+ } else {
+ VisitForStackValue(expression);
+ }
+}
void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
@@ -1411,6 +1420,7 @@
// marked expressions, no store code is emitted.
expr->CalculateEmitStore();
+ AccessorTable accessor_table(isolate()->zone());
for (int i = 0; i < expr->properties()->length(); i++) {
ObjectLiteral::Property* property = expr->properties()->at(i);
if (property->IsCompileTimeValue()) continue;
@@ -1455,22 +1465,27 @@
__ Drop(3);
}
break;
- case ObjectLiteral::Property::SETTER:
case ObjectLiteral::Property::GETTER:
- __ push(Operand(rsp, 0)); // Duplicate receiver.
- VisitForStackValue(key);
- if (property->kind() == ObjectLiteral::Property::GETTER) {
- VisitForStackValue(value);
- __ PushRoot(Heap::kNullValueRootIndex);
- } else {
- __ PushRoot(Heap::kNullValueRootIndex);
- VisitForStackValue(value);
- }
- __ Push(Smi::FromInt(NONE));
- __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ accessor_table.find(key)->second->getter = value;
break;
+ case ObjectLiteral::Property::SETTER:
+ accessor_table.find(key)->second->setter = value;
+ break;
}
}
+
+ // Emit code to define accessors, using only a single call to the
runtime for
+ // each pair of corresponding getters and setters.
+ for (AccessorTable::Iterator it = accessor_table.begin();
+ it != accessor_table.end();
+ ++it) {
+ __ push(Operand(rsp, 0)); // Duplicate receiver.
+ VisitForStackValue(it->first);
+ EmitAccessor(it->second->getter);
+ EmitAccessor(it->second->setter);
+ __ Push(Smi::FromInt(NONE));
+ __ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
+ }
if (expr->has_function()) {
ASSERT(result_saved);
=======================================
--- /branches/bleeding_edge/src/zone.h Thu Feb 23 01:12:57 2012
+++ /branches/bleeding_edge/src/zone.h Thu Mar 15 00:13:46 2012
@@ -240,7 +240,7 @@
};
-typedef TemplateHashMap<ZoneListAllocationPolicy> ZoneHashMap;
+typedef TemplateHashMapImpl<ZoneListAllocationPolicy> ZoneHashMap;
} } // namespace v8::internal
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
