http://codereview.chromium.org/669061/diff/1009/16
File src/bootstrapper.cc (right):

http://codereview.chromium.org/669061/diff/1009/16#newcode1465
src/bootstrapper.cc:1465: if (function_data->IsUndefined()) {
I wonder if this can have interesting side-effects?

Using function template info for this new pointer means that this
function will be seen as an API function (because the function_data
field will be different from undefined).  We need to make sure that this
is ok.  Even if it is ok I think it would be nice to find a less
intrusive way to do this.  It is confusing that function_data can be
different from undefined for two very different reasons now.

http://codereview.chromium.org/669061/diff/1009/19
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/669061/diff/1009/19#newcode171
src/ia32/macro-assembler-ia32.cc:171: lea(dst, Operand(object, dst,
times_half_pointer_size,
On 2010/03/04 21:53:53, antonm wrote:
btw, this dst (in Operand) seems mildly suspicious to me as it's not
initialized.

As discussed offline, this is indeed a bit strange. However, if |offset|
is zero |dst| is the array index so this is the intended behavior.

http://codereview.chromium.org/669061/diff/1009/20
File src/ia32/macro-assembler-ia32.h (right):

http://codereview.chromium.org/669061/diff/1009/20#newcode57
src/ia32/macro-assembler-ia32.h:57: // scratch can be object itself, but
it'd get clobbered.
it'd get -> it will be?

http://codereview.chromium.org/669061/diff/1009/21
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/669061/diff/1009/21#newcode1268
src/ia32/stub-cache-ia32.cc:1268: for (int i = 0; i < argc; i++) {
We should put a limit on the argc for which we unroll the loop here to
control code size.

http://codereview.chromium.org/669061/diff/1009/21#newcode1286
src/ia32/stub-cache-ia32.cc:1286: for (int i = 0; i < argc; i++) {
We should put a limit on argc for which we unroll the loop here to
control code size.  So, if argc is less than say 4 we unroll otherwise
we generate code for a loop to do this.

http://codereview.chromium.org/669061/diff/1009/21#newcode1291
src/ia32/stub-cache-ia32.cc:1291: __ push(edi);
This works well for one argument, but introduces a lot of pushes and
pops for more arguments.  We should be able to improve that by freeing
some more registers before entering the loop so the write barrier code
clobbers other registers instead.

It is not clear to me why we need to preserve ebx here?

http://codereview.chromium.org/669061

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

Reply via email to