MIPS: [turbofan] Improve test and equality compare with zero and constants.

MIPS: [turbofan] Improve test and equality compare with zero and constants.

TEST=
BUG=

Committed: https://crrev.com/0c5c479574a3dc0645ba0a2f2e8f45249f674f8e
Cr-Commit-Position: refs/heads/master@{#31222}

[dusan.milosavljevic, 2015-10-09 16:14:55]

dusan.m.milosavljevic@gmail.com changed reviewers:
+ bmeurer@chromium.org, danno@chromium.org, paul.lind@imgtec.com,
titzer@chromium.org

[dusan.milosavljevic, 2015-10-09 16:14:56]

PTAL.

[titzer, 2015-10-09 19:36:47]

lgtm

[paul.l..., 2015-10-12 03:53:45]

Nice improvement, but I do have a couple questions.

https://codereview.chromium.org/1400833002/diff/20001/src/compiler/mips/instr...
File src/compiler/mips/instruction-selector-mips.cc (right):

https://codereview.chromium.org/1400833002/diff/20001/src/compiler/mips/instr...
src/compiler/mips/instruction-selector-mips.cc:47: case kCheckedLoadFloat64:
Why do you remove the kCheckedLoadFloat32/kCheckedStoreFloat32cases here? They
have the same requirement for 16-bit immediate.

Also, none of the kCheckedL/SFloatNN cases are in mips64 CanBeImmediate(), and I
think they should be. (the assembler does take care of this now, but that should
not be done there, and will be removed per issue 182)

https://codereview.chromium.org/1400833002/diff/20001/src/compiler/mips/instr...
src/compiler/mips/instruction-selector-mips.cc:50:
std::numeric_limits<int16_t>::max() >= (value + kIntSize);
Can you explain why this is better than is_int16(value + kIntSize)? The old way
conveys the purpose more clearly. If the new condition does fix some edge-case,
you should do it in mips64 version also, no?

[dusan.milosavljevic, 2015-10-12 14:41:52]

https://codereview.chromium.org/1400833002/diff/20001/src/compiler/mips/instr...
File src/compiler/mips/instruction-selector-mips.cc (right):

https://codereview.chromium.org/1400833002/diff/20001/src/compiler/mips/instr...
src/compiler/mips/instruction-selector-mips.cc:47: case kCheckedLoadFloat64:
On 2015/10/12 03:53:45, paul.l... wrote:
> Why do you remove the kCheckedLoadFloat32/kCheckedStoreFloat32cases here? They
> have the same requirement for 16-bit immediate.
> 
> Also, none of the kCheckedL/SFloatNN cases are in mips64 CanBeImmediate(), and
I
> think they should be. (the assembler does take care of this now, but that
should
> not be done there, and will be removed per issue 182)

Yes, the requirement for kCheckedLoadFloat32/kCheckedStoreFloat32 cases still
holds, it is under default case (is_int16()).  No need for loading and storing
floats to be in same case for storing and loading doubles.

https://codereview.chromium.org/1400833002/diff/20001/src/compiler/mips/instr...
src/compiler/mips/instruction-selector-mips.cc:50:
std::numeric_limits<int16_t>::max() >= (value + kIntSize);
On 2015/10/12 03:53:45, paul.l... wrote:
> Can you explain why this is better than is_int16(value + kIntSize)? The old
way
> conveys the purpose more clearly. If the new condition does fix some
edge-case,
> you should do it in mips64 version also, no?

Old check does not handle  minimum offset edge case (min_int16 - 4), where
negative offset 16-bit range can be exceeded.  Mips64 code does not require
this, because for loading and storing doubles ldc1 and sdc1 instructions are
used instead of lwc1/swc1 pairs.

[paul.l..., 2015-10-12 15:53:58]

That all makes sense, thanks! LGTM.

[dusan.milosavljevic, 2015-10-12 23:54:04]

The CQ bit was checked by dusan.m.milosavljevic@gmail.com

[commit-bot: I haz the power, 2015-10-12 23:54:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1400833002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1400833002/20001

[commit-bot: I haz the power, 2015-10-13 00:15:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-13 00:15:54]

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