approach seems fine to me (not counting chicken and egg problem with stubs and
builtins).

there is also some hand-waving in builtins.cc Builtins::SetUp that tries to
avoid GC for no apparent reason I am a bit concerned about it (though I am
pretty sure that just generating one additional small stub will not cause GC).


https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/builtins-ia32.cc#newcode421
src/ia32/builtins-ia32.cc:421: CallConstructStub
stub(NO_CALL_FUNCTION_FLAGS);
according to isolate initialization sequence stub cache is initialized
after builtins so I sense an ordering problem here.

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (left):

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/code-stubs-ia32.cc#oldcode4576
src/ia32/code-stubs-ia32.cc:4576: void
CallFunctionStub::FinishCode(Handle<Code> code) {
Did it disappear? I can't find it.

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/code-stubs-ia32.cc#newcode4600
src/ia32/code-stubs-ia32.cc:4600: Label initialize, call;
I would rename call to done.

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/code-stubs-ia32.cc#newcode4604
src/ia32/code-stubs-ia32.cc:4604: __ mov(ebx, Operand(ebx, 1));  // 1 ~
sizeof 'test eax' opcode in bytes.
you can generate assertion into instruction stream that verifies that
next instruction is test eax, ...

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

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/full-codegen-ia32.cc#newcode2332
src/ia32/full-codegen-ia32.cc:2332: __ call(stub.GetCode(),
RelocInfo::CODE_TARGET, expr->id());
RelocInfo::CONSTRUCT_CALL

https://chromiumcodereview.appspot.com/8932004/diff/1/src/ia32/full-codegen-ia32.cc#newcode2335
src/ia32/full-codegen-ia32.cc:2335: if (record_call_target) {
I am curious if you can introduce helper method (e.g. on macro
assembler?) which both generates call to stub _and_ emits test eax
instruction if necessary. This way nobody will forget it.

https://chromiumcodereview.appspot.com/8932004/diff/1/src/type-info.cc
File src/type-info.cc (right):

https://chromiumcodereview.appspot.com/8932004/diff/1/src/type-info.cc#newcode571
src/type-info.cc:571: if (CallStub::HasCache(target)) {
It's a bit confusing that HasCache also verifies whether target is a
CallStub. Maybe call it: CallStub::IsCallStubWithCache?

https://chromiumcodereview.appspot.com/8932004/

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

Reply via email to