I think what Kevin is suggesting is to have the BinaryOpIC_Miss call directly into C++ code as the current IC miss calls does. The actual calculation of the result is handled in the C++ code (maybe by a call to a generic binary op stub or Builtins:XXX). Based on the arguments the BinaryOpIC_Miss is then transitioned to/replaced by a BinaryOpIC_XXX, where one type of XXX is Generic which contains the full binary op stub code. Non-generic BinaryOpIC will make a miss call into C++ if the arguments have other types as expected and the C++ code again calculated the result and transitions the IC. Maybe the second transition will always be to the Generic case.
/Søren On Tue, Feb 9, 2010 at 02:38, <[email protected]> wrote: > 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 > [email protected] > http://groups.google.com/group/v8-dev > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
