[Subzero][MIPS] Implement 64-bit integer compare operations

[Subzero][MIPS] Implement 64-bit integer compare operations

R=stichnot@chromium.org

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

[sagar.thakur, 2016-09-27 09:40:25]

sagar.thakur@imgtec.com changed reviewers:
+ eholk@chromium.org, jpp@chromium.org, kschimpf@google.com,
srdjan.obucina@imgtec.com, stichnot@chromium.org

[Jim Stichnoth, 2016-09-27 13:45:35]

https://codereview.chromium.org/2369323002/diff/20001/src/IceTargetLoweringMI...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2369323002/diff/20001/src/IceTargetLoweringMI...
src/IceTargetLoweringMIPS32.cpp:2924: _sltiu(Dest, T3, 1);
Dest isn't necessarily a physical register (and probably won't be in Om1 mode),
so I think you need to go through another temporary:

  _sltiu(T4, T3, 1);
  _mov(Dest, T4);

Same for the InstIcmp::Ne case.

https://codereview.chromium.org/2369323002/diff/20001/tests_lit/llvm2ice_test...
File tests_lit/llvm2ice_tests/64bit.pnacl.ll (right):

https://codereview.chromium.org/2369323002/diff/20001/tests_lit/llvm2ice_test...
tests_lit/llvm2ice_tests/64bit.pnacl.ll:1668: ; MIPS32-O2: move	{{.*}},[[T2]]
The "MIPS32-O2" and "MIPS32-OM1" lines don't actually do anything because MIPS32
is only being tested with "--check-prefix MIPS32".

It would be great if you could add change this file so that both O2 and Om1 are
being tested, but it's possible that could end up being too big for this CL.

Otherwise, the simplest may be to add "--check-prefix MIPS32-O2" to the RUN
command and handle MIPS32-OM1 later.

[sagar.thakur, 2016-09-28 08:23:23]

Addressed review comments

https://codereview.chromium.org/2369323002/diff/20001/src/IceTargetLoweringMI...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2369323002/diff/20001/src/IceTargetLoweringMI...
src/IceTargetLoweringMIPS32.cpp:2924: _sltiu(Dest, T3, 1);
On 2016/09/27 13:45:35, stichnot wrote:
> Dest isn't necessarily a physical register (and probably won't be in Om1
mode),
> so I think you need to go through another temporary:
> 
>   _sltiu(T4, T3, 1);
>   _mov(Dest, T4);
> 
> Same for the InstIcmp::Ne case.

Done.

https://codereview.chromium.org/2369323002/diff/20001/tests_lit/llvm2ice_test...
File tests_lit/llvm2ice_tests/64bit.pnacl.ll (right):

https://codereview.chromium.org/2369323002/diff/20001/tests_lit/llvm2ice_test...
tests_lit/llvm2ice_tests/64bit.pnacl.ll:1668: ; MIPS32-O2: move	{{.*}},[[T2]]
On 2016/09/27 13:45:35, stichnot wrote:
> The "MIPS32-O2" and "MIPS32-OM1" lines don't actually do anything because
MIPS32
> is only being tested with "--check-prefix MIPS32".
> 
> It would be great if you could add change this file so that both O2 and Om1
are
> being tested, but it's possible that could end up being too big for this CL.
> 
> Otherwise, the simplest may be to add "--check-prefix MIPS32-O2" to the RUN
> command and handle MIPS32-OM1 later.

Done. I will add RUN command line for Om1 in a separate CL.

[Jim Stichnoth, 2016-09-28 13:36:34]

Description was changed from

==========
[Subzero][MIPS] Implement 64-bit interger compare operations
==========

to

==========
[Subzero][MIPS] Implement 64-bit integer compare operations
==========

[Jim Stichnoth, 2016-09-28 13:36:40]

lgtm

[Jim Stichnoth, 2016-09-28 13:36:49]

Description was changed from

==========
[Subzero][MIPS] Implement 64-bit integer compare operations
==========

to

==========
[Subzero][MIPS] Implement 64-bit integer compare operations

R=stichnot@chromium.org

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

[Jim Stichnoth, 2016-09-28 13:36:52]

Committed patchset #3 (id:40001) manually as
70b6ed47d1a4692408a0adbe025329c6adcb5c10 (presubmit successful).