LGTM Did you consider making Free(...) take an intptr_t for the size?
http://codereview.chromium.org/7389008/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7389008/diff/1/src/heap.cc#newcode4995 src/heap.cc:4995: if (!ConfigureHeapDefault()) { This looks like an unnecessary change. http://codereview.chromium.org/7389008/diff/1/src/heap.cc#newcode5055 src/heap.cc:5055: if (code_space_ == NULL || !code_space_->Setup()) { These all fit on one line. http://codereview.chromium.org/7389008/diff/1/src/incremental-marking.cc File src/incremental-marking.cc (right): http://codereview.chromium.org/7389008/diff/1/src/incremental-marking.cc#newcode435 src/incremental-marking.cc:435: intptr_t current = marking_deque_.bottom(); I think making all these int instead of intptr_t should obviate the need for the static cast below. http://codereview.chromium.org/7389008/diff/1/src/mark-compact.cc File src/mark-compact.cc (right): http://codereview.chromium.org/7389008/diff/1/src/mark-compact.cc#newcode3009 src/mark-compact.cc:3009: int MarkCompactCollector::SweepConservatively(PagedSpace* space, Page* p) { Perhas this should return intptr_t? http://codereview.chromium.org/7389008/diff/1/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/7389008/diff/1/src/spaces.cc#newcode323 src/spaces.cc:323: } Unneeded change. http://codereview.chromium.org/7389008/diff/1/src/spaces.cc#newcode326 src/spaces.cc:326: static_cast<int>(OS::AllocateAlignment())); RoundUp takes a size_t and an int? Perhaps that's the bug. http://codereview.chromium.org/7389008/diff/1/src/spaces.cc#newcode671 src/spaces.cc:671: allocation_info_.top = reinterpret_cast<Address>(static_cast<uintptr_t>(2)); 2? Don't understand this at all, but it seems the number should have a name, in which case it can have the right type. http://codereview.chromium.org/7389008/diff/1/src/spaces.cc#newcode865 src/spaces.cc:865: if (base == NULL) { Grrr. http://codereview.chromium.org/7389008/diff/1/src/store-buffer.h File src/store-buffer.h (right): http://codereview.chromium.org/7389008/diff/1/src/store-buffer.h#newcode87 src/store-buffer.h:87: static const int kOldStoreBufferLength = kStoreBufferLength * 16; Merge artifact? http://codereview.chromium.org/7389008/diff/1/src/utils.h File src/utils.h (right): http://codereview.chromium.org/7389008/diff/1/src/utils.h#newcode123 src/utils.h:123: static inline T RoundUp(T x, intptr_t m) { This would seem to make unneccessary some of the other changes. http://codereview.chromium.org/7389008/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
