Kevin,

I am not sure I understand your idea.

One way I can think of would be like this:
1. CodeGenerator should generate calls to BinaryOpIC_Miss stub.
2. The stub should analyze the types of operands and do the patching to one of
the GenericBinaryOpStub variants.
3. Before returning the stub should calculate the operation result. The only
reasonable way to do that would be to call the existing ("default")
GenericBinaryOpStub, because the runtime implementation simply does not do all
that is necessary.

My current implementation is basically doing all this, only one of the versions of GenericBinaryOpStub serves as a _Miss stub. I believe that separating the two will lead to longer and more complex code (_Miss stubs will need to be generated for all possible combinations of GenericBinaryOpStub parameters and I will need to clone minor key encoding methods from GenericBinaryOpStub for each platform).

Also in my implementation if a stub always gets Smi arguments (or HeapNumbers
when NO_SMI_CODE_IN_STUB is set) then patching does not happen at all, so we
save time on patching and clearing ICs.

Of course I might be completely missing your idea.


http://codereview.chromium.org/553117/diff/2001/3014
File src/code-stubs.h (right):

http://codereview.chromium.org/553117/diff/2001/3014#newcode141
src/code-stubs.h:141: virtual int GetCodeKind();
True. I was unable to do that because Code::Kind is defined in objects.h
which includes this file (code-stubs.h) to use CodeStub.Major. I can see
two ways to break this circular dependency between Code and CodeStub:
1. Move Code::Kind definition out of Code (to globals.h?). This will
require changing Code::STUB (and the like) to STUB all over the
codebase. Also this will require renaming TypeCode.BUILTIN to resolve a
name clash in the global scope.

2. Move CodeStub::Major definition out of code-stubs.h. This is probably
less work, but I am not sure.

Which of the two ways above would you recommend?

Though I would rather not do anything before we agree on the general
approach to ICs (maybe it will all be unnecessary). Still,

On 2010/02/08 12:06:56, Kevin Millikin wrote:
This should return Code::Kind.

http://codereview.chromium.org/553117/diff/2001/3014#newcode144
src/code-stubs.h:144: virtual int GetICState();
Done. Including globals.h helped.
On 2010/02/08 12:06:56, Kevin Millikin wrote:
This should return InlineCacheState.

http://codereview.chromium.org/553117/diff/2001/3008
File src/debug.cc (right):

http://codereview.chromium.org/553117/diff/2001/3008#newcode1367
src/debug.cc:1367: UNREACHABLE();
On 2010/02/08 12:06:56, Kevin Millikin wrote:
This whole block of code should be cleaned up to use a switch over
code->kind().

Done.

http://codereview.chromium.org/553117/diff/2001/3003
File src/ia32/codegen-ia32.h (right):

http://codereview.chromium.org/553117/diff/2001/3003#newcode31
src/ia32/codegen-ia32.h:31: #include "ic-inl.h"
There was (errors like "inline function 'static v8::internal::Code*
v8::internal::IC::GetTargetAtAddress(v8::internal::byte*)' used but
never defined" for every file that includes codegen-ia32.h). I moved two
inline functions
 from ic.h to ic-inl.h and now it's all right.
On 2010/02/08 12:06:56, Kevin Millikin wrote:
Is there a reason this can't be #include "ic.h"?

http://codereview.chromium.org/553117

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

Reply via email to