LGTM

http://codereview.chromium.org/3291015/diff/10002/11001
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/3291015/diff/10002/11001#newcode1224
src/arm/stub-cache-arm.cc:1224: JSObject* holder,
is it true that holder is always a global object?  if yes, could we
ensure it with types or asserts?

http://codereview.chromium.org/3291015/diff/10002/11001#newcode1266
src/arm/stub-cache-arm.cc:1266: __ mov(r3,
Operand(Handle<SharedFunctionInfo>(function->shared())));
maybe __ Move(r3, Handle<...).  But I don't know if it's falling out of
favour or not :)

http://codereview.chromium.org/3291015/diff/10002/11001#newcode1592
src/arm/stub-cache-arm.cc:1592: Register code = r1;
sorry, that's probably only me, but could you rename code to char_code,
please: when I see code I expect v8::internal::Code* around :)

http://codereview.chromium.org/3291015/diff/10002/11001#newcode1610
src/arm/stub-cache-arm.cc:1610:
char_from_code_generator.GenerateSlow(masm(), call_helper);
just to double check: that is necessary for ::GenerateFast at  ln. 1605?
 if yes, could you add a comment here?

http://codereview.chromium.org/3291015/diff/10002/11001#newcode1856
src/arm/stub-cache-arm.cc:1856: GenerateGlobalReceiverCheck(object,
holder, name, &miss);
aha, looks like holder must be a global object, maybe it should go into
GenerateGlobalReceiverCheck signature

http://codereview.chromium.org/3291015/diff/10002/11003
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/3291015/diff/10002/11003#newcode1790
src/ia32/stub-cache-ia32.cc:1790: __ and_(code, (0xffff << kSmiTagSize)
| kSmiTag);
could it be rewritten as __ and_(Operand(code),
Immediate(Smi::FromInt(0xffff))) ?

http://codereview.chromium.org/3291015/diff/10002/11006
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/3291015/diff/10002/11006#newcode1527
src/x64/stub-cache-x64.cc:1527: // Tail call the full function.
do you want to unify this comment across platforms (see ia32 version and
words about patching)?

http://codereview.chromium.org/3291015/show

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

Reply via email to