Cool. I'll try it out. Danno On Wed, Jan 5, 2011 at 10:00 AM, <[email protected]> wrote:
> LGTM > > > http://codereview.chromium.org/5965011/diff/12016/SConstruct > File SConstruct (right): > > http://codereview.chromium.org/5965011/diff/12016/SConstruct#newcode882 > SConstruct:882: Abort("GDBJIT interface is supported only for 32-bit > Linux target.") > 32-bit should perhaps be Intel-compatible? > > http://codereview.chromium.org/5965011/diff/12016/src/assembler.h > File src/assembler.h (right): > > http://codereview.chromium.org/5965011/diff/12016/src/assembler.h#newcode38 > src/assembler.h:38: #include "gdbjit.h" > The files should really be called gdb-jit.* > > > http://codereview.chromium.org/5965011/diff/12016/src/assembler.h#newcode637 > src/assembler.h:637: #ifdef ENABLE_GDBJIT_INTERFACE > and the macro should really be called ENABLE_GDB_JIT_INTERFACE. > > http://codereview.chromium.org/5965011/diff/12016/src/bootstrapper.cc > File src/bootstrapper.cc (right): > > > http://codereview.chromium.org/5965011/diff/12016/src/bootstrapper.cc#newcode36 > src/bootstrapper.cc:36: #include "gdbjit.h" > This shouldn't be necessary. The .cc file doesn't use it and the .h > files should include what they need. > > http://codereview.chromium.org/5965011/diff/12016/src/builtins.cc > File src/builtins.cc (right): > > > http://codereview.chromium.org/5965011/diff/12016/src/builtins.cc#newcode1549 > src/builtins.cc:1549: Object* code = 0; > Not your code, but this should be NULL. Also it should be typed as a > Code* instead of being cast to it 3 places below. > > http://codereview.chromium.org/5965011/diff/12016/src/mark-compact.cc > File src/mark-compact.cc (right): > > > http://codereview.chromium.org/5965011/diff/12016/src/mark-compact.cc#newcode131 > src/mark-compact.cc:131: // If GDBJIT interface is active disable > compaction. > We should remember to fix this in the new GC. We only have to disable > compaction of code space or perhaps we can do it as a delete followed by > an add, though that may kill gdb breakpoints. Perhaps this feature > should be mentioned somewhere prominent like the --help text. > > > http://codereview.chromium.org/5965011/ > > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
