Some additional nits, mostly formatting.

https://codereview.chromium.org/11817017/diff/30002/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/11817017/diff/30002/src/code-stubs.h#newcode433
src/code-stubs.h:433: static const int kFastCloneModeCount =
LAST_CLONE_MODE + 1;
Keep empty line between these lines.

https://codereview.chromium.org/11817017/diff/30002/src/code-stubs.h#newcode453
src/code-stubs.h:453: BitField<AllocationSiteInfoMode, 8, 8> {};
You only need 1 bit for this field.

https://codereview.chromium.org/11817017/diff/30002/src/codegen.h
File src/codegen.h (right):

https://codereview.chromium.org/11817017/diff/30002/src/codegen.h#newcode99
src/codegen.h:99: // allocation_site_info_found may be null, if mode is
If |mode| is set to DONT_TRACK_ALLOCATION_SITE_INFO,
|allocation_site_info| may be NULL.

https://codereview.chromium.org/11817017/diff/30002/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/11817017/diff/30002/src/hydrogen.cc#newcode5330
src/hydrogen.cc:5330: // elements kind is the initial elements kind.
This comment seems to belong above ::GetMode(ElementsKind) rather than
here; given that it describes the return value of GetMode.

https://codereview.chromium.org/11817017/diff/30002/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/11817017/diff/30002/src/ia32/code-stubs-ia32.cc#newcode343
src/ia32/code-stubs-ia32.cc:343: size += tracking_on ?
AllocationSiteInfo::kSize + elements_size
I prefer the original formatting for this piece of code.

https://codereview.chromium.org/11817017/diff/30002/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

https://codereview.chromium.org/11817017/diff/30002/src/ia32/lithium-codegen-ia32.cc#newcode5335
src/ia32/lithium-codegen-ia32.cc:5335: FastCloneShallowArrayStub
stub(mode, DONT_TRACK_ALLOCATION_SITE_INFO,
Without _INFO it probably fits on one line; but this should normally be
formatted as

FastCloneShallowArrayStub stub(
    mode, DONT_TRACK_ALLOCATION_SITE_INFO, length);

https://codereview.chromium.org/11817017/diff/30002/src/ia32/lithium-codegen-ia32.cc#newcode5392
src/ia32/lithium-codegen-ia32.cc:5392: if (create_allocation_site_info)
{
I think it's cleaner to just merge both if branches into one.

https://codereview.chromium.org/11817017/diff/30002/src/ic.cc
File src/ic.cc (right):

https://codereview.chromium.org/11817017/diff/30002/src/ic.cc#newcode1856
src/ic.cc:1856: // TODO(mvstanton): we should set the transitioned map
to always be
Should probably clarify that we are talking about pessimistic elements
transitions.

https://codereview.chromium.org/11817017/diff/30002/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/11817017/diff/30002/src/objects.cc#newcode7520
src/objects.cc:7520: if (FLAG_track_allocation_sites) {
FLAG_track_allocation_sites && IsFastSmi...

https://codereview.chromium.org/11817017/diff/30002/src/objects.cc#newcode7533
src/objects.cc:7533: if (IsFastSmiElementsKind(from) &&
(IsFastObjectElementsKind(to) ||
Try to put statements between (..) on one line by breaking after &&.
Also join FLAG_track_allocation_sites with && like above.

https://codereview.chromium.org/11817017/diff/30002/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/11817017/diff/30002/src/objects.h#newcode6914
src/objects.h:6914:
Trim empty lines down to 2.

https://codereview.chromium.org/11817017/diff/30002/src/objects.h#newcode6928
src/objects.h:6928: static AllocationSiteInfoMode GetMode();
Just use the flag if no other arguments are required.

https://codereview.chromium.org/11817017/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to