Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Issue 7277038: make gc branch compile on Win32. (Closed)

Created:
9 years, 6 months ago by Lasse Reichstein
Modified:
9 years, 5 months ago
CC:
v8-dev
Visibility:
Public.

Description

Make gc branch compile on Win32. Just compile, there are still crashes. And not even close to compiling successfully on Win64.

Patch Set 1 #

Total comments: 6

Patch Set 2 : Addressed review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -19 lines) Patch
M src/compiler-intrinsics.h View 1 chunk +18 lines, -0 lines 0 comments Download
M src/heap.h View 1 1 chunk +3 lines, -3 lines 0 comments Download
M src/heap.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M src/incremental-marking.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M src/platform-win32.cc View 1 2 chunks +31 lines, -6 lines 0 comments Download
M src/spaces.h View 1 3 chunks +5 lines, -4 lines 0 comments Download
M src/x64/macro-assembler-x64.h View 1 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Lasse Reichstein
9 years, 6 months ago (2011-06-28 11:18:25 UTC) #1
Vyacheslav Egorov (Chromium)
LGTM http://codereview.chromium.org/7277038/diff/1/src/heap.h File src/heap.h (right): http://codereview.chromium.org/7277038/diff/1/src/heap.h#newcode241 src/heap.h:241: static bool ConfigureHeap(int max_semispace_size, Hmm. This is pretty ...
9 years, 5 months ago (2011-06-28 12:28:37 UTC) #2
Lasse Reichstein
9 years, 5 months ago (2011-06-29 10:54:58 UTC) #3
http://codereview.chromium.org/7277038/diff/1/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/7277038/diff/1/src/heap.h#newcode241
src/heap.h:241: static bool ConfigureHeap(int max_semispace_size,
The fields are ints, so passing something bigger here won't help anyway. If
anything, requiring ints to be passed will avoid silently eating overflows.

We should probably convert them all to intptr_t, but that's a larger change.

http://codereview.chromium.org/7277038/diff/1/src/platform-win32.cc
File src/platform-win32.cc (right):

http://codereview.chromium.org/7277038/diff/1/src/platform-win32.cc#newcode1415
src/platform-win32.cc:1415: return VirtualFree(address, size, MEM_DECOMMIT) !=
false;
Doesn't seem any shorter, but perhaps a little cleaner. I'll change them.

http://codereview.chromium.org/7277038/diff/1/src/x64/macro-assembler-x64.h
File src/x64/macro-assembler-x64.h (right):

http://codereview.chromium.org/7277038/diff/1/src/x64/macro-assembler-x64.h#n...
src/x64/macro-assembler-x64.h:81: //
---------------------------------------------------------------------------
Deliberate. No need to indent the separator.

Powered by Google App Engine
This is Rietveld 408576698