Subzero. ARM32. Vector lowering. And.

Subzero. ARM32. Vector lowering. And.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4076
R=jpp@chromium.org, kschimpf@google.com

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

[Eric Holk, 2016-01-26 23:24:23]

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

[Eric Holk, 2016-01-26 23:24:23]

This changes adds lowering for vector and.

https://codereview.chromium.org/1637173002/diff/1/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1637173002/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:620: // TODO(kschimpf) add support for these instructions
Is it okay to put TODOs for other people?

[John, 2016-01-27 15:22:21]

lgtm

https://codereview.chromium.org/1637173002/diff/1/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1637173002/diff/1/src/IceInstARM32.cpp#newcod...
src/IceInstARM32.cpp:620: // TODO(kschimpf) add support for these instructions
On 2016/01/26 23:24:23, Eric Holk wrote:
> Is it okay to put TODOs for other people?

usually the person in

TODO(person)

should be the one that is likely to know more about the issue. So yes, this is
fine. :)

but please say TODO(person): (you forgot the ':' )

https://codereview.chromium.org/1637173002/diff/1/tests_lit/assembler/arm32/a...
File tests_lit/assembler/arm32/and-vec.ll (right):

https://codereview.chromium.org/1637173002/diff/1/tests_lit/assembler/arm32/a...
tests_lit/assembler/arm32/and-vec.ll:6: ; RUN: %p2i --filetype=asm -i %s
--target=arm32 --args -O2 \
just a comment: I know that you have been using -O2 go get rid of the -Om1 gunk.
I also know that these lowerings are fairly simple (i.e. 1 High Level Inst -> 1
ARM32 Inst). But for more complicated lowerings (e.g., select), you will want to
use -Om1 (as well) to ensure that your basic lowering is sane.

[Karl, 2016-01-27 18:00:06]

lgtm modulo John's comments.

[Eric Holk, 2016-01-27 19:18:32]

Description was changed from

==========
Subzero. ARM32. Vector lowering. And.

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

to

==========
Subzero. ARM32. Vector lowering. And.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4076
R=jpp@chromium.org, kschimpf@google.com

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

[Eric Holk, 2016-01-27 19:18:33]

Committed patchset #1 (id:1) manually as
b58170c52af100fc57fe2c5f97d14e1deae02902 (presubmit successful).

[Eric Holk, 2016-01-27 19:18:36]

https://codereview.chromium.org/1637173002/diff/1/tests_lit/assembler/arm32/a...
File tests_lit/assembler/arm32/and-vec.ll (right):

https://codereview.chromium.org/1637173002/diff/1/tests_lit/assembler/arm32/a...
tests_lit/assembler/arm32/and-vec.ll:6: ; RUN: %p2i --filetype=asm -i %s
--target=arm32 --args -O2 \
On 2016/01/27 15:22:20, John wrote:
> just a comment: I know that you have been using -O2 go get rid of the -Om1
gunk.
> I also know that these lowerings are fairly simple (i.e. 1 High Level Inst ->
1
> ARM32 Inst). But for more complicated lowerings (e.g., select), you will want
to
> use -Om1 (as well) to ensure that your basic lowering is sane.

Thanks for the tip!

[Jim Stichnoth, 2016-01-27 21:57:33]

lgtm

https://codereview.chromium.org/1637173002/diff/1/tests_lit/assembler/arm32/a...
File tests_lit/assembler/arm32/and-vec.ll (right):

https://codereview.chromium.org/1637173002/diff/1/tests_lit/assembler/arm32/a...
tests_lit/assembler/arm32/and-vec.ll:1: ; Show that we know how to translate
vsub vector instructions.
vand