Thanks for the review. Updated per your comments, and rebased to today's
changes.
The changes for r9641 (Fix evaluation order of GT and LTE operators.) have
been
ported and submitted separately in http://codereview.chromium.org/8348028/,
which will need to be landed before this code will pass all tests. Note
that the
crankshaft portions of that change are now included in this issue.
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")
On 2011/10/18 14:27:58, fschneider wrote:
Long line.
Done.
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:
On 2011/10/18 14:27:58, fschneider wrote:
Remove extra new line.
Done.
http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode176
src/mips/deoptimizer-mips.cc:176: // ----- pc_after points here
On 2011/10/18 14:27:58, fschneider wrote:
With new GC I think you also need a write barrier here using
RecordWriteIntoCode
(see ia32 and other platforms).
Oops, we missed this. Thanks. Added it, and ported your refactoring from
r9687.
Also, I added an ASSERT that seemed like a good idea, and updated some
comments.
http://codereview.chromium.org/7934002/diff/9001/src/mips/deoptimizer-mips.cc#newcode194
src/mips/deoptimizer-mips.cc:194: check_code->entry());
On 2011/10/18 14:27:58, fschneider wrote:
With new GC also insert call to write barrier here.
Updated, as above.
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));
On 2011/10/18 14:27:58, fschneider wrote:
Maybe use kPointerSize instead of 4.
Done.
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));
On 2011/10/18 14:27:58, fschneider wrote:
Remove commented code. Looks as if this left over from the ARM file?
Yep. A porting artifact, now removed.
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.
On 2011/10/18 14:27:58, fschneider wrote:
Wrong register in comment: Function expected in a1.
Done.
http://codereview.chromium.org/7934002/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev