Nits addressed. LGTY?
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.
On 2013/04/09 17:23:25, Jakob wrote:
nit: s/interrupts/interrupt/
Done.
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode229
src/deoptimizer.h:229: Code* interrupt_code,
On 2013/04/09 17:23:25, Jakob wrote:
nit: indentation
Done.
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode233
src/deoptimizer.h:233: // back to a normal interrupts.
On 2013/04/09 17:23:25, Jakob wrote:
nit: s/interrupts/interrupt/
Done.
https://chromiumcodereview.appspot.com/13811014/diff/6003/src/deoptimizer.h#newcode453
src/deoptimizer.h:453: static const int kBackEdgeEntrySize = 2 *
kIntSize + kOneByteSize;
On 2013/04/09 17:23:25, Jakob wrote:
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?
Done.
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.
On 2013/04/09 17:23:25, Jakob wrote:
nit: the comment is outdated, just remove it. (There's only a back
edge if
there's a loop.)
Done.
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
On 2013/04/09 17:23:25, Jakob wrote:
You can remove this TODO, since it describes exactly what this CL is
doing.
Done.
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: ...
On 2013/04/09 17:23:25, Jakob wrote:
This label got lost when you moved the comment. Please add it back, it
is
meaningful.
Done.
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));
On 2013/04/09 17:23:25, Jakob wrote:
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 :-)
As discussed offline, this was refactored to be reused in
VerifyInterruptCode.
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
On 2013/04/09 17:23:25, Jakob wrote:
nit: s/back/back edge/, s/will at/will be at/
Done.
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);
On 2013/04/09 17:23:25, Jakob wrote:
typo: should be Memory::uint32_at(back_edge_cursor);
Done.
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.
On 2013/04/09 17:23:25, Jakob wrote:
nit: forgot one
Yes. And many other places. I hope I caught all of them now.
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.
