I have uploaded a new patch set that addresses your comments and includes your
suggestions. Please take another look.

https://chromiumcodereview.appspot.com/9403009/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/9403009/diff/1/src/ia32/full-codegen-ia32.cc#newcode4298
src/ia32/full-codegen-ia32.cc:4298: info_->increment_number_of_ics();
On 2012/02/15 12:07:39, Vyacheslav Egorov wrote:
Please wrap increment and __ call together into a helper function
called
CallIC() instead of spreading it.

Done.

https://chromiumcodereview.appspot.com/9403009/diff/1/src/ic.cc
File src/ic.cc (right):

https://chromiumcodereview.appspot.com/9403009/diff/1/src/ic.cc#newcode335
src/ic.cc:335: if (address > code->instruction_start() &&
On 2012/02/15 12:07:39, Vyacheslav Egorov wrote:
I think Code has Contains method.

Thanks for the suggestion; but this is obsolete anyway due to your other
suggestions :-)

https://chromiumcodereview.appspot.com/9403009/diff/1/src/ic.cc#newcode337
src/ic.cc:337: shared->set_ic_typeinfo_count(shared->ic_typeinfo_count()
+ 1);
On 2012/02/15 12:07:39, Vyacheslav Egorov wrote:
Please add a small comment that this what this code actually does: it
tries to
find s.f.i that owns code that contains given IC.

This is obsolete too.

https://chromiumcodereview.appspot.com/9403009/diff/1/src/runtime-profiler.cc
File src/runtime-profiler.cc (right):

https://chromiumcodereview.appspot.com/9403009/diff/1/src/runtime-profiler.cc#newcode114
src/runtime-profiler.cc:114: PrintF(", ICs with typeinfo: %d/%d (%d%%)",
On 2012/02/15 12:07:39, Vyacheslav Egorov wrote:
I sense SIGFPE here for code objects without ICs.

Done.

https://chromiumcodereview.appspot.com/9403009/diff/1/src/runtime-profiler.cc#newcode265
src/runtime-profiler.cc:265: int ic_typeinfo =
function->shared()->ic_typeinfo_count();
On 2012/02/15 12:07:39, Vyacheslav Egorov wrote:
I would rename ic_typeinfo_count into ic_with_typeinfo_count

Done.

https://chromiumcodereview.appspot.com/9403009/diff/1/src/runtime-profiler.cc#newcode267
src/runtime-profiler.cc:267: int percentage = 100 * ic_typeinfo /
ic_total;
On 2012/02/15 12:07:39, Vyacheslav Egorov wrote:
I sense SIGFPE here for code objects without ICs.

Done.

https://chromiumcodereview.appspot.com/9403009/

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

Reply via email to