Hi Paul,
Awesome that you've done all this work already! This is much faster than I
had
even dreamed of. And yes we are ready for it :D
Here's an initial round of comments from me.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/code-generator-mips.cc
File src/compiler/mips/code-generator-mips.cc (right):
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/code-generator-mips.cc#newcode108
src/compiler/mips/code-generator-mips.cc:108: // messy macro?
Consider expanding this in place for sll, srl, sra ops.
I agree that you should kill this macro here.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/code-generator-mips.cc#newcode170
src/compiler/mips/code-generator-mips.cc:170: i.InputOperand(1),
kCompareReg, kScratchReg);
The macro assembler methods here generate a lot of instructions. It may
not be an overall win to generate an add with overflow on MIPS, but
instead to generate an explicit tag check with a branch and a regular
add.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/code-generator-mips.cc#newcode361
src/compiler/mips/code-generator-mips.cc:361: FlagsCondition condition)
{
The handling of branches here is tough to follow. It may be that the
flags continuation which works OK on other architectures doesn't work so
great here. In particular it seems that materializing the extra bit for
AddWithOverflow, only to feed it into a branch at the end is a bit
indirect.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-codes-mips.h
File src/compiler/mips/instruction-codes-mips.h (right):
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-codes-mips.h#newcode34
src/compiler/mips/instruction-codes-mips.h:34: V(MipsFloat64Cmp)
\
Consider naming these opcodes as close as possible to the underlying
Assembler/MacroAssembler method that they end up calling. We didn't do
that on arm but I think we want to clean that up in the future.
https://codereview.chromium.org/601723002/diff/20001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):
https://codereview.chromium.org/601723002/diff/20001/src/mips/code-stubs-mips.cc#newcode1103
src/mips/code-stubs-mips.cc:1103: // a1: pointer to builtin function
Changes here and other changes outside of TurboFan already look fine
(i.e. ready to land).
Would you consider splitting those out into a separate CL and landing
them first?
https://codereview.chromium.org/601723002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
