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#newcode361
src/compiler/mips/code-generator-mips.cc:361: FlagsCondition condition)
{
Hm, we may need to refactor the AssembleArchBranch and
AssembleArchBoolean logic a bit to make it easier for MIPS. I'll think
about that.
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)
\
On 2014/09/24 at 15:23:32, titzer wrote:
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.
Yep, it's on my TODO list for ARM.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc
File src/compiler/mips/instruction-selector-mips-unittest.cc (right):
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode20
src/compiler/mips/instruction-selector-mips-unittest.cc:20:
You should add std::ostream& operator<<(std::ostream&, const
MachInst<T>&) for this, otherwise test failures will be hard to
interpret.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode37
src/compiler/mips/instruction-selector-mips-unittest.cc:37: static const
FPCmp kFPCmpInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode66
src/compiler/mips/instruction-selector-mips-unittest.cc:66: static const
MachInst2 kLogicalInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode80
src/compiler/mips/instruction-selector-mips-unittest.cc:80: static const
MachInst2 kShiftInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode96
src/compiler/mips/instruction-selector-mips-unittest.cc:96: static const
MachInst2 kMulDivInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode111
src/compiler/mips/instruction-selector-mips-unittest.cc:111: static
const MachInst2 kModInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode123
src/compiler/mips/instruction-selector-mips-unittest.cc:123: static
const MachInst2 kFPArithInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode135
src/compiler/mips/instruction-selector-mips-unittest.cc:135: static
const MachInst2 kAddSubInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode149
src/compiler/mips/instruction-selector-mips-unittest.cc:149: static
const MachInst1 kAddSubOneInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode161
src/compiler/mips/instruction-selector-mips-unittest.cc:161: static
const IntCmp kCmpInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode194
src/compiler/mips/instruction-selector-mips-unittest.cc:194: static
const Conversion kConversionInstructions[] = {
Nit: no need to use static, it's in an anonymous namespace already.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode512
src/compiler/mips/instruction-selector-mips-unittest.cc:512:
You should add
std::ostream& operator<<(std::ostream& os, const MemoryAccessImm& acc) {
OStringStream ost;
ost << acc.type;
return os << ost.c_str();
}
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode522
src/compiler/mips/instruction-selector-mips-unittest.cc:522:
Stream operator, see above.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode529
src/compiler/mips/instruction-selector-mips-unittest.cc:529: static
const MemoryAccessImm kMemoryAccessesImm[] = {
Nit: static, see above.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode581
src/compiler/mips/instruction-selector-mips-unittest.cc:581: static
const MemoryAccessImm1 kMemoryAccessImmMoreThan16bit[] = {
Nit: static, see above.
https://codereview.chromium.org/601723002/diff/20001/src/compiler/mips/instruction-selector-mips-unittest.cc#newcode795
src/compiler/mips/instruction-selector-mips-unittest.cc:795:
Nit: extra new line :-)
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.
