Yup, I like the platform-specific changes in particular. Shall we ask the MIPS
guys to review their ports? ;-)

Two questions though.


https://codereview.chromium.org/1149053004/diff/20001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/1149053004/diff/20001/src/code-stubs-hydrogen.cc#newcode902
src/code-stubs-hydrogen.cc:902: if (is_load)
environment()->Push(result);
Doesn't the store stub need to return the value to allow chained
assignments? The handwritten stub appears to have done this implicitly
by virtue of passing in the value in eax and never overwriting it. I
think you want "environment()->Push(is_load ? result : value);" here.

https://codereview.chromium.org/1149053004/diff/20001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/1149053004/diff/20001/src/ic/ic.cc#newcode2052
src/ic/ic.cc:2052: TRACE_GENERIC_IC(isolate(), "KeyedStoreIC",
"arguments receiver");
I find this code confusing. TRACE_GENERIC should be right where we make
the decision that we don't have a stub for the given situation. Since
you're now falling through to StoreElementStub() below, this decision
and the tracing call should move there.
Am I missing something?

https://codereview.chromium.org/1149053004/

--
--
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