Restore stack backtrace tests removed in revision 1785.

Restore stack backtrace tests removed in revision 1785.

To re-enable tests, instead of compiled code patching, inlined code is used.
Inlined code is only installed in test.

Committed: http://code.google.com/p/v8/source/detail?r=1892

[Kevin Millikin (Chromium), 2009-05-07 08:21:53]

Some comments, but mostly formatting/style issues.  If you address them, it
LGTM.

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) {
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.

http://codereview.chromium.org/108015/diff/1/3#newcode413
Line 413: InlineRuntimeLUT* entry = kInlineRuntimeLUT + i;
I'd write kInlineRuntimeLUT[i] here.

http://codereview.chromium.org/108015/diff/1/3#newcode415
Line 415: return i;
You could make this return entry, and NULL if not found.

http://codereview.chromium.org/108015/diff/1/3#newcode428
Line 428: InlineRuntimeLUT* entry = kInlineRuntimeLUT + idx;
kInlineRuntimeLUT[idx], but unnecessary if FindInlineRuntimeLUT returns the
pointer.

http://codereview.chromium.org/108015/diff/1/3#newcode438
Line 438: CodeGenerator::InlineRuntimeLUT& new_entry,
The style guide calls for arguments passed by reference to be const.

http://codereview.chromium.org/108015/diff/1/3#newcode442
Line 442: InlineRuntimeLUT* entry = kInlineRuntimeLUT + idx;
Same.

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);
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.

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,
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.

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.

http://codereview.chromium.org/108015/diff/1/6#newcode209
Line 209: NewString("_FastCharCodeAt"),
I think this should just be indented 4 from the start of the previous line.

http://codereview.chromium.org/108015/diff/1/6#newcode215
Line 215: CHECK(CodeGenerator::PatchInlineRuntimeEntry(
Same indentation.

[Mikhail Naganov, 2009-05-07 09:23:15]

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.