[Subzero][MIPS32] Implement bitcast operation for both 32-bit and 64-bit operands

[Subzero][MIPS32] Implement bitcast operation for both 32-bit and 64-bit operands

R=stichnot@chromium.org

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

[sagar.thakur, 2016-10-10 09:57:45]

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

[Jim Stichnoth, 2016-10-10 17:07:50]

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

https://codereview.chromium.org/2404803002/diff/1/src/IceInstMIPS32.h#newcode...
src/IceInstMIPS32.h:1219: bool isMultiDest() const { return false; }
Are these meant to be permanent changes?  (i.e. isMultiDest() and
isMultiSource() always return false)
If so, can you just remove these functions and change their users accordingly?

https://codereview.chromium.org/2404803002/diff/1/src/IceTargetLoweringMIPS32...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2404803002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:1565: int32_t typeWidth =
typeWidthInBytes(Dest->getType());
Capitalize TypeWidth variable per LLVM convention.

Also, I think you can use DestTy instead of Dest->getType().

https://codereview.chromium.org/2404803002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:1679: } else if (DestTy == IceType_f64 &&
IsSrcGPReg == true) {
remove "== true"

[sagar.thakur, 2016-10-12 06:07:06]

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

https://codereview.chromium.org/2404803002/diff/1/src/IceInstMIPS32.h#newcode...
src/IceInstMIPS32.h:1219: bool isMultiDest() const { return false; }
On 2016/10/10 17:07:49, Jim Stichnoth wrote:
> Are these meant to be permanent changes?  (i.e. isMultiDest() and
> isMultiSource() always return false)
> If so, can you just remove these functions and change their users accordingly?

Yes, a mov instruction will always have single destination and source.
Removed these functions and their uses.

https://codereview.chromium.org/2404803002/diff/1/src/IceTargetLoweringMIPS32...
File src/IceTargetLoweringMIPS32.cpp (right):

https://codereview.chromium.org/2404803002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:1565: int32_t typeWidth =
typeWidthInBytes(Dest->getType());
On 2016/10/10 17:07:49, Jim Stichnoth wrote:
> Capitalize TypeWidth variable per LLVM convention.
> 
> Also, I think you can use DestTy instead of Dest->getType().

Done.

https://codereview.chromium.org/2404803002/diff/1/src/IceTargetLoweringMIPS32...
src/IceTargetLoweringMIPS32.cpp:1679: } else if (DestTy == IceType_f64 &&
IsSrcGPReg == true) {
On 2016/10/10 17:07:49, Jim Stichnoth wrote:
> remove "== true"

Done.

[Jim Stichnoth, 2016-10-12 06:35:47]

lgtm

https://codereview.chromium.org/2404803002/diff/20001/tests_lit/llvm2ice_test...
File tests_lit/llvm2ice_tests/bitcast.ll (right):

https://codereview.chromium.org/2404803002/diff/20001/tests_lit/llvm2ice_test...
tests_lit/llvm2ice_tests/bitcast.ll:16: ; RUN: %if --need=target_MIPS32
--command %p2i --filetype=asm \
These new tests fail in a MINIMAL build because filetype=asm isn't supported in
a MINIMAL build.

The fix is to add "--need=allow_dump" to all 4 %if commands.

(I went ahead and made this change.)

[Jim Stichnoth, 2016-10-12 06:36:03]

Description was changed from

==========
[Subzero][MIPS32] Implement bitcast operation for both 32-bit and 64-bit
operands
==========

to

==========
[Subzero][MIPS32] Implement bitcast operation for both 32-bit and 64-bit
operands

R=stichnot@chromium.org

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

[Jim Stichnoth, 2016-10-12 06:36:05]

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