MIPS: Enable serialization for MIPS architecture.

MIPS: Enable serialization for MIPS architecture.

This is a re-submission of http://codereview.chromium.org/8491008/ with all the issues addressed.

Changes since previous submission:
The embedded object references in code that can be already found in root array are
replaced with relative loads from root set during code generation before builtins
are serialized.

Also included: Refactoring of MacroAssembler::li().
Added enum flags for li() instead of using bool parameter to indicate
if instuction sequence for loading immediate will be constant or
optimized.

BUG=
TEST=

[kalmard, 2012-02-25 09:58:30]

rebased on r10833.

[kalmard, 2012-02-28 14:30:30]

rebased on r10858.

[Erik Corry, 2012-02-29 14:38:13]

http://codereview.chromium.org/9372063/diff/7001/src/mips/assembler-mips.cc
File src/mips/assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/assembler-mips.cc#n...
src/mips/assembler-mips.cc:853: bool
Assembler::can_use_relative_load(RelocInfo::Mode rmode) {
This should be called CanUseRelativeLoad

http://codereview.chromium.org/9372063/diff/7001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/code-stubs-mips.cc#...
src/mips/code-stubs-mips.cc:7055: RelocInfo::CODE_TARGET), CONSTANT_SIZE);
This line is incorrectly indented.

http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mip...
File src/mips/macro-assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mip...
src/mips/macro-assembler-mips.cc:793: int32_t index =
FindRootIndex(*(reinterpret_cast<Object**>(j.imm32_)));
This will be too slow.  I suggest you make it into
ASSERT(FindRootIndex(...) == kInvalidRootIndex) and then fix the callers to use
LoadRoot.

[kalmard, 2012-03-12 12:40:00]

Thanks for the review.

About the li->LoadRoot changes:

Most caller locations have been changed to LoadRoot() in this CL, as requested.
However in some places we can't tell if the li() operand is roots-array value or
not until runtime. For example, a load of 'var->name()', is usually a normal
string/symbol, but sometimes is "global" which is a string/symbol in the roots
array. In that latter case, when serializing, we must emit the code for
LoadRoot().

Affected li() calls at full-codegen-mips.cc: 483, 489, 1430, 1753, 2055, 2164,
3768 and stub-cache-mips.cc: 2544

http://codereview.chromium.org/9372063/diff/7001/src/mips/assembler-mips.cc
File src/mips/assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/assembler-mips.cc#n...
src/mips/assembler-mips.cc:853: bool
Assembler::can_use_relative_load(RelocInfo::Mode rmode) {
On 2012/02/29 14:38:13, Erik Corry wrote:
> This should be called CanUseRelativeLoad

Renamed to SerializingTryLoadFromRoot to match your style recommendation and
also make it apparent that this only works if serialization is enabled.

http://codereview.chromium.org/9372063/diff/7001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/code-stubs-mips.cc#...
src/mips/code-stubs-mips.cc:7055: RelocInfo::CODE_TARGET), CONSTANT_SIZE);
On 2012/02/29 14:38:13, Erik Corry wrote:
> This line is incorrectly indented.

Done.

http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mip...
File src/mips/macro-assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mip...
src/mips/macro-assembler-mips.cc:793: int32_t index =
FindRootIndex(*(reinterpret_cast<Object**>(j.imm32_)));
On 2012/02/29 14:38:13, Erik Corry wrote:
> This will be too slow.  I suggest you make it into
> ASSERT(FindRootIndex(...) == kInvalidRootIndex) and then fix the callers to
use
> LoadRoot.

The FindRootIndex() lookup only happens at serialization (e.g. Build) time.
Performance is not an issue. Clarified this in the code via function name
change.

Please see the main comment about the LoadRoot change.

[Erik Corry, 2012-03-17 02:49:18]

This is really great, it's good to finally have the snapshots working in the
official V8 repo.

The changes you made to use the root array more in generated code will reduce
the size of the reloc info which is good, but for the cases where it is not easy
to use the root array I decided to move the handling of that into the
serializer, rather than the macro assembler.

In the end I decided it would be easier to put your changes and my comments into
a new changelist.  Please take a look at 
https://chromiumcodereview.appspot.com/9722020

http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mip...
File src/mips/macro-assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mip...
src/mips/macro-assembler-mips.cc:793: int32_t index =
FindRootIndex(*(reinterpret_cast<Object**>(j.imm32_)));
On 2012/03/12 12:40:00, kalmard wrote:
> On 2012/02/29 14:38:13, Erik Corry wrote:
> > This will be too slow.  I suggest you make it into
> > ASSERT(FindRootIndex(...) == kInvalidRootIndex) and then fix the callers to
> use
> > LoadRoot.
> 
> The FindRootIndex() lookup only happens at serialization (e.g. Build) time.
> Performance is not an issue. Clarified this in the code via function name
> change.

In that case you should assert that the Serializer::enabled()

> 
> Please see the main comment about the LoadRoot change.

http://codereview.chromium.org/9372063/diff/17001/src/mips/assembler-mips.cc
File src/mips/assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/17001/src/mips/assembler-mips.cc#...
src/mips/assembler-mips.cc:855: }
This is gone.

http://codereview.chromium.org/9372063/diff/17001/src/mips/assembler-mips.h
File src/mips/assembler-mips.h (right):

http://codereview.chromium.org/9372063/diff/17001/src/mips/assembler-mips.h#n...
src/mips/assembler-mips.h:1012: bool SerializingTryLoadFromRoot(RelocInfo::Mode
rmode);
I got rid of this.

http://codereview.chromium.org/9372063/diff/17001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

http://codereview.chromium.org/9372063/diff/17001/src/mips/code-stubs-mips.cc...
src/mips/code-stubs-mips.cc:3959: __ Branch(throw_termination_exception, eq,
This all fits on one line now.

http://codereview.chromium.org/9372063/diff/17001/src/mips/code-stubs-mips.cc...
src/mips/code-stubs-mips.cc:7086: RelocInfo::CODE_TARGET), CONSTANT_SIZE);
I fixed some strange line breaking here.

http://codereview.chromium.org/9372063/diff/17001/src/mips/ic-mips.cc
File src/mips/ic-mips.cc (right):

http://codereview.chromium.org/9372063/diff/17001/src/mips/ic-mips.cc#newcode775
src/mips/ic-mips.cc:775: __ CheckMap(scratch1, scratch2,
Heap::kNonStrictArgumentsElementsMapRootIndex,
Arguments should be all on one line or one per line here and in some other
places.

http://codereview.chromium.org/9372063/diff/17001/src/mips/macro-assembler-mi...
File src/mips/macro-assembler-mips.cc (right):

http://codereview.chromium.org/9372063/diff/17001/src/mips/macro-assembler-mi...
src/mips/macro-assembler-mips.cc:766: if (heap->InNewSpace(heap_object)) return
kInvalidRootIndex;
Since your comment indicated that this function is only used in the serializer,
it function should contain an assertion that the serializer is active.  That
would serve both as a check and a sort of comment.  In the event I removed this
function though.

http://codereview.chromium.org/9372063/diff/17001/src/mips/macro-assembler-mi...
src/mips/macro-assembler-mips.cc:792: } else if
(SerializingTryLoadFromRoot(j.rmode_) && mode == OPTIMIZE_SIZE) {
I took out this change.

http://codereview.chromium.org/9372063/diff/17001/src/mips/macro-assembler-mi...
File src/mips/macro-assembler-mips.h (right):

http://codereview.chromium.org/9372063/diff/17001/src/mips/macro-assembler-mi...
src/mips/macro-assembler-mips.h:253: int FindRootIndex(Object* heap_object);
I got rid of this.

http://codereview.chromium.org/9372063/diff/17001/src/serialize.cc
File src/serialize.cc (right):

http://codereview.chromium.org/9372063/diff/17001/src/serialize.cc#newcode782
src/serialize.cc:782: (addr - 3 * kPointerSize)
This should be kInstructionSize (and I moved the functionality into the MIPS
assembler instead of this ifdef).

http://codereview.chromium.org/9372063/diff/17001/src/serialize.cc#newcode859
src/serialize.cc:859: reinterpret_cast<Address>(current);                       
    \
It turns out this code can be simplified a little for all architecture, since
the kFromCode instructions are only emitted if the current_was_incremented
handling needs to be special in some way, and the PATCH_SITE_ADJUST ugliness can
be moved into the assembler.

http://codereview.chromium.org/9372063/diff/17001/src/serialize.cc#newcode1014
src/serialize.cc:1014: ALL_SPACES(kBackref, kFromCode, kStartOfObject)
The order is not important here so I moved these into the ifdef above for a tiny
code size improvement without an increase in the number of ifdefs.

http://codereview.chromium.org/9372063/diff/17001/test/cctest/cctest.status
File test/cctest/cctest.status (left):

http://codereview.chromium.org/9372063/diff/17001/test/cctest/cctest.status#o...
test/cctest/cctest.status:89: 
Yay!

[kalmard, 2012-03-22 10:15:57]

Erik's replacement patch (https://chromiumcodereview.appspot.com/9722020) has
been commited (thanks!). Closing this one.