Add VCMP{s,sz,d,dz} Instructions to ARM integrated assembler.

Add VCMP{s,sz,d,dz} Instructions to ARM integrated assembler.

Also fixes bug in emitVFPddd.

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=cd5e07ec935e179d4d087379afb480c3897d8316

[Karl, 2016-01-08 16:44:29]

Description was changed from

==========
Add VCMP{s,sz,d,dz} Instructions to ARM integrated assembler.

Also fixes bug in emitVFPddd.

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

to

==========
Add VCMP{s,sz,d,dz} Instructions to ARM integrated assembler.

Also fixes bug in emitVFPddd.

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

[Karl, 2016-01-08 16:44:29]

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

[Jim Stichnoth, 2016-01-09 01:34:28]

lgtm

https://codereview.chromium.org/1564393002/diff/20001/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1564393002/diff/20001/src/IceAssemblerARM32.h...
src/IceAssemblerARM32.h:329: void vcmpdz(const Operand *OpDd, CondARM32::Cond
cond); // compare with 0.0
Can you clarify the comment?  Does it matter whether it's +0.0 or -0.0?

https://codereview.chromium.org/1564393002/diff/20001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1564393002/diff/20001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:1929: Type Ty = Src0->getType();
const Type

https://codereview.chromium.org/1564393002/diff/20001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vcmp.ll (right):

https://codereview.chromium.org/1564393002/diff/20001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vcmp.ll:6: ; RUN: %p2i --filetype=asm -i %s
--target=arm32 --args -Om1 \
Can you use -O2 like in many/most other tests, to simplify the output to test?

[Karl, 2016-01-11 16:42:20]

PLEASE REVIEW.

I simplified the lit test, and hence I am sending it for re-review.

https://codereview.chromium.org/1564393002/diff/20001/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1564393002/diff/20001/src/IceAssemblerARM32.h...
src/IceAssemblerARM32.h:329: void vcmpdz(const Operand *OpDd, CondARM32::Cond
cond); // compare with 0.0
On 2016/01/09 01:34:28, stichnot wrote:
> Can you clarify the comment?  Does it matter whether it's +0.0 or -0.0?

Done.

https://codereview.chromium.org/1564393002/diff/20001/src/IceInstARM32.cpp
File src/IceInstARM32.cpp (right):

https://codereview.chromium.org/1564393002/diff/20001/src/IceInstARM32.cpp#ne...
src/IceInstARM32.cpp:1929: Type Ty = Src0->getType();
On 2016/01/09 01:34:28, stichnot wrote:
> const Type

Done.

https://codereview.chromium.org/1564393002/diff/20001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vcmp.ll (right):

https://codereview.chromium.org/1564393002/diff/20001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vcmp.ll:6: ; RUN: %p2i --filetype=asm -i %s
--target=arm32 --args -Om1 \
On 2016/01/09 01:34:28, stichnot wrote:
> Can you use -O2 like in many/most other tests, to simplify the output to test?

If you use -O2, you will get different register assignments. Since the register
mask CL is not ready yet, I limited it to -Om1.

On the other hand, I realized over the weekend that would could greatly simplify
these tests by taking advantage of the disassembly more. That is, we know the
vcmp instructions got translated if the dissassembly is the same, and if "vcmp"
does not appear in the hybrid assembly text.

Hence, I have simplified this test accordingly.

[Jim Stichnoth, 2016-01-11 17:27:04]

still lgtm

https://codereview.chromium.org/1564393002/diff/40001/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1564393002/diff/40001/src/IceAssemblerARM32.h...
src/IceAssemblerARM32.h:329: // Second argument of comare is zero (+0.0).
compare

https://codereview.chromium.org/1564393002/diff/40001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vcmp.ll (right):

https://codereview.chromium.org/1564393002/diff/40001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vcmp.ll:5: ; TODO(kschimpf): Use include registers for
compare instructions, so
Reflow to 80-col?

[Karl, 2016-01-11 18:12:26]

Description was changed from

==========
Add VCMP{s,sz,d,dz} Instructions to ARM integrated assembler.

Also fixes bug in emitVFPddd.

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

to

==========
Add VCMP{s,sz,d,dz} Instructions to ARM integrated assembler.

Also fixes bug in emitVFPddd.

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-01-11 18:12:27]

Committed patchset #4 (id:60001) manually as
cd5e07ec935e179d4d087379afb480c3897d8316 (presubmit successful).

[Karl, 2016-01-11 18:12:33]

https://codereview.chromium.org/1564393002/diff/40001/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1564393002/diff/40001/src/IceAssemblerARM32.h...
src/IceAssemblerARM32.h:329: // Second argument of comare is zero (+0.0).
On 2016/01/11 17:27:03, stichnot wrote:
> compare

Done.

https://codereview.chromium.org/1564393002/diff/40001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vcmp.ll (right):

https://codereview.chromium.org/1564393002/diff/40001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vcmp.ll:5: ; TODO(kschimpf): Use include registers for
compare instructions, so
On 2016/01/11 17:27:03, stichnot wrote:
> Reflow to 80-col?

Done.