Create and use shared stub for for DictionaryValue-based elements.

Unify handling of element IC stubs.

In the process, add shared stubs for DictionaryValue lookups that are handled in the same way as fast elements and external array elements.

Includes code for MIPS, which compiles and run polymorph-arrays.js successfully.

R=jkummerow@chromium.org
BUG=none
TEST=test/mjsunit/polymorph-arrays.js

Committed: http://code.google.com/p/v8/source/detail?r=8579

[danno, 2011-07-06 16:30:01]

PTAL.

Reviewer guidance:

Start in code-stubs.h/.cc, where you will see that the different code stub types
for elements have been merged into two class (one for store, one for load) that
use the minor key in the stub to distinguish themselves from one another. 

Proceed to ic.cc and stub-cache.cc, where you will see the cascading changes due
to this merge of stub classes. You will also see that the merging of code paths
now requires that number dictionary elements ICs are structured identically to
ExternalArray and FastElement element ICs, which requires platforms to implement
a new method on KeyedLoadStubCompiler, called GenerateLoadDictionaryElement,
which generates the non-map checked number dictionary load.

The platform specific changes are to implement this new method, which required
moving the number dictionary loading code to the macro assembler to share it
between the stubs and the ICs.

[Jakob Kummerow, 2011-07-07 13:55:38]

LGTM with a couple of nits. I like this change!

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.cc
File src/code-stubs.cc (right):

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.cc#newcode249
src/code-stubs.cc:249: case JSObject::FAST_ELEMENTS:
nit: indent "case" lines inside "switch" statements.

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.cc#newcode278
src/code-stubs.cc:278: case JSObject::FAST_ELEMENTS:
Same indentation nit here.

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.h
File src/code-stubs.h (right):

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.h#newcode953
src/code-stubs.h:953: explicit KeyedStoreElementStub(bool is_js_array,
"explicit" isn't needed here.

http://codereview.chromium.org/7227010/diff/4001/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/7227010/diff/4001/src/ic.cc#newcode1693
src/ic.cc:1693: Code* generic_stub) {
I think you no longer need |generic_stub|.

http://codereview.chromium.org/7227010/diff/4001/src/stub-cache.h
File src/stub-cache.h (right):

http://codereview.chromium.org/7227010/diff/4001/src/stub-cache.h#newcode665
src/stub-cache.h:665: static void GenerateLoadDictionaryElement(MacroAssembler*
masmx);
x?

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
File test/mjsunit/polymorph-arrays.js (right):

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
test/mjsunit/polymorph-arrays.js:29: // Flags: --allow-natives-syntax
--expose-gc
AFAICS you're not calling GC anywhere, so the first "// Flags:" line should be
enough.

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
test/mjsunit/polymorph-arrays.js:80: assertEquals(1, load(object_array, 1));
Since exactly this assertion is performed inside
make_polymorphic_load_function() as well, I think it's unnecessary to do it
again here. The same is true for the next few lines, so I guess this whole
paragraph could be removed without loss of test coverage. But I don't feel
strongly about this, feel free to leave it as is if you prefer the explicitness
of it.

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
test/mjsunit/polymorph-arrays.js:108: assertEquals(undefined, load(js_array, new
Object()));
Duplicate line.

[danno, 2011-07-08 11:00:24]

Committed

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.cc
File src/code-stubs.cc (right):

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.cc#newcode249
src/code-stubs.cc:249: case JSObject::FAST_ELEMENTS:
On 2011/07/07 13:55:38, jkummerow wrote:
> nit: indent "case" lines inside "switch" statements.

Done.

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.cc#newcode278
src/code-stubs.cc:278: case JSObject::FAST_ELEMENTS:
On 2011/07/07 13:55:38, jkummerow wrote:
> Same indentation nit here.

Done.

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.h
File src/code-stubs.h (right):

http://codereview.chromium.org/7227010/diff/4001/src/code-stubs.h#newcode953
src/code-stubs.h:953: explicit KeyedStoreElementStub(bool is_js_array,
On 2011/07/07 13:55:38, jkummerow wrote:
> "explicit" isn't needed here.

Done.

http://codereview.chromium.org/7227010/diff/4001/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/7227010/diff/4001/src/ic.cc#newcode1693
src/ic.cc:1693: Code* generic_stub) {
On 2011/07/07 13:55:38, jkummerow wrote:
> I think you no longer need |generic_stub|.

Done.

http://codereview.chromium.org/7227010/diff/4001/src/stub-cache.h
File src/stub-cache.h (right):

http://codereview.chromium.org/7227010/diff/4001/src/stub-cache.h#newcode665
src/stub-cache.h:665: static void GenerateLoadDictionaryElement(MacroAssembler*
masmx);
On 2011/07/07 13:55:38, jkummerow wrote:
> x?

Done.

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
File test/mjsunit/polymorph-arrays.js (right):

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
test/mjsunit/polymorph-arrays.js:29: // Flags: --allow-natives-syntax
--expose-gc
On 2011/07/07 13:55:38, jkummerow wrote:
> AFAICS you're not calling GC anywhere, so the first "// Flags:" line should be
> enough.

Done.

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
test/mjsunit/polymorph-arrays.js:80: assertEquals(1, load(object_array, 1));
On 2011/07/07 13:55:38, jkummerow wrote:
> Since exactly this assertion is performed inside
> make_polymorphic_load_function() as well, I think it's unnecessary to do it
> again here. The same is true for the next few lines, so I guess this whole
> paragraph could be removed without loss of test coverage. But I don't feel
> strongly about this, feel free to leave it as is if you prefer the
explicitness
> of it.

Done.

http://codereview.chromium.org/7227010/diff/4001/test/mjsunit/polymorph-array...
test/mjsunit/polymorph-arrays.js:108: assertEquals(undefined, load(js_array, new
Object()));
On 2011/07/07 13:55:38, jkummerow wrote:
> Duplicate line.

Done.