First round of feedback. I looked at the files up to including
lithium-codegen.h.




http://codereview.chromium.org/7934002/diff/9001/src/flag-definitions.h
File src/flag-definitions.h (right):

http://codereview.chromium.org/7934002/diff/9001/src/flag-definitions.h#newcode322
src/flag-definitions.h:322: DEFINE_bool(check_icache, false, "Check
icache flushes in ARM and MIPS simulator")
Long line.

http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc
File src/mips/deoptimizer-mips.cc (right):

http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode116
src/mips/deoptimizer-mips.cc:116:
Remove extra new line.

http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode176
src/mips/deoptimizer-mips.cc:176: // ----- pc_after points here
With new GC I think you also need a write barrier here using
RecordWriteIntoCode (see ia32 and other platforms).

http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode194
src/mips/deoptimizer-mips.cc:194: check_code->entry());
With new GC also insert call to write barrier here.

http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode589
src/mips/deoptimizer-mips.cc:589: __ sw(ToRegister(i), MemOperand(sp, 4
* i));
Maybe use kPointerSize instead of 4.

http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode705
src/mips/deoptimizer-mips.cc:705: // __ add(r1, r0, Operand(r1, LSL,
2));
Remove commented code. Looks as if this left over from the ARM file?

http://codereview.chromium.org/7934002/diff/9001/src/mips/lithium-codegen-mips.h
File src/mips/lithium-codegen-mips.h (right):

http://codereview.chromium.org/7934002/diff/9001/src/mips/lithium-codegen-mips.h#newcode210
src/mips/lithium-codegen-mips.h:210: // to be in edi.
Wrong register in comment: Function expected in a1.

http://codereview.chromium.org/7934002/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to