I find "Unsafe" a bad name for what it's doing. It's totally unclear from
the
name.
I also think the "unsafe" variant is used way too frequently. We should
always
use a map when there is one. I basically only see one valid use-case
currently
for the unsafe variant: where we are clearing the next word after an object.
That value has nothing to do with the current object. And for that usecase,
we
should definitely not track that value, since it's not part of any object.
Which
is the opposite of what "unsafe" does now, afaict.
All other accesses should get a map as input. Most often you know exactly
which
map is used. Only if there's a type-check involved we don't know the exact
map.
In that case maybe we should use a prototypical map. Eg for JSValue, just
use a
sample map of JSValues (eg the number map). For string map, use any of the
string maps as sample. They should always have the same instance_size
anyway (as
indicated by the instance type).
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.cc#newcode3598
src/hydrogen-instructions.cc:3598: Representation::Smi());
filler_map
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.cc#newcode4248
src/hydrogen-instructions.cc:4248: if (!map.is_null()) {
I think you should use map->instance_size() -
map->unused_property_fields() * kPointerSize as the limit between
visible and nonvisible memory; that way also header-fields and hidden
inobject properties are included.
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (left):
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.h#oldcode6514
src/hydrogen-instructions.h:6514: obj->IsAllocate() ||
obj->IsInnerAllocatedObject());
The equivalent ASSERT should still hold using the access'
ExistingInobjectPropertyField right?
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.h#oldcode6704
src/hydrogen-instructions.h:6704: obj->IsAllocate() ||
obj->IsInnerAllocatedObject());
Same as above
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.h#newcode5756
src/hydrogen-instructions.h:5756: return HObjectAccess(kDouble,
spurious change
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen-instructions.h#newcode5939
src/hydrogen-instructions.h:5939: static HObjectAccess
ForJSTypedArrayLength() {
You can probably get a map for each of the unsafe accesses below.
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc#newcode8021
src/hydrogen.cc:8021:
HObjectAccess::ForJSObjectOffsetUnsafe(JSFunction::kContextOffset));
target->map();
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc#newcode8028
src/hydrogen.cc:8028: GlobalObject::kGlobalReceiverOffset));
target->context()->global_object()->map()
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc#newcode8418
src/hydrogen.cc:8418: JSFunction::kPrototypeOrInitialMapOffset));
constructor->map()
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc#newcode8525
src/hydrogen.cc:8525: HObjectAccess::ForJSObjectOffsetUnsafe(offset),
I presume we can get the map in from where we start building this?
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc#newcode10404
src/hydrogen.cc:10404:
HObjectAccess::ForJSObjectOffsetUnsafe(JSValue::kValueOffset)));
Maybe we should pass in a prototypical JSValue map? Eg the one for
numbers?
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.cc#newcode10475
src/hydrogen.cc:10475:
HObjectAccess::ForJSObjectOffsetUnsafe(JSValue::kValueOffset), value);
Same here.
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.h
File src/hydrogen.h (right):
https://codereview.chromium.org/149063010/diff/20001/src/hydrogen.h#newcode2313
src/hydrogen.h:2313: *access =
HObjectAccess::ForJSObjectOffsetUnsafe(offset);
Use map()
https://codereview.chromium.org/149063010/
--
--
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/groups/opt_out.
