Begins implementation of ARM neon instructions.

Begins implementation of ARM neon instructions.

Just adds the vadd instruction, but also changes the
arm assembler to expose the Q registers rather than the
D registers. Q0 = D1:D0 = S3:S2:S1:S0. This is similar
to how ia32 and x64 use only the xmm registers for
floating point.

R=regis@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24982

[regis, 2013-07-12 22:16:01]

LGTM with some suggestions for naming convention.

https://codereview.chromium.org/18684008/diff/10001/runtime/platform/globals.h
File runtime/platform/globals.h (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/platform/globals....
runtime/platform/globals.h:126: #if defined(TARGET_ARCH_ARM)
It looks like TARGET_ARCH_ARM is not special anymore and can be handled by the
else part.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm.cc
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.cc:2060: DRegister firstv =
static_cast<DRegister>(kDartFirstVolatileFpuReg * 2);
An inlined function or #define hiding the static cast would make this more
readable.
Something like
EvenDRegisterOf(QRegister)
OddDRegisterOf(QRegister)

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.cc:2061: DRegister lastv =
static_cast<DRegister>(kDartLastVolatileFpuReg * 2);
You miss one DRegister here, *2+1, or use the second macro above.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.cc:2079: DRegister lastv =
static_cast<DRegister>(kDartLastVolatileFpuReg * 2);
ditto

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.h:482: void vaddi(int sz, QRegister qd, QRegister qn,
QRegister qm);
I think this would be a good time to merge the enums LoadOperandType and
StoreOperandType to a single OperandSize enum:
  kByte,
  kUnsignedByte,
  kHalfword,
  kUnsignedHalfword,
  kWord,
  kUnsignedWord,   <-- new
  kWordPair,
  kSWord,
  kDWord,

Then, how about splitting this instruction in two:
vaddqi(OperandSize sz, QRegister qd, QRegister qn, QRegister qm)
vadddi(OperandSize sz, DRegister dd, DRegister dn, DRegister dm)

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.h:483: void vaddf(QRegister qd, QRegister qn, QRegister
qm);
How about
void vaddqs(QRegister qd, QRegister qn, QRegister qm);
and
void vaddds(DRegister dd, DRegister dn, DRegister dm);

VADDL and VADDW would be:
void vaddql(OperandSize sz, QRegister qd, DRegister dn, DRegister dm);
void vaddqw(OperandSize sz, QRegister qd, QRegister qn, DRegister dm);

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/flow_graph_com...
File runtime/vm/flow_graph_compiler_arm.cc (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/flow_graph_com...
runtime/vm/flow_graph_compiler_arm.cc:1509: DRegister d2 =
static_cast<DRegister>(fpu_reg * 2 + 1);
Use macros

[zra, 2013-07-12 23:22:17]

Thanks!

https://codereview.chromium.org/18684008/diff/10001/runtime/platform/globals.h
File runtime/platform/globals.h (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/platform/globals....
runtime/platform/globals.h:126: #if defined(TARGET_ARCH_ARM)
On 2013/07/12 22:16:01, regis wrote:
> It looks like TARGET_ARCH_ARM is not special anymore and can be handled by the
> else part.

Done.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm.cc
File runtime/vm/assembler_arm.cc (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.cc:2060: DRegister firstv =
static_cast<DRegister>(kDartFirstVolatileFpuReg * 2);
On 2013/07/12 22:16:01, regis wrote:
> An inlined function or #define hiding the static cast would make this more
> readable.
> Something like
> EvenDRegisterOf(QRegister)
> OddDRegisterOf(QRegister)

Done.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.cc:2061: DRegister lastv =
static_cast<DRegister>(kDartLastVolatileFpuReg * 2);
On 2013/07/12 22:16:01, regis wrote:
> You miss one DRegister here, *2+1, or use the second macro above.

Done.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.cc:2079: DRegister lastv =
static_cast<DRegister>(kDartLastVolatileFpuReg * 2);
On 2013/07/12 22:16:01, regis wrote:
> ditto

Done.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm.h
File runtime/vm/assembler_arm.h (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.h:482: void vaddi(int sz, QRegister qd, QRegister qn,
QRegister qm);
On 2013/07/12 22:16:01, regis wrote:
> I think this would be a good time to merge the enums LoadOperandType and
> StoreOperandType to a single OperandSize enum:
>   kByte,
>   kUnsignedByte,
>   kHalfword,
>   kUnsignedHalfword,
>   kWord,
>   kUnsignedWord,   <-- new
>   kWordPair,
>   kSWord,
>   kDWord,

I'll tackle this in my next CL.

> Then, how about splitting this instruction in two:
> vaddqi(OperandSize sz, QRegister qd, QRegister qn, QRegister qm)
> vadddi(OperandSize sz, DRegister dd, DRegister dn, DRegister dm)

Sounds reasonable. Changed. (I'll add the vadddi in a subsequent change.)

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/assembler_arm....
runtime/vm/assembler_arm.h:483: void vaddf(QRegister qd, QRegister qn, QRegister
qm);
On 2013/07/12 22:16:01, regis wrote:
> How about
> void vaddqs(QRegister qd, QRegister qn, QRegister qm);
> and
> void vaddds(DRegister dd, DRegister dn, DRegister dm);

Done.

> VADDL and VADDW would be:
> void vaddql(OperandSize sz, QRegister qd, DRegister dn, DRegister dm);
> void vaddqw(OperandSize sz, QRegister qd, QRegister qn, DRegister dm);

sgtm.

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/flow_graph_com...
File runtime/vm/flow_graph_compiler_arm.cc (right):

https://codereview.chromium.org/18684008/diff/10001/runtime/vm/flow_graph_com...
runtime/vm/flow_graph_compiler_arm.cc:1509: DRegister d2 =
static_cast<DRegister>(fpu_reg * 2 + 1);
On 2013/07/12 22:16:01, regis wrote:
> Use macros

Done.

[zra, 2013-07-12 23:22:57]

Committed patchset #5 manually as r24982 (presubmit successful).

[regis, 2013-07-16 00:18:42]

LGTM with a few suggestions.

https://codereview.chromium.org/18684008/diff/23002/runtime/vm/flow_graph_com...
File runtime/vm/flow_graph_compiler_arm.cc (right):

https://codereview.chromium.org/18684008/diff/23002/runtime/vm/flow_graph_com...
runtime/vm/flow_graph_compiler_arm.cc:1515: __ vstrd(d2, Address(SP, offset + 2
* kWordSize));
Fine for now, but you should add a TODO (my name is fine :-) to make use of the
vstm instruction that could save some code.

https://codereview.chromium.org/18684008/diff/23002/runtime/vm/flow_graph_com...
runtime/vm/flow_graph_compiler_arm.cc:1551: __ vldrd(d2, Address(SP, offset + 2
* kWordSize));
TODO(regis): Use vldm instruction.

https://codereview.chromium.org/18684008/diff/23002/runtime/vm/intermediate_l...
File runtime/vm/intermediate_language_arm.cc (right):

https://codereview.chromium.org/18684008/diff/23002/runtime/vm/intermediate_l...
runtime/vm/intermediate_language_arm.cc:3116: // QRegister result =
locs()->out().fpu_reg();
Why are those lines commented out?