Feedback addressed. Landing.
http://codereview.chromium.org/10170030/diff/18004/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):
http://codereview.chromium.org/10170030/diff/18004/src/arm/lithium-codegen-arm.cc#newcode3915
src/arm/lithium-codegen-arm.cc:3915: IsFastElementsKind(to_kind)) {
On 2012/05/22 17:36:49, Jakob wrote:
IsFastObjectElementsKind(to_kind) ?
Done.
http://codereview.chromium.org/10170030/diff/18004/src/codegen.h
File src/codegen.h (right):
http://codereview.chromium.org/10170030/diff/18004/src/codegen.h#newcode98
src/codegen.h:98: static void
GenerateMapChangeElementTransition(MacroAssembler* masm);
On 2012/05/22 17:36:49, Jakob wrote:
nit: for naming consistency, I'd add an 's':
Element*s*TransitionGenerator::GenerateMapChangeElement*s*Transition
Done.
http://codereview.chromium.org/10170030/diff/18004/src/elements-kind.h
File src/elements-kind.h (right):
http://codereview.chromium.org/10170030/diff/18004/src/elements-kind.h#newcode136
src/elements-kind.h:136: !IsHoleyElementsKind(kind);
On 2012/05/22 17:36:49, Jakob wrote:
nit: IsFastHoleyElementsKind would be ever so slightly faster.
Done.
http://codereview.chromium.org/10170030/diff/18004/src/flag-definitions.h
File src/flag-definitions.h (right):
http://codereview.chromium.org/10170030/diff/18004/src/flag-definitions.h#newcode153
src/flag-definitions.h:153: DEFINE_bool(packed_arrays, true, "optimizes
arrays that have no holes")
On 2012/05/22 17:36:49, Jakob wrote:
intentional?
Done.
http://codereview.chromium.org/10170030/diff/18004/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
http://codereview.chromium.org/10170030/diff/18004/src/ia32/lithium-codegen-ia32.cc#newcode3572
src/ia32/lithium-codegen-ia32.cc:3572: IsFastElementsKind(to_kind)) {
On 2012/05/22 17:36:49, Jakob wrote:
IsFastObjectElementsKind(to_kind) ?
Done.
http://codereview.chromium.org/10170030/diff/18004/src/mips/lithium-codegen-mips.cc
File src/mips/lithium-codegen-mips.cc (right):
http://codereview.chromium.org/10170030/diff/18004/src/mips/lithium-codegen-mips.cc#newcode3705
src/mips/lithium-codegen-mips.cc:3705: IsFastElementsKind(to_kind)) {
On 2012/05/22 17:36:49, Jakob wrote:
IsFastObjectElementsKind(to_kind) ?
Done.
http://codereview.chromium.org/10170030/diff/18004/src/mips/lithium-codegen-mips.cc#newcode4461
src/mips/lithium-codegen-mips.cc:4461: if (boilerplate_elements_kind !=
TERMINAL_FAST_ELEMENTS_KIND) {
On 2012/05/22 17:36:49, Jakob wrote:
Other arches ask "if
(CanTransitionToMoreGeneralFastElementsKind(boilerplate_elements_kind,
true))"
here
Done.
http://codereview.chromium.org/10170030/diff/18004/src/objects-inl.h
File src/objects-inl.h (right):
http://codereview.chromium.org/10170030/diff/18004/src/objects-inl.h#newcode4857
src/objects-inl.h:4857: (IsFastSmiElementsKind(GetElementsKind()) &&
That's a little bit different. The () make sure that if it's Smi, then
it can contain only Smis or Holes.
On 2012/05/22 17:36:49, Jakob wrote:
nit: could just use IsFastSmiOrObjectElementsKind
http://codereview.chromium.org/10170030/diff/18004/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):
http://codereview.chromium.org/10170030/diff/18004/src/x64/lithium-codegen-x64.cc#newcode3515
src/x64/lithium-codegen-x64.cc:3515: IsFastElementsKind(to_kind)) {
On 2012/05/22 17:36:49, Jakob wrote:
IsFastObjectElementsKind(to_kind) ?
Done.
http://codereview.chromium.org/10170030/diff/18004/src/x64/lithium-codegen-x64.cc#newcode4339
src/x64/lithium-codegen-x64.cc:4339: // already been converted to
FAST_ELEMENTS.
On 2012/05/22 17:36:49, Jakob wrote:
nit: s/FAST_ELEMENTS/TERMINAL_FAST_ELEMENTS_KIND/
Done.
http://codereview.chromium.org/10170030/diff/18004/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):
http://codereview.chromium.org/10170030/diff/18004/src/x64/lithium-x64.cc#newcode2017
src/x64/lithium-x64.cc:2017: bool simple_map_change =
(GetHoleyElementsKind(from_kind) == to_kind) ||
On 2012/05/22 17:36:49, Jakob wrote:
This computation is used often enough that it would be nice to factor
it out
into elements-kind.h:
bool IsSimpleMapChange(ElementsKind from_kind, ElementsKind to_kind) {
return (GetHoleyElementsKind(from_kind) == to_kind) ||
(IsFastSmiElementsKind(from_kind) &&
IsFastObjectElementsKind(to_kind));
}
But then again, this CL is large enough as it is, so feel free to
postpone such
cleanup.
Done.
http://codereview.chromium.org/10170030/diff/18004/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):
http://codereview.chromium.org/10170030/diff/18004/src/x64/stub-cache-x64.cc#newcode1452
src/x64/stub-cache-x64.cc:1452: __ movq(rdi, FieldOperand(rdx,
JSArray::kElementsOffset));
On 2012/05/22 17:36:49, Jakob wrote:
You have rdi's original value in r9, can use that (or remove line 1440
and copy
this (1452) to 1463).
Done.
http://codereview.chromium.org/10170030/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
