Mostly nits.
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h
File src/deoptimizer.h (right):
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode227
src/deoptimizer.h:227: // back to a normal interrupts.
nit: s/interrupts/interrupt/
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode229
src/deoptimizer.h:229: Code* interrupt_code,
nit: indentation
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode233
src/deoptimizer.h:233: // back to a normal interrupts.
nit: s/interrupts/interrupt/
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode453
src/deoptimizer.h:453: static const int kBackEdgeEntrySize = 2 *
kIntSize + kOneByteSize;
This constant belongs close to the definition of the BackEdgeEntry
struct in objects.h. Is there any reason you can't move it there, and
remove it both here, in objects.cc and in runtime.cc?
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/full-codegen.h
File src/full-codegen.h (right):
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/full-codegen.h#newcode95
src/full-codegen.h:95: back_edges_(2, info->zone()), // There's always
at least one.
nit: the comment is outdated, just remove it. (There's only a back edge
if there's a loop.)
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/full-codegen.h#newcode827
src/full-codegen.h:827: // TODO(svenpanne) Rename this to something like
back_edges_ and rename
You can remove this TODO, since it describes exactly what this CL is
doing.
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (left):
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/ia32/deoptimizer-ia32.cc#oldcode233
src/ia32/deoptimizer-ia32.cc:233: // ok: ...
This label got lost when you moved the comment. Please add it back, it
is meaningful.
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/ia32/deoptimizer-ia32.cc#oldcode244
src/ia32/deoptimizer-ia32.cc:244: ASSERT_EQ(kJnsInstruction,
*(call_target_address - 3));
I have to say that I preferred this version of the checks, because here
it's trivially easy to convince yourself that exactly the right thing is
being checked.
Happy to discuss this if you prefer the new version :-)
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/objects.cc
File src/objects.cc (right):
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/objects.cc#newcode9619
src/objects.cc:9619: // If there is no back table, the "table start"
will at or after
nit: s/back/back edge/, s/will at/will be at/
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/objects.cc#newcode9628
src/objects.cc:9628: uint32_t ast_id =
Memory::uint32_at(back_edge_cursor + kIntSize);
typo: should be Memory::uint32_at(back_edge_cursor);
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/objects.h
File src/objects.h (right):
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/objects.h#newcode4507
src/objects.h:4507: // instruction stream where the stack check table
starts.
nit: forgot one
https://chromiumcodereview.appspot.com/13811014/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
