http://codereview.chromium.org/8491008/diff/13001/src/mips/assembler-mips-inl.h File src/mips/assembler-mips-inl.h (right):
http://codereview.chromium.org/8491008/diff/13001/src/mips/assembler-mips-inl.h#newcode111 src/mips/assembler-mips-inl.h:111: || rmode_ == EXTERNAL_REFERENCE); The funky indentation here doesn't really add anything. Let's just stick with one subexpression per line with a || at the end of each line like we do in the rest of the VM. http://codereview.chromium.org/8491008/diff/13001/src/mips/assembler-mips-inl.h#newcode117 src/mips/assembler-mips-inl.h:117: // For an instructions like LUI/ORI where the target bits are mixed into the s/an instructions/an instruction/ http://codereview.chromium.org/8491008/diff/13001/src/mips/assembler-mips-inl.h#newcode123 src/mips/assembler-mips-inl.h:123: // place, ready to be patched with the target. In our case, after jump s/In our case// http://codereview.chromium.org/8491008/diff/13001/src/serialize.cc File src/serialize.cc (right): http://codereview.chromium.org/8491008/diff/13001/src/serialize.cc#newcode764 src/serialize.cc:764: static const int kInstructionsForSplitImmediate = 3*kPointerSize; Inconsistency: This constant is premultiplied by kPOinterSize, whereas kInstructionsFor32BitConstant is measured in instructions and has to be multiplied by kInstructionSize when used. http://codereview.chromium.org/8491008/diff/13001/src/serialize.cc#newcode838 src/serialize.cc:838: if (Assembler::kCallTargetSize == 0) { \ The size of a call target is only relevant if we are dealing with a call instruction. In that case 'within == kFirstInstruction' so we will get into the 'if' just below, which already sets current_was_incremented. In other words I think this can be simplified. http://codereview.chromium.org/8491008/diff/13001/src/serialize.cc#newcode978 src/serialize.cc:978: ONE_PER_SPACE(kNewObject, kFromCode, kStartOfObject) I'm a bit worried about the code size here. The deserialization is performance critical for startup and context creation, and this macro expands a lot. I'd like this to be in an ifdef MIPS. The others in this file don't expand as much. http://codereview.chromium.org/8491008/diff/13001/src/serialize.cc#newcode1014 src/serialize.cc:1014: CASE_STATEMENT(kRootArray, kFromCode, kStartOfObject, 0) Can we replace these with LoadRoot which does the load relative to a root array array and needs no relocation info? http://codereview.chromium.org/8491008/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
