Thanks Danno, comments addressed. I couldn't address the crankshaft stub idea
though, since we need a number of things like tail calls, and looking at the
receiver on the stack without building a frame in some cases. I will think more
about that though.
--Michael

ps - working on ports..


https://codereview.chromium.org/279423005/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/279423005/diff/1/src/ast.h#newcode1733
src/ast.h:1733: class AllocationSiteInfo: public ZoneObject {
On 2014/05/16 16:34:11, danno wrote:
CallInfo? Don't wrap this puppy until we actually have another usage
where you
need this abstraction.

Sounds good.

https://codereview.chromium.org/279423005/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/279423005/diff/1/src/hydrogen.cc#newcode8502
src/hydrogen.cc:8502: if (argument_count == 0) {
On 2014/05/16 16:34:11, danno wrote:
ASSERT(argument_count == 0); and remove the if.

I do need to support the 1 argument case too though. It's been vetted
beforehand to be a single argument within a range that prevents building
a loop construct. But I added the assert (>=0 && <=1) and compactified
the expression that initializes new_object.

https://codereview.chromium.org/279423005/diff/1/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/279423005/diff/1/src/hydrogen.h#newcode1296
src/hydrogen.h:1296: static const int kLoopUnfoldLimit = 8;
On 2014/05/16 16:34:11, danno wrote:
I've always been suspicious of this name. You should call this
kElementLoopUnrollThreshold.

Done.

https://codereview.chromium.org/279423005/diff/1/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/279423005/diff/1/src/ia32/code-stubs-ia32.cc#newcode2550
src/ia32/code-stubs-ia32.cc:2550: __ int3();
On 2014/05/16 16:34:11, danno wrote:
I think there might be a macro for this.

There isn't, code even in the macro assembler uses int3() to indicate
"should not come here." I could make one, something like __ Trap(),
which gets used the same on all platforms, worthwhile?

https://codereview.chromium.org/279423005/diff/1/src/ic.cc
File src/ic.cc (right):

https://codereview.chromium.org/279423005/diff/1/src/ic.cc#newcode513
src/ic.cc:513: // Install default stub with the immutable parts of
existing state.
On 2014/05/16 16:34:11, danno wrote:
Code below is unreachable. Just make above
ASSERT(existing_state.stub_type() == MONOMORPHIC_ARRAY);
return;

Done.

https://codereview.chromium.org/279423005/diff/1/src/ic.h
File src/ic.h (right):

https://codereview.chromium.org/279423005/diff/1/src/ic.h#newcode389
src/ic.h:389: CallType call_type,
On 2014/05/16 16:34:11, danno wrote:
fits on a single line?

Done.

https://codereview.chromium.org/279423005/diff/1/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):

https://codereview.chromium.org/279423005/diff/1/src/objects-visiting-inl.h#newcode298
src/objects-visiting-inl.h:298: target->kind() == Code::CALL_IC) ||
On 2014/05/16 16:34:11, danno wrote:
This code can be removed.

Done.

https://codereview.chromium.org/279423005/diff/1/src/type-info.cc
File src/type-info.cc (right):

https://codereview.chromium.org/279423005/diff/1/src/type-info.cc#newcode137
src/type-info.cc:137: return
Handle<JSFunction>(isolate()->native_context()->array_function());
On 2014/05/16 16:34:11, danno wrote:
 From our conversation: Don't clear the function the TypeVector for
call_new if
it is the array function, since that won't leak.

Okay, I've put that over in ClearFunctionTypeFeedback

https://codereview.chromium.org/279423005/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to