The patch looks incomplete.  Where is the ShouldSelfOptimize function?


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

https://chromiumcodereview.appspot.com/9361026/diff/1/src/ia32/full-codegen-ia32.cc#newcode135
src/ia32/full-codegen-ia32.cc:135: if (FLAG_counting_profiler &&
These two flags can be tested inside ShouldSelfOptimize for a reduction
in verbosity.  Perhaps the flags on the shared info can also be tested
there?

https://chromiumcodereview.appspot.com/9361026/diff/1/src/ia32/full-codegen-ia32.cc#newcode151
src/ia32/full-codegen-ia32.cc:151: Immediate(Smi::FromInt(1)));
It seems you can do:

__ sub(Operand::Cell(Handle<JSGlobalPropertyCell>(cell)),
Immediate(...))

https://chromiumcodereview.appspot.com/9361026/diff/1/src/ia32/full-codegen-ia32.cc#newcode154
src/ia32/full-codegen-ia32.cc:154: __ j(zero, compile_stub);
There is an assumption here that the Smi tag is zero and that we
decrement one at a time.  I would prefer j(mi, ... instead.

https://chromiumcodereview.appspot.com/9361026/diff/1/src/ic-inl.h
File src/ic-inl.h (right):

https://chromiumcodereview.appspot.com/9361026/diff/1/src/ic-inl.h#newcode94
src/ic-inl.h:94: if (FLAG_counting_profiler) {
Suggested comment:

// We do not want to optimize until the ICs have settled down
// so when they are patched, we postpone optimization for the current
function
// and the functions above it on the stack that might want to
// inline this one.

https://chromiumcodereview.appspot.com/9361026/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/9361026/diff/1/src/objects.h#newcode5388
src/objects.h:5388: kThisPropertyAssignmentsOffset + kPointerSize;
This is a Smi field.  Can we find another one and put them both in one
field on 64 bit like we do for the ones below?

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

https://chromiumcodereview.appspot.com/9361026/diff/1/src/runtime-profiler.cc#newcode50
src/runtime-profiler.cc:50: // Constants for old profiler.
s/old/statistical/

https://chromiumcodereview.appspot.com/9361026/diff/1/src/runtime-profiler.cc#newcode63
src/runtime-profiler.cc:63: // Constants for new profiler.
s/new/counter based/

https://chromiumcodereview.appspot.com/9361026/diff/1/src/runtime-profiler.cc#newcode71
src/runtime-profiler.cc:71: static const int kSizeLimitEarlyOpt = 500;
Name should contain 'Max'

https://chromiumcodereview.appspot.com/9361026/diff/1/src/runtime-profiler.cc#newcode238
src/runtime-profiler.cc:238: int threshold =
function->shared()->ast_node_count() > 300
constant needs a name

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

https://chromiumcodereview.appspot.com/9361026/diff/1/src/runtime-profiler.h#newcode66
src/runtime-profiler.h:66: }
Missing blank line.

https://chromiumcodereview.appspot.com/9361026/

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

Reply via email to