Implements the vector add instructions in the integrated ARM assembler.

Implements the vector add instructions in the integrated ARM assembler.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4334
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=2c0764e2da74e61b1ef1316980972e5177512a74

[Karl, 2016-02-01 18:02:01]

Description was changed from

==========
Implements the vector add instructions in the integrated ARM assembler.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4334
==========

to

==========
Implements the vector add instructions in the integrated ARM assembler.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4334
==========

[Karl, 2016-02-01 18:02:01]

kschimpf@google.com changed reviewers:
+ eholk@chromium.org, jpp@chromium.org, sehr@chromium.org, stichnot@chromium.org

[Jim Stichnoth, 2016-02-01 21:00:24]

lgtm

https://codereview.chromium.org/1652173002/diff/20001/src/DartARM32/assembler...
File src/DartARM32/assembler_arm.cc (right):

https://codereview.chromium.org/1652173002/diff/20001/src/DartARM32/assembler...
src/DartARM32/assembler_arm.cc:1234: endif
#endif

https://codereview.chromium.org/1652173002/diff/20001/src/DartARM32/assembler...
src/DartARM32/assembler_arm.cc:1269: #edndif
#endif

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:161: std::string("SIMD op: Don't understand element
type") +
Nit: I think this might be a bit clearer:

  ...error("SIMD op: Don't understand element type" +
std::string(typeString(ElmtTy)));

Same comment in the other .cpp file.

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:1084: emitSIMD(Opcode, ElmtTy, Qd << 1, Qn << 1, Qm <<
1, UseQRegs);
The "Qx << 1" is a bit jarring compared to the normal pattern.

What do you think of running it through a trivial self-documenting helper
function, e.g.:

  IValueT mapQtoD(IValueT EncodedQRegNum) { return EncodedQRegNum << 1; }

[Karl, 2016-02-01 21:44:40]

Description was changed from

==========
Implements the vector add instructions in the integrated ARM assembler.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4334
==========

to

==========
Implements the vector add instructions in the integrated ARM assembler.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4334
R=stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Karl, 2016-02-01 21:44:41]

Committed patchset #3 (id:40001) manually as
2c0764e2da74e61b1ef1316980972e5177512a74 (presubmit successful).

[Karl, 2016-02-01 21:44:53]

https://codereview.chromium.org/1652173002/diff/20001/src/DartARM32/assembler...
File src/DartARM32/assembler_arm.cc (right):

https://codereview.chromium.org/1652173002/diff/20001/src/DartARM32/assembler...
src/DartARM32/assembler_arm.cc:1234: endif
On 2016/02/01 21:00:23, stichnot wrote:
> #endif

Done.

https://codereview.chromium.org/1652173002/diff/20001/src/DartARM32/assembler...
src/DartARM32/assembler_arm.cc:1269: #edndif
On 2016/02/01 21:00:23, stichnot wrote:
> #endif

Done.

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:161: std::string("SIMD op: Don't understand element
type") +
On 2016/02/01 21:00:23, stichnot wrote:
> Nit: I think this might be a bit clearer:
> 
>   ...error("SIMD op: Don't understand element type" +
> std::string(typeString(ElmtTy)));
> 
> Same comment in the other .cpp file.

Since (a) the name is typeString, and (b) c++ string concatentation is either a
sdt::string, const char *, or a char, I don't see the point of constructing
another string here.

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:1084: emitSIMD(Opcode, ElmtTy, Qd << 1, Qn << 1, Qm <<
1, UseQRegs);
On 2016/02/01 21:00:23, stichnot wrote:
> The "Qx << 1" is a bit jarring compared to the normal pattern.
> 
> What do you think of running it through a trivial self-documenting helper
> function, e.g.:
> 
>   IValueT mapQtoD(IValueT EncodedQRegNum) { return EncodedQRegNum << 1; }

Done.

[Karl, 2016-02-01 21:59:00]

See CL https://codereview.chromium.org/1654803003

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.cpp
File src/IceAssemblerARM32.cpp (right):

https://codereview.chromium.org/1652173002/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:161: std::string("SIMD op: Don't understand element
type") +
On 2016/02/01 21:44:53, Karl wrote:
> On 2016/02/01 21:00:23, stichnot wrote:
> > Nit: I think this might be a bit clearer:
> > 
> >   ...error("SIMD op: Don't understand element type" +
> > std::string(typeString(ElmtTy)));
> > 
> > Same comment in the other .cpp file.
> 
> Since (a) the name is typeString, and (b) c++ string concatentation is either
a
> sdt::string, const char *, or a char, I don't see the point of constructing
> another string here.

Ok. I understand the request. Doing in separate Cl.