Thanks for the review. Martyn should have uploaded the updated patch.
Alexandre 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 { On 2010/11/19 09:08:47, Søren Gjesse wrote:
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.
Left here as discussed. Moving this to the Masm was not very convenient because of inlined function and included files dependencies. 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. Changed the comment. On 2010/11/19 09:08:47, Søren Gjesse wrote:
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) { On 2010/11/19 09:08:47, Søren Gjesse wrote:
Please move the FLAG_debug_code to the beginning of the condition for
clarity. Done. 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 = On 2010/11/19 09:08:47, Søren Gjesse wrote:
I think you should loose the default value here.
Done. 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 I refactored the code following your idea to encode is_dont_delete in the marker, and we now no longer need this extra if. On 2010/11/19 09:08:47, Søren Gjesse wrote:
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
