Sorry for all the typos.
Regarding the speed then when the debugger is active all code will be
compiled
by the full compiler which generates code which is 20-30% slower. With the
added
code to debug break slots code size will also increase depending on the
function.
http://codereview.chromium.org/2693002/diff/18001/19003
File src/arm/assembler-arm.h (right):
http://codereview.chromium.org/2693002/diff/18001/19003#newcode646
src/arm/assembler-arm.h:646: // Return sequence is:
On 2010/06/08 10:12:55, Mads Ager wrote:
The comments describe the return sequence. Update?
Done.
http://codereview.chromium.org/2693002/diff/18001/19006
File src/arm/debug-arm.cc (right):
http://codereview.chromium.org/2693002/diff/18001/19006#newcode97
src/arm/debug-arm.cc:97: // Patch the code changing the return from JS
function sequence from
On 2010/06/08 10:12:55, Mads Ager wrote:
This talks about the return sequence. The rest of the comment seems to
have been
updated correctly.
Done.
http://codereview.chromium.org/2693002/diff/18001/19006#newcode101
src/arm/debug-arm.cc:101: // to a call to the debug break return code.
On 2010/06/08 10:12:55, Mads Ager wrote:
return code -> code?
Done (return code -> slot code).
http://codereview.chromium.org/2693002/diff/18001/19009
File src/assembler.h (right):
http://codereview.chromium.org/2693002/diff/18001/19009#newcode250
src/assembler.h:250: // Check whether this return debug break slot has
been patched
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
typo: this return debug -> this debug
Done.
http://codereview.chromium.org/2693002/diff/18001/19016
File src/debug.cc (right):
http://codereview.chromium.org/2693002/diff/18001/19016#newcode133
src/debug.cc:133: // There is always a possible break point as a debug
break slot.
On 2010/06/08 10:12:55, Mads Ager wrote:
as -> at
Done.
http://codereview.chromium.org/2693002/diff/18001/19016#newcode1670
src/debug.cc:1670: addr - Assembler::kPatchReturnSequenceAddressOffset);
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
should be kPatchDebugBreakSlotAddressOffset
Done.
http://codereview.chromium.org/2693002/diff/18001/19016#newcode1697
src/debug.cc:1697: // address to jump to to complete the call which is
replaced by a call to
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
typo: to to -> to
Rephrased.
http://codereview.chromium.org/2693002/diff/18001/19019
File src/full-codegen.cc (right):
http://codereview.chromium.org/2693002/diff/18001/19019#newcode522
src/full-codegen.cc:522: // Mark for statements breakable breakable if
the condition expression is.
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
breakable breakable -> breakable
Done.
http://codereview.chromium.org/2693002/diff/18001/19019#newcode795
src/full-codegen.cc:795: if (position_recorded) {
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
why test for !checker.is_breakable() wouldn't work here?
There are situations where just checking for !checker.is_breakable()
here would generate to many break slots.
http://codereview.chromium.org/2693002/diff/18001/19019#newcode809
src/full-codegen.cc:809: if (!Debugger::IsDebuggerActive()) {
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
This piece of code mostly resembles the one above, consider extracting
it in a
method.
Yes, I know, however with the difference in argument type refactoring
will not unify much.
http://codereview.chromium.org/2693002/diff/18001/19019#newcode832
src/full-codegen.cc:832: CodeGenerator::RecordPositions(masm_,
stmt->statement_pos());
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
there is no stmt variable in this scope
Good catch. I will ensure it builds with ENABLE_DEBUGGER_SUPPORT not
defined before committing.
http://codereview.chromium.org/2693002/diff/18001/19020
File src/full-codegen.h (right):
http://codereview.chromium.org/2693002/diff/18001/19020#newcode64
src/full-codegen.h:64: // that there will be an IC (loat/store/call) in
the code generated for the
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
typo: loat -> load
Done.
http://codereview.chromium.org/2693002/diff/18001/19034
File src/x64/assembler-x64.h (right):
http://codereview.chromium.org/2693002/diff/18001/19034#newcode458
src/x64/assembler-x64.h:458: // Distance between start of patched debug
break slot and and where the
On 2010/06/08 08:06:04, Yury Semikhatsky wrote:
and and -> and
Done.
http://codereview.chromium.org/2693002/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
