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. http://codereview.chromium.org/5140002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
