LGTM (with the appropriate .gyp changes), but it's somewhat hard to review
with
all the code that's being moved around. I trust that the -heavy versions
are the
only slightly modified from the originals. I've reviewed the rest more
thoroughly.
http://codereview.chromium.org/1164002/diff/1/10
File src/SConscript (right):
http://codereview.chromium.org/1164002/diff/1/10#newcode132
src/SConscript:132: jump-target-light.cc
You need to update at least the .gyp file too (and possibly other
project files).
http://codereview.chromium.org/1164002/diff/1/16
File src/arm/virtual-frame-arm.cc (right):
http://codereview.chromium.org/1164002/diff/1/16#newcode51
src/arm/virtual-frame-arm.cc:51: // All elements are in memory on ARM
(ie, synced).
I guess this is called on ARM. Do you want to try to inline it?
http://codereview.chromium.org/1164002/diff/1/17
File src/arm/virtual-frame-arm.h (right):
http://codereview.chromium.org/1164002/diff/1/17#newcode335
src/arm/virtual-frame-arm.h:335: int element_count_;
Maybe add a comment here? Number of elements on the stack? I guess so.
http://codereview.chromium.org/1164002/diff/1/7
File src/jump-target-heavy.cc (right):
http://codereview.chromium.org/1164002/diff/1/7#newcode46
src/jump-target-heavy.cc:46: #ifdef DEBUG
Maybe move these nasty macros above the first method?
http://codereview.chromium.org/1164002/diff/1/7#newcode47
src/jump-target-heavy.cc:47: #define DECLARE_ARGCHECK_VARS(name)
\
Maybe add a comment here on what these macros intend to accomplish.
http://codereview.chromium.org/1164002/diff/1/2
File src/jump-target-inl.h (right):
http://codereview.chromium.org/1164002/diff/1/2#newcode38
src/jump-target-inl.h:38: namespace v8 {
Extra newline before namespace.
http://codereview.chromium.org/1164002
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply
to this email with the words "REMOVE ME" as the subject.
