MIPS: [turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

MIPS:[turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt.

BUG=

Committed: https://crrev.com/084db022ef7c3bfcbdab1be6a6fb434cfa511084
Cr-Commit-Position: refs/heads/master@{#33555}

[Marija Antic, 2016-01-15 08:35:36]

Description was changed from

==========
[MIPS][turbofan] Implement of Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

to

==========
[MIPS][turbofan] Implement of Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

[Marija Antic, 2016-01-15 08:35:36]

marija.antic@imgtec.com changed reviewers:
+ akos.palfi@imgtec.com, ilija.pavlovic@imgtec.com,
ivica.bogosavljevic@imgtec.com, miran.karic@imgtec.com, paul.lind@imgtec.com

[paul.l..., 2016-01-18 05:19:57]

nit: by convention, for arch-specific changes like this one, the message should
start with "MIPS: ", and not use brackets. The brackets are used for denoting v8
subsystems, like [turbofan] here, not CPU architectures.

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

https://codereview.chromium.org/1588383002/diff/1/src/compiler/mips/code-gene...
src/compiler/mips/code-generator-mips.cc:695: Label end;
It's not obvious to me what the algorithm is here, might be nice to put that in
a comment, if you can get it in a line or two.

https://codereview.chromium.org/1588383002/diff/1/src/compiler/mips/code-gene...
src/compiler/mips/code-generator-mips.cc:720: __ And(reg1, reg1, m1);
In these two And() ops, you are anding a reg with constant m1, which is larger
than int16_t, so the And() macro will load it to 'at' register, and then do a
3-reg and(). And the li() is a pair of instructions, lui/ori.

But since both And() macros here use m1, you will load value 0x55555555 to 'at'
twice. You can save 2 instructions by making the load explicit:
    li(at, m1);
    And(reg2, i.InputRegister(0), at);
    And(reg1, reg1, at);  // Note that these And()'s could now be plain-asm
'and_()' instr.

This same pattern repeats for m2, m4, m8, m16 so you'd save a lot of
instructions.

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

https://codereview.chromium.org/1588383002/diff/1/src/compiler/mips64/code-ge...
src/compiler/mips64/code-generator-mips64.cc:785: __ And(reg1, reg1, m1);
Same comment as in mips32 version....

[balazs.kilvady, 2016-01-22 16:14:40]

balazs.kilvady@imgtec.com changed reviewers:
+ balazs.kilvady@imgtec.com

[balazs.kilvady, 2016-01-22 16:14:40]

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

https://codereview.chromium.org/1588383002/diff/1/src/compiler/mips/code-gene...
src/compiler/mips/code-generator-mips.cc:697: __ subu(reg2, zero_reg,
i.InputRegister(0));
Please use Branch(&jmp, eq, ...) instead of beq() and if you want to put subu()
(or even better: Subu()) into the delay slot then Branch(USE_DELAY_SLOT, &jmp,
eq, ...). Also I think jmp is not a lucky name for a label as jmp is a (pseudo)
instruction. Please try to find a better neme.

https://codereview.chromium.org/1588383002/diff/1/src/compiler/mips/code-gene...
src/compiler/mips/code-generator-mips.cc:704: __ nop();
To emit an uncoditional branch please use Branch(&end); and you don't have to
add the nop() into the delay slot (Branch() does it for you if USE_DELAY_SLOT
parameter is not used).

[Ilija.Pavlovic1, 2016-01-26 09:36:57]

On 2016/01/25 15:39:22, marija.antic wrote:

This seems to me OK.

[balazs.kilvady, 2016-01-27 10:42:12]

On 2016/01/26 09:36:57, Ilija.Pavlovic1 wrote:
> On 2016/01/25 15:39:22, marija.antic wrote:
> 
> This seems to me OK.

Also please note Paul's nit: by convention, for arch-specific changes like this
one, the message should
start with "MIPS: ", and not use brackets. The brackets are used for denoting v8
subsystems, like [turbofan] here, not CPU architectures.

[Marija Antic, 2016-01-27 10:55:00]

Description was changed from

==========
[MIPS][turbofan] Implement of Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

to

==========
MIPS:[turbofan] Implement of Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

[Marija Antic, 2016-01-27 10:59:26]

On 2016/01/27 10:58:39, marija.antic wrote:

Updated the issue description

[Marija Antic, 2016-01-27 11:00:38]

Description was changed from

==========
MIPS:[turbofan] Implement of Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

to

==========
MIPS:[turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

[akos.palfi.imgtec, 2016-01-27 11:37:37]

Looks fine to me, just a few nits, please see my inline comments.

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

https://codereview.chromium.org/1588383002/diff/40001/src/compiler/mips/code-...
src/compiler/mips/code-generator-mips.cc:699: __ subu(reg2, zero_reg,
i.InputRegister(0));
The 'subu' is totally correct here, but since you use 'Subu' below, I would use
'Subu' here as well, just for consistency.

https://codereview.chromium.org/1588383002/diff/40001/src/compiler/mips/code-...
src/compiler/mips/code-generator-mips.cc:705: __ mov(i.OutputRegister(), reg2);
The move here is not necessary, use the Subu to store the result in the
OutputRegister.

__ Subu(i.OutputRegister(), reg1, reg2);

Same on MIPS64 (Ctz, Dctz).

https://codereview.chromium.org/1588383002/diff/40001/src/compiler/mips/code-...
src/compiler/mips/code-generator-mips.cc:756: __ mov(i.OutputRegister(), reg1);
Same here, combine the mov into the addu above.

Same on MIPS64.

[akos.palfi.imgtec, 2016-01-27 14:16:03]

LGTM

[Marija Antic, 2016-01-27 14:23:11]

The CQ bit was checked by marija.antic@imgtec.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 14:23:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1588383002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1588383002/60001

[commit-bot: I haz the power, 2016-01-27 14:45:52]

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

[commit-bot: I haz the power, 2016-01-27 14:45:52]

Dry run: This issue passed the CQ dry run.

[Marija Antic, 2016-01-27 14:46:12]

The CQ bit was checked by marija.antic@imgtec.com

[commit-bot: I haz the power, 2016-01-27 14:46:18]

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

[commit-bot: I haz the power, 2016-01-27 14:48:53]

Description was changed from

==========
MIPS:[turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

to

==========
MIPS:[turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

[commit-bot: I haz the power, 2016-01-27 14:48:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-27 14:49:48]

Description was changed from

==========
MIPS:[turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

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

to

==========
MIPS:[turbofan] Implement Word32Ctz, Word64Ctz, Word32Popcnt and Word64Popcnt

Implement the optional turbofan operators Word32Ctz, Word64Ctz, Word32Popcnt and
Word64Popcnt.

BUG=

Committed: https://crrev.com/084db022ef7c3bfcbdab1be6a6fb434cfa511084
Cr-Commit-Position: refs/heads/master@{#33555}
==========

[commit-bot: I haz the power, 2016-01-27 14:49:49]

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