Thanks Toon, updated. You might want to check if the new asserts around
code-stubs.h:453 and the bitfields are okay. Otherwise formatting fixes.
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;
On 2013/01/16 10:53:42, Toon Verwaest wrote:
Keep empty line between these lines.
Done.
https://codereview.chromium.org/11817017/diff/30002/src/code-stubs.h#newcode453
src/code-stubs.h:453: BitField<AllocationSiteInfoMode, 8, 8> {};
On 2013/01/16 10:53:42, Toon Verwaest wrote:
You only need 1 bit for this field.
Fixed, and added a static assert to support it.
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
On 2013/01/16 10:53:42, Toon Verwaest wrote:
If |mode| is set to DONT_TRACK_ALLOCATION_SITE_INFO,
|allocation_site_info| may
be NULL.
Done.
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.
On 2013/01/16 10:53:42, Toon Verwaest wrote:
This comment seems to belong above ::GetMode(ElementsKind) rather than
here;
given that it describes the return value of GetMode.
Yep, indeed!
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
On 2013/01/16 10:53:42, Toon Verwaest wrote:
I prefer the original formatting for this piece of code.
Done.
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,
On 2013/01/16 10:53:42, Toon Verwaest wrote:
Without _INFO it probably fits on one line; but this should normally
be
formatted as
FastCloneShallowArrayStub stub(
mode, DONT_TRACK_ALLOCATION_SITE_INFO, length);
thx, now it does fit on one line.
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)
{
On 2013/01/16 10:53:42, Toon Verwaest wrote:
I think it's cleaner to just merge both if branches into one.
big time, thx.
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
On 2013/01/16 10:53:42, Toon Verwaest wrote:
Should probably clarify that we are talking about pessimistic elements
transitions.
Done.
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) {
On 2013/01/16 10:53:42, Toon Verwaest wrote:
FLAG_track_allocation_sites && IsFastSmi...
Done.
https://codereview.chromium.org/11817017/diff/30002/src/objects.cc#newcode7533
src/objects.cc:7533: if (IsFastSmiElementsKind(from) &&
(IsFastObjectElementsKind(to) ||
On 2013/01/16 10:53:42, Toon Verwaest wrote:
Try to put statements between (..) on one line by breaking after &&.
Also join
FLAG_track_allocation_sites with && like above.
Done.
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:
On 2013/01/16 10:53:42, Toon Verwaest wrote:
Trim empty lines down to 2.
Done.
https://codereview.chromium.org/11817017/diff/30002/src/objects.h#newcode6928
src/objects.h:6928: static AllocationSiteInfoMode GetMode();
On 2013/01/16 10:53:42, Toon Verwaest wrote:
Just use the flag if no other arguments are required.
Done.
https://codereview.chromium.org/11817017/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
