MIPS64: Fix [KeyedLoadIC] Support Smi "handlers" for element loads

MIPS64: Fix [KeyedLoadIC] Support Smi "handlers" for element loads

Fix 2cf2eef77bec7d30ccc80cb74950f35bc37404ed

Fix test failures with tests working on external Uint32 arrays. Problem
started to appear because Uint32 value was compared using Int32 compare
operators in ChangeUint32ToTagged. On MIPS64. Uint32 value is not sign-
extended, so upper 32 bits of this value are zero. MIPS64 doesn't have
Word32Compare instructions but uses Word64Compare instructions in
combination with properly sign-extended Int32 values.

BUG=cctest/test-api/Uint32Array,cctest/test-api/SharedUint32Array,
cctest/test-api/FixedUint32Array,mjsunit/compiler/uint32

Committed: https://crrev.com/3b7fbafe72389b990da0f07fbb28c98d425fb2cd
Cr-Commit-Position: refs/heads/master@{#38680}

[ivica.bogosavljevic, 2016-08-10 15:01:51]

ivica.bogosavljevic@imgtec.com changed reviewers:
+ bmeurer@chromium.org, danno@chromium.org, ishell@chromium.org,
jkummerow@chromium.org, verwaest@chromium.org

[ivica.bogosavljevic, 2016-08-10 15:01:52]

Request for comments:

On MIPS64, several tests started failing after `Support Smi "handlers" for
element loads` landed. The reason for this is that in case of Uint32 external
arrays, we load the element using 

Load(MachineType::Uint32()); 

and then try to figure out if it can fit Smi using:

Branch(Int32LessThan(value, Int32Constant(0)) in ChangeUint32ToTagged function

MIPS64 doesn't have Int32 compare operators, it expects that Int32 values are
loaded using Load(Int32) which properly sign-extends to 64 bit wide registers.
After that we could use Int64 compare operators.

So the above trick is not possible on MIPS64. I created a patch to fix this, by
sign-extending Uint32 value using Word32Shl operators. On MIPS64, this properly
fils the upper bits. 

I don't like this fix very much. The other alternative would be to sign-extend
32 bit values before each branch on MIPS64, but this is performance killer
because it adds one more instruction, but almost all values are already sign
extended in case of MIPS64.

[Benedikt Meurer, 2016-08-11 13:01:27]

https://codereview.chromium.org/2231053002/diff/1/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2231053002/diff/1/src/code-stub-assembler.cc#...
src/code-stub-assembler.cc:1831: &if_not_overflow);
This should probably use Uint32LessThan with kMaxSmiValue here.

[ivica.bogosavljevic, 2016-08-12 13:43:18]

On 2016/08/11 13:01:27, Benedikt Meurer wrote:
> https://codereview.chromium.org/2231053002/diff/1/src/code-stub-assembler.cc
> File src/code-stub-assembler.cc (right):
> 
>
https://codereview.chromium.org/2231053002/diff/1/src/code-stub-assembler.cc#...
> src/code-stub-assembler.cc:1831: &if_not_overflow);
> This should probably use Uint32LessThan with kMaxSmiValue here.

There is no Uint32Constant as a turbofan operator. How do we handle this? Do we
use Int32Constant?

[ivica.bogosavljevic, 2016-08-16 09:36:20]

Addressed code review remarks, PTAL

[Jakob Kummerow, 2016-08-17 09:46:35]

lgtm

[ivica.bogosavljevic, 2016-08-17 11:13:35]

The CQ bit was checked by ivica.bogosavljevic@imgtec.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 11:13:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 11:48:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-17 11:48:14]

Dry run: This issue passed the CQ dry run.

[ivica.bogosavljevic, 2016-08-17 12:38:30]

The CQ bit was checked by ivica.bogosavljevic@imgtec.com

[commit-bot: I haz the power, 2016-08-17 12:38:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 12:40:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-17 12:40:55]

Description was changed from

==========
MIPS64: Fix [KeyedLoadIC] Support Smi "handlers" for element loads

Fix 2cf2eef77bec7d30ccc80cb74950f35bc37404ed

Fix test failures with tests working on external Uint32 arrays. Problem
started to appear because Uint32 value was compared using Int32 compare
operators in ChangeUint32ToTagged. On MIPS64. Uint32 value is not sign-
extended, so upper 32 bits of this value are zero. MIPS64 doesn't have
Word32Compare instructions but uses Word64Compare instructions in
combination with properly sign-extended Int32 values.

BUG=cctest/test-api/Uint32Array,cctest/test-api/SharedUint32Array,
cctest/test-api/FixedUint32Array,mjsunit/compiler/uint32
==========

to

==========
MIPS64: Fix [KeyedLoadIC] Support Smi "handlers" for element loads

Fix 2cf2eef77bec7d30ccc80cb74950f35bc37404ed

Fix test failures with tests working on external Uint32 arrays. Problem
started to appear because Uint32 value was compared using Int32 compare
operators in ChangeUint32ToTagged. On MIPS64. Uint32 value is not sign-
extended, so upper 32 bits of this value are zero. MIPS64 doesn't have
Word32Compare instructions but uses Word64Compare instructions in
combination with properly sign-extended Int32 values.

BUG=cctest/test-api/Uint32Array,cctest/test-api/SharedUint32Array,
cctest/test-api/FixedUint32Array,mjsunit/compiler/uint32

Committed: https://crrev.com/3b7fbafe72389b990da0f07fbb28c98d425fb2cd
Cr-Commit-Position: refs/heads/master@{#38680}
==========

[commit-bot: I haz the power, 2016-08-17 12:40:56]

Patchset 2 (id:??) landed as
https://crrev.com/3b7fbafe72389b990da0f07fbb28c98d425fb2cd
Cr-Commit-Position: refs/heads/master@{#38680}