High-level comment:
I don't think overloading the functionality of the GenerateFoo() methods to
serve two pretty different purposes is the best approach here. It's sort of
OK
when looking at this CL alone, but it's not a good basis for future
modifications (such as the upcoming --trace-elements-transitions flag)
because
things will just become too complicated.
So what I'd suggest is to split each of them up into three methods (naming
is
just a rough draft):
• GenerateStoreStubWithTransition, used to generate a stub that's called
from
the keyed store ICs, handles the appropriate runtime call (and, in the
future,
tracing flag) for this use case
• GenerateTransitionStub, used to generate a builtin that's called from
Crankshaft (could replace KeyedStoreIC::GenerateTransitionElements*)
• GenerateElementsTransitionInternal, called from the other two to do the
majority of the work
http://codereview.chromium.org/8344045/diff/1/src/code-stubs.h
File src/code-stubs.h (right):
http://codereview.chromium.org/8344045/diff/1/src/code-stubs.h#newcode1058
src/code-stubs.h:1058: class FromBits: public
BitField<ElementsKind, 0, 4> {};
why this change? (I'm not saying it's bad; it's just surprising that
you've changed one ElementsKind bit field and not the other.)
http://codereview.chromium.org/8344045/diff/1/test/mjsunit/elements-transition.js
File test/mjsunit/elements-transition.js (right):
http://codereview.chromium.org/8344045/diff/1/test/mjsunit/elements-transition.js#newcode41
test/mjsunit/elements-transition.js:41: print("smi->double [" +
test_double +
Unintentional debugging leftover?
http://codereview.chromium.org/8344045/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
