Thanks all!
https://codereview.chromium.org/1328603003/diff/40001/src/ast.cc
File src/ast.cc (right):
https://codereview.chromium.org/1328603003/diff/40001/src/ast.cc#newcode353
src/ast.cc:353: if (saw_computed_name &&
On 2015/09/03 11:27:39, Jakob wrote:
This is surprising: having seen a computed name previously affects how
the
current property is handled? Are you sure this is correct? (I bet we
lack test
coverage for this; NeedsHomeObject() is fairly arcane.)
You are totally right. It is true though. As we discussed offline, I
will be refactoring this so that each ObjectLiteral::Property knows how
many slots it needs, and the code will be less fragile (also, it'll be
tractable from the point of view of the oracle).
https://codereview.chromium.org/1328603003/diff/40001/test/cctest/test-feedback-vector.cc
File test/cctest/test-feedback-vector.cc (right):
https://codereview.chromium.org/1328603003/diff/40001/test/cctest/test-feedback-vector.cc#newcode419
test/cctest/test-feedback-vector.cc:419: CHECK_EQ(FLAG_vector_stores ? 4
: 2, feedback_vector->ICSlots());
On 2015/09/03 11:27:40, Jakob wrote:
suggestion:
if (FLAG_vector_stores) {
CHECK_EQ(4, feedback_vector->ICSlots());
CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(0)) ==
Code::LOAD_IC);
CHECK( 1
STORE_IC);
CHECK( 2
LOAD_IC);
CHECK( 3
STORE_IC);
} else {
CHECK_EQ(2, feedback_vector->ICSlots());
CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(0)) ==
Code::LOAD_IC);
CHECK(feedback_vector->GetKind(FeedbackVectorICSlot(1)) ==
Code::LOAD_IC);
}
Provides more test coverage now, and is trivial to simplify when
FLAG_vector_stores is removed.
I like it, done.
https://codereview.chromium.org/1328603003/diff/40001/test/cctest/test-feedback-vector.cc#newcode453
test/cctest/test-feedback-vector.cc:453:
FeedbackVectorICSlot(FLAG_vector_stores ? 3 : 2)) == Code::CALL_IC);
On 2015/09/03 11:27:40, Jakob wrote:
suggestion:
int slot = 0;
CHECK(...(FeedbackVectorICSlot(slot++)) == Code::CALL_IC);
CHECK(...(FeedbackVectorICSlot(slot++)) == Code::LOAD_IC);
if (FLAG_vector_stores) {
CHECK(...(FeedbackVectorICSlot(slot++)) == Code::STORE_IC);
}
...
Allow me to follow up on this suggestion in a follow-up that reduces the
repetitiveness of these lines. We have a FeedbackVectorSpec class, and
we can ask if the spec differs from the vector. This way I'll be able to
write an array like { Code::LOAD_IC, Code::STORE_IC, ... } and compare.
https://codereview.chromium.org/1328603003/diff/40001/test/cctest/test-feedback-vector.cc#newcode503
test/cctest/test-feedback-vector.cc:503: // Function f has 3 LoadICs,
one for each o, but the ICs share the same
On 2015/09/03 11:27:40, Jakob wrote:
lolwut?
Cosmically weird comment removed :).
https://codereview.chromium.org/1328603003/
--
--
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.
