Finally, the moment you've been waiting for for a long, long time...
*drumroll* LGTM WITH NITS! :-) https://chromiumcodereview.appspot.com/10170030/diff/18004/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/arm/lithium-codegen-arm.cc#newcode3915 src/arm/lithium-codegen-arm.cc:3915: IsFastElementsKind(to_kind)) { IsFastObjectElementsKind(to_kind) ? https://chromiumcodereview.appspot.com/10170030/diff/18004/src/codegen.h File src/codegen.h (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/codegen.h#newcode98 src/codegen.h:98: static void GenerateMapChangeElementTransition(MacroAssembler* masm); nit: for naming consistency, I'd add an 's': Element*s*TransitionGenerator::GenerateMapChangeElement*s*Transition https://chromiumcodereview.appspot.com/10170030/diff/18004/src/elements-kind.h File src/elements-kind.h (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/elements-kind.h#newcode136 src/elements-kind.h:136: !IsHoleyElementsKind(kind); nit: IsFastHoleyElementsKind would be ever so slightly faster. https://chromiumcodereview.appspot.com/10170030/diff/18004/src/flag-definitions.h File src/flag-definitions.h (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/flag-definitions.h#newcode153 src/flag-definitions.h:153: DEFINE_bool(packed_arrays, true, "optimizes arrays that have no holes") intentional? https://chromiumcodereview.appspot.com/10170030/diff/18004/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/ia32/lithium-codegen-ia32.cc#newcode3572 src/ia32/lithium-codegen-ia32.cc:3572: IsFastElementsKind(to_kind)) { IsFastObjectElementsKind(to_kind) ? https://chromiumcodereview.appspot.com/10170030/diff/18004/src/mips/lithium-codegen-mips.cc File src/mips/lithium-codegen-mips.cc (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/mips/lithium-codegen-mips.cc#newcode3705 src/mips/lithium-codegen-mips.cc:3705: IsFastElementsKind(to_kind)) { IsFastObjectElementsKind(to_kind) ? https://chromiumcodereview.appspot.com/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) { Other arches ask "if (CanTransitionToMoreGeneralFastElementsKind(boilerplate_elements_kind, true))" here https://chromiumcodereview.appspot.com/10170030/diff/18004/src/objects-inl.h File src/objects-inl.h (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/objects-inl.h#newcode4857 src/objects-inl.h:4857: (IsFastSmiElementsKind(GetElementsKind()) && nit: could just use IsFastSmiOrObjectElementsKind https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/lithium-codegen-x64.cc File src/x64/lithium-codegen-x64.cc (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/lithium-codegen-x64.cc#newcode3515 src/x64/lithium-codegen-x64.cc:3515: IsFastElementsKind(to_kind)) { IsFastObjectElementsKind(to_kind) ? https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/lithium-codegen-x64.cc#newcode4339 src/x64/lithium-codegen-x64.cc:4339: // already been converted to FAST_ELEMENTS. nit: s/FAST_ELEMENTS/TERMINAL_FAST_ELEMENTS_KIND/ https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/lithium-x64.cc File src/x64/lithium-x64.cc (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/lithium-x64.cc#newcode2017 src/x64/lithium-x64.cc:2017: bool simple_map_change = (GetHoleyElementsKind(from_kind) == to_kind) || 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. https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/stub-cache-x64.cc File src/x64/stub-cache-x64.cc (right): https://chromiumcodereview.appspot.com/10170030/diff/18004/src/x64/stub-cache-x64.cc#newcode1452 src/x64/stub-cache-x64.cc:1452: __ movq(rdi, FieldOperand(rdx, JSArray::kElementsOffset)); You have rdi's original value in r9, can use that (or remove line 1440 and copy this (1452) to 1463). https://chromiumcodereview.appspot.com/10170030/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
