Addressed comments. Also take into account the is_jsarray flag.
PTAL
https://codereview.chromium.org/18881004/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
https://codereview.chromium.org/18881004/diff/1/src/arm/code-stubs-arm.cc#newcode269
src/arm/code-stubs-arm.cc:269: descriptor->deoptimization_handler_ =
Runtime::FunctionForId(
On 2013/07/10 09:52:37, danno wrote:
Yikes. In general, you should access members of the Stub during
descriptor
initialization, this seems fragile. I would much prefer having one
descriptor
per stub class and one stub class per descriptor .
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc#newcode937
src/code-stubs-hydrogen.cc:937: set_current_block(NULL);
On 2013/07/10 09:52:37, danno wrote:
I think it's cleaner if you return value here and then the code below
doesn't
have to be in an else.
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc#newcode945
src/code-stubs-hydrogen.cc:945: AddStore(array, HObjectAccess::ForMap(),
target_map);
On 2013/07/10 09:52:37, danno wrote:
Why the special casing here? Either the stub should have no
side-effects and
defer to the runtime, or it should handle the case completely. Why can
you not
just use BuildUncheckedMonomorphicElementAccess with the "holeyified"
elements
kind? Also, this case applies to all packed->holey transitions, I
think, e.g.
the if should be:
if (GetPackedElementsKind(from_kind) == GetPackedElementsKind(to_kind)
&&
IsFastPackedElementsKind(from_kind) &&
IsFastHoleyElementsKind(to_kind)) {
...
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc#newcode956
src/code-stubs-hydrogen.cc:956: IsFastObjectElementsKind(to_kind))) {
On 2013/07/10 09:52:37, danno wrote:
Can you create a predicate in elements-kind.h to the effect
DoesTransitionChangeElementsBufferFormat or something similar?
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc#newcode963
src/code-stubs-hydrogen.cc:963:
if_fixed.If<HCompareNumericAndBranch>(elements_length,
On 2013/07/10 09:52:37, danno wrote:
You can use IfNot here and then put the meat of the code below in the
"Then" and
omit the "Else"
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc#newcode974
src/code-stubs-hydrogen.cc:974: BuildNewSpaceArrayCheck(elements_length,
to_kind);
On 2013/07/10 09:52:37, danno wrote:
Why can't you use the helper function "BuildGrowElementsCapacity" for
this line
and the following ~10 or so?
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs-hydrogen.cc#newcode996
src/code-stubs-hydrogen.cc:996:
BuildUncheckedMonomorphicElementAccess(array, key, value, NULL,
On 2013/07/10 09:52:37, danno wrote:
You can probably factor this out to only be called once for both the
elements
format change and simple map changes. For simple map changes, you will
have to
first do a
to_kind = GetHoleyElementsKind(to_kind);
Maybe I'm missing the point here, but
BuildUncheckedMonomorphicElementAccess is called only once.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/18881004/diff/1/src/code-stubs.h#newcode2214
src/code-stubs.h:2214: class ElementsTransitionAndStoreStub : public
HydrogenCodeStub {
On 2013/07/10 09:52:37, danno wrote:
I kind of like the existing invariant that each stub has it's own
descriptor and
there is always a 1-to-1 mapping. It think it would be cleaner if you
make this
a superclass with two subclasses, one for strict and one for
non-strict. then
you could override the script_mode() method (make it virtual), and
everything
else should just work.
Done.
https://codereview.chromium.org/18881004/diff/1/src/code-stubs.h#newcode2259
src/code-stubs.h:2259: StoreModeBits::encode(store_mode_);
On 2013/07/10 09:52:37, danno wrote:
If you choose to do it this way, use the accessor functions.
Done.
https://codereview.chromium.org/18881004/
--
--
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.
