On 2010/11/19 09:08:47, Søren Gjesse wrote:
LGTM with the comments addressed.
http://codereview.chromium.org/5140002/diff/1/src/arm/assembler-arm.h
File src/arm/assembler-arm.h (right):
http://codereview.chromium.org/5140002/diff/1/src/arm/assembler-arm.h#newcode1085
src/arm/assembler-arm.h:1085: enum NopMarkerTypes {
I think this enum should be moved to the macro-assembler, and nop/IsNop
should
keep taking int argument. I know that will require that some of the debug
break
slot stuff is moved there as well.
http://codereview.chromium.org/5140002/diff/1/src/arm/codegen-arm.cc
File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/5140002/diff/1/src/arm/codegen-arm.cc#newcode6840
src/arm/codegen-arm.cc:6840: // This code checks for the_hole_value when
we
DontDelete cell.
Some words are missing in "when we DontDelete cell"
http://codereview.chromium.org/5140002/diff/1/src/arm/codegen-arm.cc#newcode6845
src/arm/codegen-arm.cc:6845: if (is_contextual && is_dont_delete &&
FLAG_debug_code) {
Please move the FLAG_debug_code to the beginning of the condition for
clarity.
http://codereview.chromium.org/5140002/diff/1/src/arm/ic-arm.cc
File src/arm/ic-arm.cc (right):
http://codereview.chromium.org/5140002/diff/1/src/arm/ic-arm.cc#newcode910
src/arm/ic-arm.cc:910: Assembler::NopMarkerTypes type =
I think you should loose the default value here.
http://codereview.chromium.org/5140002/diff/1/src/arm/ic-arm.cc#newcode1009
src/arm/ic-arm.cc:1009: // LoadIC::ClearInlinedVersion will call
PatchInlinedContextualLoad with
Could you please elaborate a bit here on the different instructions
generated
for is_dont_delete true/false, and how it is patched.
Just an additional idea - mayby you can encode the is_dont_delete hint in
the
code marker.
http://codereview.chromium.org/5140002/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
