MIPS: Move Maps' back pointers from "transitions" to "constructor" field

MIPS: Move Maps' back pointers from "transitions" to "constructor" field

Port affcfaf42801414d25b3e57271a9803a026f40ed

BUG=

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

[akos.palfi.imgtec, 2015-02-25 01:59:57]

akos.palfi@imgtec.com changed reviewers:
+ balazs.kilvady@imgtec.com, danno@chromium.org, dusan.milosavljevic@imgtec.com,
gergely.kis@imgtec.com, jkummerow@chromium.org, paul.lind@imgtec.com

[akos.palfi.imgtec, 2015-02-25 01:59:57]

PTAL.

[paul.l..., 2015-02-25 02:17:56]

LGTM. But a couple nits, and a suggestion for a follow-on small optimization.

https://codereview.chromium.org/954863002/diff/40001/src/mips/lithium-codegen...
File src/mips/lithium-codegen-mips.cc (right):

https://codereview.chromium.org/954863002/diff/40001/src/mips/lithium-codegen...
src/mips/lithium-codegen-mips.cc:2660: Register instance_type = scratch1();
Suggest adding DCHECK(!instance_type.is(temp));
DCHECK(!instance_type.is(temp2)); here

https://codereview.chromium.org/954863002/diff/40001/src/mips/macro-assembler...
File src/mips/macro-assembler-mips.cc (right):

https://codereview.chromium.org/954863002/diff/40001/src/mips/macro-assembler...
src/mips/macro-assembler-mips.cc:4332: lw(result, FieldMemOperand(result,
Map::kConstructorOrBackPointerOffset));
If we know (via DCHECK) that Map::kConstructorOrBackPointerOffset is < int16_t,
then the lw() will be single instruction and could be moved into branch delay
slot of the Branch(&loop).

Since it is late, maybe you could do that as a second (well-tested) CL tomorrow,
and not take a chance on breaking the build in this CL. Same for mips64.

https://codereview.chromium.org/954863002/diff/40001/src/mips64/lithium-codeg...
File src/mips64/lithium-codegen-mips64.cc (right):

https://codereview.chromium.org/954863002/diff/40001/src/mips64/lithium-codeg...
src/mips64/lithium-codegen-mips64.cc:2621: Register instance_type = scratch1();
Suggest adding DCHECK(!instance_type.is(temp));
DCHECK(!instance_type.is(temp2)); here

[akos.palfi.imgtec, 2015-02-25 02:25:38]

Fixed nits.

https://codereview.chromium.org/954863002/diff/40001/src/mips/lithium-codegen...
File src/mips/lithium-codegen-mips.cc (right):

https://codereview.chromium.org/954863002/diff/40001/src/mips/lithium-codegen...
src/mips/lithium-codegen-mips.cc:2660: Register instance_type = scratch1();
On 2015/02/25 02:17:56, paul.l... wrote:
> Suggest adding DCHECK(!instance_type.is(temp));
> DCHECK(!instance_type.is(temp2)); here

Done.

https://codereview.chromium.org/954863002/diff/40001/src/mips/macro-assembler...
File src/mips/macro-assembler-mips.cc (right):

https://codereview.chromium.org/954863002/diff/40001/src/mips/macro-assembler...
src/mips/macro-assembler-mips.cc:4332: lw(result, FieldMemOperand(result,
Map::kConstructorOrBackPointerOffset));
On 2015/02/25 02:17:56, paul.l... wrote:
> If we know (via DCHECK) that Map::kConstructorOrBackPointerOffset is <
int16_t,
> then the lw() will be single instruction and could be moved into branch delay
> slot of the Branch(&loop).
> 
> Since it is late, maybe you could do that as a second (well-tested) CL
tomorrow,
> and not take a chance on breaking the build in this CL. Same for mips64.

Acknowledged.

https://codereview.chromium.org/954863002/diff/40001/src/mips64/lithium-codeg...
File src/mips64/lithium-codegen-mips64.cc (right):

https://codereview.chromium.org/954863002/diff/40001/src/mips64/lithium-codeg...
src/mips64/lithium-codegen-mips64.cc:2621: Register instance_type = scratch1();
On 2015/02/25 02:17:56, paul.l... wrote:
> Suggest adding DCHECK(!instance_type.is(temp));
> DCHECK(!instance_type.is(temp2)); here

Done.

[akos.palfi.imgtec, 2015-02-25 02:37:36]

The CQ bit was checked by akos.palfi@imgtec.com

[akos.palfi.imgtec, 2015-02-25 02:37:37]

The patchset sent to the CQ was uploaded after l-g-t-m from paul.lind@imgtec.com
Link to the patchset: https://codereview.chromium.org/954863002/#ps80001 (title:
"Fixed mips64.")

[commit-bot: I haz the power, 2015-02-25 02:38:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954863002/80001

[commit-bot: I haz the power, 2015-02-25 03:25:00]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-02-25 03:25:16]

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

[paul.l..., 2015-02-25 05:00:10]

We need to find different scratch register usage, as we get DCHECK fails. This
fixes the build, so I don't want to revert it. But please fix this in the
morning.

https://codereview.chromium.org/954863002/diff/60001/src/mips/macro-assembler...
File src/mips/macro-assembler-mips.cc (right):

https://codereview.chromium.org/954863002/diff/60001/src/mips/macro-assembler...
src/mips/macro-assembler-mips.cc:4390: GetMapConstructor(result, result,
scratch, at);
Use of 'at' here causes a assert fail in the Branch above at line 4331. temp2
cannot be 'at'.

[Jakob Kummerow, 2015-02-25 09:39:46]

https://codereview.chromium.org/954863002/diff/60001/src/mips/macro-assembler...
File src/mips/macro-assembler-mips.cc (right):

https://codereview.chromium.org/954863002/diff/60001/src/mips/macro-assembler...
src/mips/macro-assembler-mips.cc:4390: GetMapConstructor(result, result,
scratch, at);
On 2015/02/25 05:00:09, paul.l... wrote:
> Use of 'at' here causes a assert fail in the Branch above at line 4331. temp2
> cannot be 'at'.

You can probably just use "(result, result, scratch, scratch)".

https://codereview.chromium.org/954863002/diff/80001/src/mips/lithium-codegen...
File src/mips/lithium-codegen-mips.cc (right):

https://codereview.chromium.org/954863002/diff/80001/src/mips/lithium-codegen...
src/mips/lithium-codegen-mips.cc:2662: DCHECK(!instance_type.is(temp2));
DBC: having |instance_type| and |temp2| overlap shouldn't matter, since the code
below doesn't care about the contents of |temp2|.