Lgtm
http://codereview.chromium.org/88025/diff/1/12 File src/assembler-ia32.cc (right): http://codereview.chromium.org/88025/diff/1/12#newcode2226 Line 2226: const char* return_address = (&file_line)[-1]; Maybe check or assert that coverage_log is set? http://codereview.chromium.org/88025/diff/1/12#newcode2235 Line 2235: byte* ia32_coverage_function = As discussed, there must be a way to make this nastiness go away. http://codereview.chromium.org/88025/diff/1/13 File src/codegen-ia32.cc (right): http://codereview.chromium.org/88025/diff/1/13#newcode5281 Line 5281: int delta_to_patch_site = masm_->SizeOfCodeGeneratedSince(patch_site()); Maybe a word somewhere about why you have to use masm_ instead of __. http://codereview.chromium.org/88025/diff/1/2 File src/macro-assembler-arm.h (right): http://codereview.chromium.org/88025/diff/1/2#newcode305 Line 305: #define DEFINE_MASM(masm) masm->stop(__FILE_LINE__); masm-> What is the significance of the name 'DEFINE_MASM'? Sounds odd to me. http://codereview.chromium.org/88025 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
