Thanks for the review,
I'll be working on branch delay slots. I think I will make the
MacroAssembler
generate nop instructions. Using the branch delay slot would then need to
use
directly the Assembler instead.
I will also add some tests for function calls. Rather than adding tests in
test-assembler-mips.cc for such 'high level' tests, shouldn't I add a
test-mips-codegen.cc file in which future tests could also be implemented?
Paul from MIPS has also fixed some bugs and implemented more tests. We will
upload the patch after this issue.
Regards,
Alexandre
http://codereview.chromium.org/1018001/diff/16001/17002
File src/mips/builtins-mips.cc (right):
http://codereview.chromium.org/1018001/diff/16001/17002#newcode78
src/mips/builtins-mips.cc:78:
On 2010/03/17 14:49:09, Søren Gjesse wrote:
Please format these comments without *'s, also there is no need to
have _'s in
register content description.
// Registers
// a0: entry address
// a1: function
// ..
//
// Stack
// ...
Done.
http://codereview.chromium.org/1018001/diff/16001/17002#newcode87
src/mips/builtins-mips.cc:87: // * Stack:
On 2010/03/17 14:49:09, Søren Gjesse wrote:
All other comments holding a stack layout have TOS first.
Using TOS first from now on.
http://codereview.chromium.org/1018001/diff/16001/17002#newcode121
src/mips/builtins-mips.cc:121: __ lw(t0, MemOperand(s0)); // read next
parameter
On 2010/03/17 14:49:09, Søren Gjesse wrote:
read next parameter -> Read next parameter. Also for two more comments
below.
(I know we have end of line comments which does not have uppercase and
period,
but we are trying to avoid these).
Done.
http://codereview.chromium.org/1018001/diff/16001/17007
File src/mips/codegen-mips.cc (right):
http://codereview.chromium.org/1018001/diff/16001/17007#newcode129
src/mips/codegen-mips.cc:129:
On 2010/03/17 14:49:09, Søren Gjesse wrote:
See other comment regarding the format these state comments.
Done.
http://codereview.chromium.org/1018001/diff/16001/17007#newcode246
src/mips/codegen-mips.cc:246: masm_->addiu(sp, sp,
(scope()->num_parameters() + 1)*kPointerSize);
On 2010/03/17 14:49:09, Søren Gjesse wrote:
Spaces around binary operation.
Done.
http://codereview.chromium.org/1018001/diff/16001/17007#newcode1155
src/mips/codegen-mips.cc:1155: // check for failure result
On 2010/03/17 14:49:09, Søren Gjesse wrote:
Format comment with uppercase letter and period - more below.
Done.
http://codereview.chromium.org/1018001/diff/16001/17005
File src/mips/ic-mips.cc (right):
http://codereview.chromium.org/1018001/diff/16001/17005#newcode76
src/mips/ic-mips.cc:76: UNIMPLEMENTED_MIPS();
On 2010/03/17 14:49:09, Søren Gjesse wrote:
This code is not really used.
Indeed. I remove it.
http://codereview.chromium.org/1018001/diff/16001/17011
File src/mips/stub-cache-mips.cc (right):
http://codereview.chromium.org/1018001/diff/16001/17011#newcode162
src/mips/stub-cache-mips.cc:162: Object*
StubCompiler::CompileLazyCompile(Code::Flags flags) {
On 2010/03/17 14:49:09, Søren Gjesse wrote:
Please format the comment without *'s, e.g.
// State
// a1: function
// ra: return address
Done.
http://codereview.chromium.org/1018001
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
