Subzero, MIPS32: lowerSelect for i64

Subzero, MIPS32: lowerSelect for i64

Implements lowerSelect for i64.

R=stichnot@chromium.org

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

[obucinac, 2016-09-23 17:53:22]

Description was changed from

==========
Subzero, MIPS32: lowerSelect for i64

Implements lowerSelect for i64.
==========

to

==========
Subzero, MIPS32: lowerSelect for i64

Implements lowerSelect for i64.
==========

[obucinac, 2016-09-23 17:53:22]

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

[Jim Stichnoth, 2016-09-24 14:39:59]

https://codereview.chromium.org/2364143002/diff/1/tests_lit/llvm2ice_tests/se...
File tests_lit/llvm2ice_tests/select-opt.ll (right):

https://codereview.chromium.org/2364143002/diff/1/tests_lit/llvm2ice_tests/se...
tests_lit/llvm2ice_tests/select-opt.ll:113: ; MIPS32: move
I think you should probably remove these two 'move' checks.

"make check-lit" is failing on this test with -Om1.

The reason is that the Om1 register allocator spills the results to the stack
and then reloads from the stack into the return-value registers.  It does this
with sw/lw instructions, and not 'move' instructions.

So my next question is, why aren't all the other 'move' checks in this file
hitting the same problem?  I think it's because the other lowering sequences
compute the result into a temporary infinite-weight variable (register) and then
copy that into the result variable, whereas the i64 select lowering computes
(via movn) into a register pair and then moves directly into dest (via either sw
or move).

For example, the existing i32 select lowering:

  case IceType_i32:
    _movn(SrcFR, SrcTR, ConditionR);
    _mov(DestR, SrcFR);
    _mov(Dest, DestR);
    break;

could probably be changed to:

  case IceType_i32:
    _movn(SrcFR, SrcTR, ConditionR);
    _mov(Dest, SrcFR);
    break;

and it should still be correct but would generate "movn; sw" in Om1 mode rather
than "movn; move".

[obucinac, 2016-09-25 17:19:42]

https://codereview.chromium.org/2364143002/diff/1/tests_lit/llvm2ice_tests/se...
File tests_lit/llvm2ice_tests/select-opt.ll (right):

https://codereview.chromium.org/2364143002/diff/1/tests_lit/llvm2ice_tests/se...
tests_lit/llvm2ice_tests/select-opt.ll:113: ; MIPS32: move
On 2016/09/24 14:39:59, stichnot wrote:
> I think you should probably remove these two 'move' checks.
> 
> "make check-lit" is failing on this test with -Om1.
> 
> The reason is that the Om1 register allocator spills the results to the stack
> and then reloads from the stack into the return-value registers.  It does this
> with sw/lw instructions, and not 'move' instructions.
> 
> So my next question is, why aren't all the other 'move' checks in this file
> hitting the same problem?  I think it's because the other lowering sequences
> compute the result into a temporary infinite-weight variable (register) and
then
> copy that into the result variable, whereas the i64 select lowering computes
> (via movn) into a register pair and then moves directly into dest (via either
sw
> or move).
> 
> For example, the existing i32 select lowering:
> 
>   case IceType_i32:
>     _movn(SrcFR, SrcTR, ConditionR);
>     _mov(DestR, SrcFR);
>     _mov(Dest, DestR);
>     break;
> 
> could probably be changed to:
> 
>   case IceType_i32:
>     _movn(SrcFR, SrcTR, ConditionR);
>     _mov(Dest, SrcFR);
>     break;
> 
> and it should still be correct but would generate "movn; sw" in Om1 mode
rather
> than "movn; move".

Done.

[Jim Stichnoth, 2016-09-25 17:32:09]

lgtm

[Jim Stichnoth, 2016-09-25 17:32:22]

Description was changed from

==========
Subzero, MIPS32: lowerSelect for i64

Implements lowerSelect for i64.
==========

to

==========
Subzero, MIPS32: lowerSelect for i64

Implements lowerSelect for i64.

R=stichnot@chromium.org

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

[Jim Stichnoth, 2016-09-25 17:32:24]

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