Add fabs(<4 x float>) to the integrated ARM assembler.

Add fabs(<4 x float>) to the integrated ARM assembler.

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

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

[Karl, 2016-02-16 21:42:26]

Description was changed from

==========
Add fabs(<4 x float>) to the integrated ARM assembler.

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

to

==========
Add fabs(<4 x float>) to the integrated ARM assembler.

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

[Karl, 2016-02-16 21:42:26]

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

[Eric Holk, 2016-02-16 22:31:34]

lgtm

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

https://codereview.chromium.org/1697263007/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:1155: (getYInRegYXXXX(Dd) << 22) |
(getXXXXInRegYXXXX(Dn) << 16) |
Optional, but should these other shift amounts be given a name, like
kConditionShift?

https://codereview.chromium.org/1697263007/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:3126: constexpr IValueT VsubqfOpcode = B21 | B11 | B8;
Was setting B10 before a bug?

[Jim Stichnoth, 2016-02-17 03:58:40]

lgtm

https://codereview.chromium.org/1697263007/diff/20001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vabs-vec.ll (right):

https://codereview.chromium.org/1697263007/diff/20001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vabs-vec.ll:1: ; Show that we translate intrinsics for
fabs on vectors.
There's only one such intrinsic, right?  Maybe a comments like:

Show that we know how to translate the fabs intrinsic on vectors.

[Karl, 2016-02-17 17:51:25]

Description was changed from

==========
Add fabs(<4 x float>) to the integrated ARM assembler.

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

to

==========
Add fabs(<4 x float>) to the integrated ARM assembler.

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

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

[Karl, 2016-02-17 17:51:27]

Committed patchset #3 (id:40001) manually as
425e06b05945520e3182e4ccf2bb3cb4db7f8690 (presubmit successful).

[Karl, 2016-02-17 17:51:45]

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

https://codereview.chromium.org/1697263007/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:1155: (getYInRegYXXXX(Dd) << 22) |
(getXXXXInRegYXXXX(Dn) << 16) |
On 2016/02/16 22:31:34, Eric Holk wrote:
> Optional, but should these other shift amounts be given a name, like
> kConditionShift?

DART has been somewhat inconsistent on this. I have (somewhat) followed the same
pattern. However, when I do notice that several methods use the same
(semantically meaningful) shift I do generate symbolic constants.

https://codereview.chromium.org/1697263007/diff/20001/src/IceAssemblerARM32.c...
src/IceAssemblerARM32.cpp:3126: constexpr IValueT VsubqfOpcode = B21 | B11 | B8;
On 2016/02/16 22:31:34, Eric Holk wrote:
> Was setting B10 before a bug?

No. I noticed that whenever there were V-based instructions operating on
float/double values, this bit is set. Hence, I have moved the inclusion of the
bit happen at a lower level (since the type is passed to lower level routines
anyway).

https://codereview.chromium.org/1697263007/diff/20001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vabs-vec.ll (right):

https://codereview.chromium.org/1697263007/diff/20001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vabs-vec.ll:1: ; Show that we translate intrinsics for
fabs on vectors.
On 2016/02/17 03:58:39, stichnot wrote:
> There's only one such intrinsic, right?  Maybe a comments like:
> 
> Show that we know how to translate the fabs intrinsic on vectors.

Done.