http://codereview.chromium.org/108015/diff/1/3 File src/codegen.cc (right):
http://codereview.chromium.org/108015/diff/1/3#newcode409 Line 409: int CodeGenerator::FindInlineRuntimeLUT(Handle<String> name) { On 2009/05/07 08:21:53, Kevin Millikin wrote: > There is a comment at the top of codegen.h where we try to list all the codegen > files whose implementation is shared between platforms. It should be updated > with the new functions. Done. http://codereview.chromium.org/108015/diff/1/3#newcode413 Line 413: InlineRuntimeLUT* entry = kInlineRuntimeLUT + i; On 2009/05/07 08:21:53, Kevin Millikin wrote: > I'd write kInlineRuntimeLUT[i] here. But I need a pointer, to avoid copying an entry. Really it should be &kInlineRuntimeLUT[i]. Done. http://codereview.chromium.org/108015/diff/1/3#newcode415 Line 415: return i; On 2009/05/07 08:21:53, Kevin Millikin wrote: > You could make this return entry, and NULL if not found. Thanks! Done. http://codereview.chromium.org/108015/diff/1/3#newcode428 Line 428: InlineRuntimeLUT* entry = kInlineRuntimeLUT + idx; On 2009/05/07 08:21:53, Kevin Millikin wrote: > kInlineRuntimeLUT[idx], but unnecessary if FindInlineRuntimeLUT returns the > pointer. Done. http://codereview.chromium.org/108015/diff/1/3#newcode438 Line 438: CodeGenerator::InlineRuntimeLUT& new_entry, On 2009/05/07 08:21:53, Kevin Millikin wrote: > The style guide calls for arguments passed by reference to be const. Done. http://codereview.chromium.org/108015/diff/1/3#newcode442 Line 442: InlineRuntimeLUT* entry = kInlineRuntimeLUT + idx; On 2009/05/07 08:21:53, Kevin Millikin wrote: > Same. Done. http://codereview.chromium.org/108015/diff/1/4 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/108015/diff/1/4#newcode4553 Line 4553: __ shr(ebp_as_smi.reg(), times_2); On 2009/05/07 08:21:53, Kevin Millikin wrote: > Change times_2 to kSmiTagSize. They happen to be the same (and some existing > code relies on it), but times_2 is really part of the instruction encoding and > not really an int. Done. Thanks for an explanation. http://codereview.chromium.org/108015/diff/1/6 File test/cctest/test-log-ia32.cc (right): http://codereview.chromium.org/108015/diff/1/6#newcode70 Line 70: unsigned int ret_addr, On 2009/05/07 08:21:53, Kevin Millikin wrote: > To make the test more nearly 64-bit ready, you could change unsigned int to > uintptr_t wherever it is an address cast to an int as uintptr_t. > If it was only cast for printing, I think the new thing is to use the %p format > specifier and pass the pointer, instead of %x and cast. Internal Address type fits here better because it is a pointer, not an int (e.g. "%p" expects a pointer type). http://codereview.chromium.org/108015/diff/1/6#newcode209 Line 209: NewString("_FastCharCodeAt"), On 2009/05/07 08:21:53, Kevin Millikin wrote: > I think this should just be indented 4 from the start of the previous line. Done. http://codereview.chromium.org/108015/diff/1/6#newcode215 Line 215: CHECK(CodeGenerator::PatchInlineRuntimeEntry( On 2009/05/07 08:21:53, Kevin Millikin wrote: > Same indentation. Done. http://codereview.chromium.org/108015 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
