LGTM

The debugger part looks OK.


http://codereview.chromium.org/543207/diff/9007/8015
File src/codegen.cc (right):

http://codereview.chromium.org/543207/diff/9007/8015#newcode498
src/codegen.cc:498: masm->TailCallRuntime(ExternalReference(f),
I think these parameters can fit on one line.

http://codereview.chromium.org/543207/diff/9007/8012
File src/codegen.h (right):

http://codereview.chromium.org/543207/diff/9007/8012#newcode388
src/codegen.h:388: // Markd the debugger statemet to be recognized bu
debugger (by the MajorKey)
Markd -> mark

http://codereview.chromium.org/543207/diff/9007/8012#newcode389
src/codegen.h:389: class CEntryDebugBreakStub : public CodeStub {
This is not in itself a C-entry stub. Please rename it to DebugBreakStub
and its major key to DebugBreak.

As it is only used for the debugger statement calling it
DebuggerStatement would be more precise and will not cause confusion
with other ways to enter the debugger.

Maybe introducing new relocation info type and just using a normal
runtime call could be considered.

http://codereview.chromium.org/543207/diff/9007/8012#newcode397
src/codegen.h:397: int MinorKey() { return 1; }
Use a minor key of 0 as there is only one instance of this stub.

http://codereview.chromium.org/543207/diff/9007/8016
File src/debug.cc (right):

http://codereview.chromium.org/543207/diff/9007/8016#newcode80
src/debug.cc:80: debug_break_stub_ = CEntryDebugBreakStub().GetCode();
As the check is now on the major key the member debug_break_stub_ should
not be needed any more.

http://codereview.chromium.org/543207/diff/9007/8019
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/543207/diff/9007/8019#newcode6983
src/x64/codegen-x64.cc:6983: return (result_size_ < 2) ? 0 :
result_size_ * 2;
Does this special assignment of minor key on 64-bit Windows make sense
any more?

http://codereview.chromium.org/543207

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to