MIPS[64]: Optimize kMips[64]Tst in code-generator-mips[64].cc

MIPS[64]: Optimize kMips[64]Tst in code-generator-mips[64].cc

A small change that brings a lot of benefit since it is used in a lot
of places.

BUG=

Committed: https://crrev.com/e18b03e651a9ab6dea61a43d6eb337de418dae95
Cr-Commit-Position: refs/heads/master@{#40982}

[ivica.bogosavljevic, 2016-11-04 15:05:14]

ivica.bogosavljevic@imgtec.com changed reviewers:
+ dusan.simicic@imgtec.com, miran.karic@imgtec.com

[ivica.bogosavljevic, 2016-11-04 15:05:15]

PTAL

[dusan.simicic, 2016-11-09 12:40:49]

https://codereview.chromium.org/2477453005/diff/1/src/compiler/mips/code-gene...
File src/compiler/mips/code-generator-mips.cc (right):

https://codereview.chromium.org/2477453005/diff/1/src/compiler/mips/code-gene...
src/compiler/mips/code-generator-mips.cc:1719: if
(instr->InputAt(1)->IsImmediate() &&
I think this can be generalised for all 2^n constants, where n=0..31, not only
for n=0 (imm value 1). Not sure how often we are checking for all of this
constant values (2,4,8,16,...), but maybe this could bring some benefit.

https://codereview.chromium.org/2477453005/diff/1/src/compiler/mips64/code-ge...
File src/compiler/mips64/code-generator-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/1/src/compiler/mips64/code-ge...
src/compiler/mips64/code-generator-mips64.cc:2029: if
(instr->InputAt(1)->IsImmediate() &&
Take a look at comment for mips32, the same could be applied here.

[dusan.simicic, 2016-11-10 10:03:14]

https://codereview.chromium.org/2477453005/diff/20001/src/mips64/macro-assemb...
File src/mips64/macro-assembler-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/20001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1799: DCHECK(size < 0 && size <= 32);
Typo, size > 0.

[ivica.bogosavljevic, 2016-11-10 10:58:50]

PTAL

[miran.karic, 2016-11-10 12:30:12]

miran.karic@imgtec.com changed reviewers:
+ Miran.Karic@imgtec.com

[miran.karic, 2016-11-10 12:30:13]

https://codereview.chromium.org/2477453005/diff/20001/src/compiler/mips/code-...
File src/compiler/mips/code-generator-mips.cc (right):

https://codereview.chromium.org/2477453005/diff/20001/src/compiler/mips/code-...
src/compiler/mips/code-generator-mips.cc:1730: __ xori(result, result, 1);
Do we need xori when using Ext? If not this if should be in the above else
block.

https://codereview.chromium.org/2477453005/diff/20001/src/mips64/macro-assemb...
File src/mips64/macro-assembler-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/20001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1772: } else if (pos < 64) {
Can be just else Dextu(...

[miran.karic, 2016-11-10 13:12:59]

https://codereview.chromium.org/2477453005/diff/40001/src/compiler/mips64/cod...
File src/compiler/mips64/code-generator-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/40001/src/compiler/mips64/cod...
src/compiler/mips64/code-generator-mips64.cc:2040: __ xori(result, result, 1);
Same here as for mips32.

[ivica.bogosavljevic, 2016-11-10 13:31:41]

ivica.bogosavljevic@imgtec.com changed reviewers:
- Miran.Karic@imgtec.com

[ivica.bogosavljevic, 2016-11-10 13:31:41]

Addressed remarks, PTAL

https://codereview.chromium.org/2477453005/diff/20001/src/compiler/mips/code-...
File src/compiler/mips/code-generator-mips.cc (right):

https://codereview.chromium.org/2477453005/diff/20001/src/compiler/mips/code-...
src/compiler/mips/code-generator-mips.cc:1730: __ xori(result, result, 1);
On 2016/11/10 12:30:12, miran.karic wrote:
> Do we need xori when using Ext? If not this if should be in the above else
> block.

We need xori in both cases

https://codereview.chromium.org/2477453005/diff/20001/src/mips64/macro-assemb...
File src/mips64/macro-assembler-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/20001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1772: } else if (pos < 64) {
On 2016/11/10 12:30:12, miran.karic wrote:
> Can be just else Dextu(...

Done.

[miran.karic, 2016-11-10 16:18:14]

miran.karic@imgtec.com changed reviewers:
+ Miran.Karic@imgtec.com

[miran.karic, 2016-11-10 16:18:14]

lgtm

[miran.karic, 2016-11-10 16:31:57]

https://codereview.chromium.org/2477453005/diff/60001/src/mips64/macro-assemb...
File src/mips64/macro-assembler-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/60001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1772: } else if (pos < 64) {
This if after else and DCHECK line below are redundant as you have everything
covered with DCHECKs above.

[ivica.bogosavljevic, 2016-11-15 09:14:06]

ivica.bogosavljevic@imgtec.com changed reviewers:
- Miran.Karic@imgtec.com

[ivica.bogosavljevic, 2016-11-15 09:14:06]

https://codereview.chromium.org/2477453005/diff/60001/src/mips64/macro-assemb...
File src/mips64/macro-assembler-mips64.cc (right):

https://codereview.chromium.org/2477453005/diff/60001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1772: } else if (pos < 64) {
On 2016/11/10 16:31:56, miran.karic wrote:
> This if after else and DCHECK line below are redundant as you have everything
> covered with DCHECKs above.

This is true, but I would like to leave this as it is for two reasons:
1) if anyone changes something above, this check will fail
2) implicitly for the programmer, so he knows that we expect here size to be
less-equal than 32 (this could have been done in the comment though)

[ivica.bogosavljevic, 2016-11-15 09:14:14]

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

[commit-bot: I haz the power, 2016-11-15 09:14:20]

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-11-15 09:37:57]

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

[commit-bot: I haz the power, 2016-11-15 09:37:58]

Dry run: This issue passed the CQ dry run.

[ivica.bogosavljevic, 2016-11-15 09:38:48]

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

[commit-bot: I haz the power, 2016-11-15 09:38:51]

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-11-15 09:41:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-17 22:33:32]

Description was changed from

==========
MIPS[64]: Optimize kMips[64]Tst in code-generator-mips[64].cc

A small change that brings a lot of benefit since it is used in a lot
of places.

BUG=
==========

to

==========
MIPS[64]: Optimize kMips[64]Tst in code-generator-mips[64].cc

A small change that brings a lot of benefit since it is used in a lot
of places.

BUG=

Committed: https://crrev.com/e18b03e651a9ab6dea61a43d6eb337de418dae95
Cr-Commit-Position: refs/heads/master@{#40982}
==========

[commit-bot: I haz the power, 2016-11-17 22:33:33]

Patchset 4 (id:??) landed as
https://crrev.com/e18b03e651a9ab6dea61a43d6eb337de418dae95
Cr-Commit-Position: refs/heads/master@{#40982}