Subzero, MIPS32: Encoding of FP comparison instructions

Subzero, MIPS32: Encoding of FP comparison instructions

Patch implements encoding for instructions used for floating point number comparison.

R=stichnot@chromium.org

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

[obucinac, 2016-09-19 17:41:12]

Description was changed from

==========
Subzero, MIPS32: Encoding of FP comparison instructions

Patch implements encoding for instructions used for floating point
number comparison.
==========

to

==========
Subzero, MIPS32: Encoding of FP comparison instructions

Patch implements encoding for instructions used for floating point number
comparison.
==========

[obucinac, 2016-09-19 17:41:12]

obucinac@gmail.com changed reviewers:
+ eholk@chromium.org, jaydeep.patil@imgtec.com, jpp@chromium.org,
kschimpf@google.com, rich_fuhler@imgtec.com, sagar.thakur@imgtec.com,
stichnot@chromium.org

[obucinac, 2016-09-19 19:04:51]

obucinac@gmail.com changed reviewers:
- rich_fuhler@imgtec.com

[Jim Stichnoth, 2016-09-20 00:09:23]

This is nice.  The real test will be in the cross tests.

https://codereview.chromium.org/2350833002/diff/1/src/IceAssemblerMIPS32.cpp
File src/IceAssemblerMIPS32.cpp (right):

https://codereview.chromium.org/2350833002/diff/1/src/IceAssemblerMIPS32.cpp#...
src/IceAssemblerMIPS32.cpp:607: IValueT InstEncodingFalse = 0;
const or constexpr

https://codereview.chromium.org/2350833002/diff/1/src/IceAssemblerMIPS32.cpp#...
src/IceAssemblerMIPS32.cpp:636: IValueT InstEncodingTrue = 1;
const or constexpr

https://codereview.chromium.org/2350833002/diff/1/src/IceInstMIPS32.h
File src/IceInstMIPS32.h (right):

https://codereview.chromium.org/2350833002/diff/1/src/IceInstMIPS32.h#newcode79
src/IceInstMIPS32.h:79: typedef enum { FCC0 = 0, FCC1, FCC2, FCC3, FCC4, FCC5,
FCC6, FCC7 } FCC;
Is this change necessary?  That pattern is used a lot in the code base.

If it is necessary, prefer "using" over "typedef".

(Actually I bet this was only a problem before the FpCondCode renaming.)

https://codereview.chromium.org/2350833002/diff/1/tests_lit/assembler/mips32/...
File tests_lit/assembler/mips32/encoding_test_fcmp.ll (right):

https://codereview.chromium.org/2350833002/diff/1/tests_lit/assembler/mips32/...
tests_lit/assembler/mips32/encoding_test_fcmp.ll:1: ; Test encoding of MIPS32
floating point comaprison
comparison

though when you work with floating-point long enough, it may feel like a
coma/prison...  :)

[obucinac, 2016-09-20 13:24:52]

https://codereview.chromium.org/2350833002/diff/1/src/IceAssemblerMIPS32.cpp
File src/IceAssemblerMIPS32.cpp (right):

https://codereview.chromium.org/2350833002/diff/1/src/IceAssemblerMIPS32.cpp#...
src/IceAssemblerMIPS32.cpp:607: IValueT InstEncodingFalse = 0;
On 2016/09/20 00:09:23, stichnot wrote:
> const or constexpr

Done.

https://codereview.chromium.org/2350833002/diff/1/src/IceAssemblerMIPS32.cpp#...
src/IceAssemblerMIPS32.cpp:636: IValueT InstEncodingTrue = 1;
On 2016/09/20 00:09:23, stichnot wrote:
> const or constexpr

Done.

https://codereview.chromium.org/2350833002/diff/1/src/IceInstMIPS32.h
File src/IceInstMIPS32.h (right):

https://codereview.chromium.org/2350833002/diff/1/src/IceInstMIPS32.h#newcode79
src/IceInstMIPS32.h:79: typedef enum { FCC0 = 0, FCC1, FCC2, FCC3, FCC4, FCC5,
FCC6, FCC7 } FCC;
On 2016/09/20 00:09:23, stichnot wrote:
> Is this change necessary?  That pattern is used a lot in the code base.
> 
> If it is necessary, prefer "using" over "typedef".
> 
> (Actually I bet this was only a problem before the FpCondCode renaming.)

Done.

https://codereview.chromium.org/2350833002/diff/1/tests_lit/assembler/mips32/...
File tests_lit/assembler/mips32/encoding_test_fcmp.ll (right):

https://codereview.chromium.org/2350833002/diff/1/tests_lit/assembler/mips32/...
tests_lit/assembler/mips32/encoding_test_fcmp.ll:1: ; Test encoding of MIPS32
floating point comaprison
On 2016/09/20 00:09:23, stichnot wrote:
> comparison
> 
> though when you work with floating-point long enough, it may feel like a
> coma/prison...  :)

Done.

[Jim Stichnoth, 2016-09-20 15:44:35]

lgtm

[Jim Stichnoth, 2016-09-20 15:44:47]

Description was changed from

==========
Subzero, MIPS32: Encoding of FP comparison instructions

Patch implements encoding for instructions used for floating point number
comparison.
==========

to

==========
Subzero, MIPS32: Encoding of FP comparison instructions

Patch implements encoding for instructions used for floating point number
comparison.

R=stichnot@chromium.org

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

[Jim Stichnoth, 2016-09-20 15:44:50]

Committed patchset #2 (id:20001) manually as
8d16c1d3bbdb6259ad346ff9c0d0e3229a03e1e8 (presubmit successful).