https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h
File src/builtins.h (right):
https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h#newcode93
src/builtins.h:93: LoadICState::kStrongModeState)
\
On 2015/06/09 12:46:44, mvstanton wrote:
You don't need KeyedLoadIC_Initialize_Strong (and could delete
KeyedLoadIC_Initialize while you are at it if you like).
Done.
https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h
File src/ic/ic-state.h (right):
https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h#newcode205
src/ic/ic-state.h:205: class ContextualModeBits : public
BitField<ContextualMode, 0, 1> {};
On 2015/06/09 11:58:04, Toon Verwaest wrote:
Why was this moved up? That looks odd...
Because of the initialisation order, if I want to be able to have a
static member kStrongModeState inititalised using the bitfield then the
bitfield needs to be declared first.
In StoreICState where something similar is done, the bitfield is
declared as public, in an order which gets around this. Not sure which
is better.
https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic.cc
File src/ic/ic.cc (right):
https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic.cc#newcode937
src/ic/ic.cc:937: return strong ?
isolate->builtins()->KeyedLoadIC_Initialize_Strong()
On 2015/06/09 12:46:44, mvstanton wrote:
I think this shows some unfinished clean-up work now that vector ics
area always
on for loads. We can't achieve UNINITIALIZED state here because of the
statement
above ("if (initialization_state != MEGAMORPHIC)").
Suggests that you could eliminate KeyedLoadIC_Initialize() from the
builtins,
and your strong variant then too.
Done.
https://codereview.chromium.org/1168093002/diff/140001/src/ic/x64/ic-x64.cc
File src/ic/x64/ic-x64.cc (right):
https://codereview.chromium.org/1168093002/diff/140001/src/ic/x64/ic-x64.cc#newcode228
src/ic/x64/ic-x64.cc:228: __ bind(&return_undefined);
On 2015/06/09 11:58:04, Toon Verwaest wrote:
This probably shouldn't be named return_undefined anymore, but rather
"absent"
or so.
Done.
https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode130
src/objects.cc:130: MaybeHandle<Object>
Object::GetProperty(LookupIterator* it, Strength strength) {
On 2015/06/09 11:58:04, Toon Verwaest wrote:
It would be nicer if GetProperty etc would also get a LanguageMode.
Both
DeleteProperty and SetProperty already get that. And if we do that, we
could
encapsulate it in the LookupIterator. I see how this is a bit annoying
since you
are saving bits though ... not sure.
Yeah, this is kind of annoying. Putting more than a bit in the minor key
means that sloppy and strict versions of the stub would get cached
separately though, so it's hard to have these functions taking
LanguageMode instead of Strength without either lying or causing a
regression.
https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode152
src/objects.cc:152: if (is_strong(strength)) {
On 2015/06/09 11:58:04, Toon Verwaest wrote:
What about extracting this in a helper method. See
WriteToReadOnlyProperty for
an example.
Done.
https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-classes.cc
File src/runtime/runtime-classes.cc (right):
https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-classes.cc#newcode265
src/runtime/runtime-classes.cc:265: if (!proto->IsJSReceiver()) {
On 2015/06/09 11:58:04, Toon Verwaest wrote:
Helper method. If you just change the return type to
MaybeHandle<Object> and
ASSIGN_RETURN_FAILURE_ON_EXCEPTION in the client you can use the same
here as in
objects.cc
Done.
https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-classes.cc#newcode334
src/runtime/runtime-classes.cc:334: language_mode);
On 2015/06/09 11:58:04, Toon Verwaest wrote:
Not clear when you use strength and when language_mode ...
Currently language_mode is used for runtime functions (and associated
helpers) that never get called from stubs, and strength is used for
functions that could be called from stubs. There's not a particularly
good reason for this, it just naturally fell out from trying to keep the
full language mode around for as long as possible.
Everything could be changed to talk in terms of Strength instead.
https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):
https://codereview.chromium.org/1168093002/diff/140001/src/runtime/runtime-object.cc#newcode184
src/runtime/runtime-object.cc:184: ASSIGN_RETURN_ON_EXCEPTION(
On 2015/06/09 11:58:04, Toon Verwaest wrote:
If you don't use result you can just return GetObjectProperty(...);
Done.
https://codereview.chromium.org/1168093002/
--
--
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.
