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#newcode853
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#newcode7055
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-mips.cc
File src/mips/macro-assembler-mips.cc (right):
http://codereview.chromium.org/9372063/diff/7001/src/mips/macro-assembler-mips.cc#newcode793
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.
http://codereview.chromium.org/9372063/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
