MIPS64: Add/fix bit insertion/extraction instrs.

MIPS64: Add/fix bit insertion/extraction instrs.

Added support for DINSM and DINSU bit insertion instructions. Also fixed
errors with bit extraction instructions, added disassembler tests and
adjusted the code to make it more compact.

BUG=
TEST=cctest/test-assembler-mips/Dins
     cctest/test-disasm-mips/Type0

Review-Url: https://codereview.chromium.org/2871663002
Cr-Commit-Position: refs/heads/master@{#45226}
Committed: https://chromium.googlesource.com/v8/v8/+/838d0b4bd2106dd0da43b9a97c96d789ceb60dfe

[miran.karic, 2017-05-08 16:36:39]

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

[miran.karic, 2017-05-08 16:36:40]

PTAL

[dusan.simicic, 2017-05-09 08:42:37]

On 2017/05/08 16:36:40, miran.karic wrote:
> PTAL

All changes are for mips64, right? Change title to: "MIPS64: Add/fix bit
insertion/extraction instrs."

[miran.karic, 2017-05-09 08:42:58]

Description was changed from

==========
MIPS: Add/fix bit insertion/extraction instrs.

Added support for DINSM and DINSU bit insertion instructions. Also fixed
errors with bit extraction instructions, added disassembler tests and
adjusted the code to make it more compact.

BUG=
TEST=cctest/test-assembler-mips/Dins
     cctest/test-disasm-mips/Type0
==========

to

==========
MIPS64: Add/fix bit insertion/extraction instrs.

Added support for DINSM and DINSU bit insertion instructions. Also fixed
errors with bit extraction instructions, added disassembler tests and
adjusted the code to make it more compact.

BUG=
TEST=cctest/test-assembler-mips/Dins
     cctest/test-disasm-mips/Type0
==========

[miran.karic, 2017-05-09 08:44:00]

On 2017/05/09 08:42:37, dusan.simicic wrote:
> On 2017/05/08 16:36:40, miran.karic wrote:
> > PTAL
> 
> All changes are for mips64, right? Change title to: "MIPS64: Add/fix bit
> insertion/extraction instrs."

Yes, just changed it.

[Ilija.Pavlovic1, 2017-05-09 09:17:45]

ilija.pavlovic@imgtec.com changed reviewers:
+ Ilija.Pavlovic@imgtec.com

[Ilija.Pavlovic1, 2017-05-09 09:17:46]

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips64.cc (right):

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:6090: uint64_t imm;
Instead "imm" maybe more suitable name is "res".

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:6101: {0x0, 0x1FABCDEFF, 31, 1,
0x80000000},
Instead 0x0, use another value (for example 0xAAAAAAAA) which will show that
operation is performed on expected way.

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:6131: TEST(Dins_all) {
Maybe not needed to go through all combinations.

[miran.karic, 2017-05-09 13:52:42]

miran.karic@imgtec.com changed reviewers:
- Ilija.Pavlovic@imgtec.com

[miran.karic, 2017-05-09 13:52:43]

Fixed Dins test cases and removed Dins_all test. PTAL.

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips64.cc (right):

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:6090: uint64_t imm;
On 2017/05/09 09:17:46, Ilija.Pavlovic1 wrote:
> Instead "imm" maybe more suitable name is "res".

This is initial value of target register, not result, I think it can stay as it
is.

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:6101: {0x0, 0x1FABCDEFF, 31, 1,
0x80000000},
On 2017/05/09 09:17:46, Ilija.Pavlovic1 wrote:
> Instead 0x0, use another value (for example 0xAAAAAAAA) which will show that
> operation is performed on expected way.

Yes I think that will be better, I adjusted test cases to cover this.

https://codereview.chromium.org/2871663002/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:6131: TEST(Dins_all) {
On 2017/05/09 09:17:46, Ilija.Pavlovic1 wrote:
> Maybe not needed to go through all combinations.

I agree, I removed this test and only left Dins test that covers a number of
cases.

[Ilija.Pavlovic1, 2017-05-10 06:10:14]

ilija.pavlovic@imgtec.com changed reviewers:
+ Ilija.Pavlovic@imgtec.com

[Ilija.Pavlovic1, 2017-05-10 06:10:15]

lgtm

[ivica.bogosavljevic, 2017-05-10 07:43:27]

ivica.bogosavljevic@imgtec.com changed reviewers:
- Ilija.Pavlovic@imgtec.com

[ivica.bogosavljevic, 2017-05-10 07:43:28]

lgtm with nits

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

https://codereview.chromium.org/2871663002/diff/20001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1958: if (size > 32)
You should put braces here {}, since they are everywhere else

[miran.karic, 2017-05-10 08:30:48]

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

https://codereview.chromium.org/2871663002/diff/20001/src/mips64/macro-assemb...
src/mips64/macro-assembler-mips64.cc:1958: if (size > 32)
On 2017/05/10 07:43:27, ivica.bogosavljevic wrote:
> You should put braces here {}, since they are everywhere else

Done. We actually use the format without braces (e.g.
src/mips64/simulator-mips64.cc:4297) but it is rare.

[miran.karic, 2017-05-10 08:31:25]

The CQ bit was checked by miran.karic@imgtec.com

[miran.karic, 2017-05-10 08:31:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
ivica.bogosavljevic@imgtec.com, Ilija.Pavlovic@imgtec.com
Link to the patchset: https://codereview.chromium.org/2871663002/#ps40001
(title: "Add braces")

[commit-bot: I haz the power, 2017-05-10 08:31:32]

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, 2017-05-10 08:38:07]

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

[commit-bot: I haz the power, 2017-05-10 08:38:08]

Try jobs failed on following builders:
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[miran.karic, 2017-05-10 09:36:27]

On 2017/05/10 08:38:08, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

Try job failed because helper variable pps was only used in debug mode in Dext
macro assembler function. Removed the variable use altogether.

[miran.karic, 2017-05-10 09:37:46]

The CQ bit was checked by miran.karic@imgtec.com

[miran.karic, 2017-05-10 09:37:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
ivica.bogosavljevic@imgtec.com, Ilija.Pavlovic@imgtec.com
Link to the patchset: https://codereview.chromium.org/2871663002/#ps60001
(title: "Remove pps variable use")

[commit-bot: I haz the power, 2017-05-10 09:37:54]

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, 2017-05-10 10:06:54]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494409066371000,
"parent_rev": "755a809ee5f91f5f63d5f056af37090960be002c", "commit_rev":
"838d0b4bd2106dd0da43b9a97c96d789ceb60dfe"}

[commit-bot: I haz the power, 2017-05-10 10:07:00]

Description was changed from

==========
MIPS64: Add/fix bit insertion/extraction instrs.

Added support for DINSM and DINSU bit insertion instructions. Also fixed
errors with bit extraction instructions, added disassembler tests and
adjusted the code to make it more compact.

BUG=
TEST=cctest/test-assembler-mips/Dins
     cctest/test-disasm-mips/Type0
==========

to

==========
MIPS64: Add/fix bit insertion/extraction instrs.

Added support for DINSM and DINSU bit insertion instructions. Also fixed
errors with bit extraction instructions, added disassembler tests and
adjusted the code to make it more compact.

BUG=
TEST=cctest/test-assembler-mips/Dins
     cctest/test-disasm-mips/Type0

Review-Url: https://codereview.chromium.org/2871663002
Cr-Commit-Position: refs/heads/master@{#45226}
Committed:
https://chromium.googlesource.com/v8/v8/+/838d0b4bd2106dd0da43b9a97c96d789ceb...
==========

[commit-bot: I haz the power, 2017-05-10 10:07:02]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/838d0b4bd2106dd0da43b9a97c96d789ceb...