LGTM with comments.

I did not review the assembly code in detail. You just moved that from
ic-<arch>.cc to code-stubs-<arch>.cc, right?


https://chromiumcodereview.appspot.com/11896091/diff/1/src/ic.cc
File src/ic.cc (right):

https://chromiumcodereview.appspot.com/11896091/diff/1/src/ic.cc#newcode816
src/ic.cc:816: TRACE_IC("KeyedCallIC", key, state, target());
We should only TRACE_IC after setting a new target. Please move this
above the '}'.

https://chromiumcodereview.appspot.com/11896091/diff/1/src/ic.cc#newcode1275
src/ic.cc:1275: TRACE_IC("KeyedLoadIC", key, state, target());
Same here. (Due to the conditions, this code actually does the right
thing currently, but let's make it explicit nevertheless that TRACE_IC()
happens iff set_target() was called.)

https://chromiumcodereview.appspot.com/11896091/diff/1/src/ic.cc#newcode1763
src/ic.cc:1763: TRACE_IC("KeyedStoreIC", key, state, target());
Again, please move this to right after set_target().

https://chromiumcodereview.appspot.com/11896091/

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


Reply via email to