* Move almost all roots into an array ready for use by a constant-pool...

* Move almost all roots into an array ready for use by a constant-pool
register on ARM.
* Make some compile-time loops into run-time loops for compactness.

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

[Kasper Lund, 2009-07-08 12:42:37]

LGTM, but maybe you should get Lars' feedback too?

http://codereview.chromium.org/149324/diff/1/2
File src/heap.cc (right):

http://codereview.chromium.org/149324/diff/1/2#newcode1063
Line 1063: roots_[kMetaMapRootIndex] = new_meta_map;
How about automagically generating a private setter for all these?

http://codereview.chromium.org/149324/diff/1/11
File src/heap.h (right):

http://codereview.chromium.org/149324/diff/1/11#newcode38
Line 38: V(Map, meta_map, MetaMap)                                              
     \
Is it really worth giving everything two names to comply with the style guide
for the index constant? I think I'd prefer just having one name...

http://codereview.chromium.org/149324/diff/1/11#newcode137
Line 137: V(Object, symbol_table, SymbolTable)
Why isn't this of type SymbolTable*?

http://codereview.chromium.org/149324/diff/1/11#newcode918
Line 918: kRootListLength};
Put }; on a new line.

http://codereview.chromium.org/149324/diff/1/10
File src/mark-compact.cc (right):

http://codereview.chromium.org/149324/diff/1/10#newcode227
Line 227: if (reinterpret_cast<String*>(second) !=
Why is the reinterpret_cast necessary here? It's really an Object* identity
check, right? It should fit nicely on a single line.

http://codereview.chromium.org/149324/diff/1/10#newcode597
Line 597: reinterpret_cast<SymbolTable*>(Heap::raw_unchecked_symbol_table());
Doesn't raw_unchecked_symbol_table return something of type SymbolTable*
already? Can't you get rid of reinterpret_cast?

http://codereview.chromium.org/149324/diff/1/10#newcode790
Line 790: reinterpret_cast<SymbolTable*>(Heap::raw_unchecked_symbol_table());
Do you need reinterpret_cast here?

[Erik Corry, 2009-07-08 14:19:50]

http://codereview.chromium.org/149324/diff/1/2
File src/heap.cc (right):

http://codereview.chromium.org/149324/diff/1/2#newcode1063
Line 1063: roots_[kMetaMapRootIndex] = new_meta_map;
On 2009/07/08 12:42:37, Kasper Lund wrote:
> How about automagically generating a private setter for all these?

Done.

http://codereview.chromium.org/149324/diff/1/11
File src/heap.h (right):

http://codereview.chromium.org/149324/diff/1/11#newcode38
Line 38: V(Map, meta_map, MetaMap)                                              
     \
On 2009/07/08 12:42:37, Kasper Lund wrote:
> Is it really worth giving everything two names to comply with the style guide
> for the index constant? I think I'd prefer just having one name...

I think I'll stick with both names because the camel case names will be natural
to use in the ARM code generator.

http://codereview.chromium.org/149324/diff/1/11#newcode137
Line 137: V(Object, symbol_table, SymbolTable)
On 2009/07/08 12:42:37, Kasper Lund wrote:
> Why isn't this of type SymbolTable*?

It is now.

http://codereview.chromium.org/149324/diff/1/11#newcode918
Line 918: kRootListLength};
On 2009/07/08 12:42:37, Kasper Lund wrote:
> Put }; on a new line.

OK

http://codereview.chromium.org/149324/diff/1/10
File src/mark-compact.cc (right):

http://codereview.chromium.org/149324/diff/1/10#newcode227
Line 227: if (reinterpret_cast<String*>(second) !=
On 2009/07/08 12:42:37, Kasper Lund wrote:
> Why is the reinterpret_cast necessary here? It's really an Object* identity
> check, right? It should fit nicely on a single line.

It wasn't necessary.