Looking good, but I still have some comments. Lets wait until after the
branch.
https://codereview.chromium.org/167303005/diff/150001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/167303005/diff/150001/src/hydrogen.cc#newcode5489
src/hydrogen.cc:5489: field_map_->AddDependentCompilationInfo(
You really only need to add a dependency on these two things for loads,
unless you care about speeding up optimized code later once the field is
generalized. I presume the current choice is good enough, but we might
need to revisit.
https://codereview.chromium.org/167303005/diff/150001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/167303005/diff/150001/src/objects.cc#newcode2028
src/objects.cc:2028: ? HeapType::OfCurrently(value, isolate)
If it's FORCE_TAGGED we also shouldn't be tracking types, right?
What about having a function that gets the representation in, in
addition to the object? That way, if the representation is tagged, we
don't track?
https://codereview.chromium.org/167303005/diff/150001/src/objects.cc#newcode2753
src/objects.cc:2753: if (old_details.type() == FIELD) {
Only if new_representation is heap_object right?
https://codereview.chromium.org/167303005/diff/150001/src/objects.cc#newcode2818
src/objects.cc:2818: new_descriptors->SetValue(modify_index,
*new_field_type);
Only if new_representation.IsHeapObject()
https://codereview.chromium.org/167303005/diff/150001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/167303005/diff/150001/src/objects.h#newcode1499
src/objects.h:1499: FLAG_track_heap_object_fields) && IsUninitialized())
{
Why did you add this flag? Which piece of code passes in uninitialized
for non-computed fields?
https://codereview.chromium.org/167303005/diff/150001/src/property.h
File src/property.h (right):
https://codereview.chromium.org/167303005/diff/150001/src/property.h#newcode472
src/property.h:472: return GetFieldTypeFromMap(holder()->map());
Aarg, this is ugly. We really need to cache holder()->map() rather than
holder(). Future CL I guess.
https://codereview.chromium.org/167303005/
--
--
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.
