[ic] Fix megamorphic stub cache probing on some platforms.

[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers' flags
was accidentally offsetting the underflow bug in stub cache probing code on arm, arm64,
mips and mips64.

Stub cache tests now work even when snapshot is enabled.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
Committed: https://crrev.com/7da34f8acb99bc513f366aaa8be23742eb30807b
Cr-Commit-Position: refs/heads/master@{#37910}

[Igor Sheludko, 2016-07-19 17:53:32]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 17:53:40]

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-07-19 18:19:39]

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

[commit-bot: I haz the power, 2016-07-19 18:19:40]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/9309)
  v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[Igor Sheludko, 2016-07-19 18:45:07]

Patchset #1 (id:1) has been deleted

[Igor Sheludko, 2016-07-19 18:55:56]

Description was changed from

==========
Updated tests

1 step

All fixes are done

fixes

fixes

fixes: support kSecondaryMagic

foxes

test updated

Improve the stub-cache tests

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

to

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

BUG=chromium:618701
==========

[Igor Sheludko, 2016-07-19 19:04:37]

Description was changed from

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

BUG=chromium:618701
==========

to

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
==========

[Igor Sheludko, 2016-07-19 19:05:29]

Patchset #1 (id:20001) has been deleted

[Igor Sheludko, 2016-07-19 19:12:52]

Description was changed from

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
==========

to

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Stub cache tests now work even when snapshot is enabled.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
==========

[Igor Sheludko, 2016-07-19 19:13:05]

Patchset #1 (id:40001) has been deleted

[Igor Sheludko, 2016-07-19 19:13:10]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 19:13:14]

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-07-19 19:42:26]

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

[commit-bot: I haz the power, 2016-07-19 19:42:27]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-07-20 08:30:26]

ishell@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Igor Sheludko, 2016-07-20 08:30:26]

PTAL

[Jakob Kummerow, 2016-07-20 12:09:46]

LGTM with comments.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm/stub-cache-a...
File src/ic/arm/stub-cache-arm.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm/stub-cache-a...
src/ic/arm/stub-cache-arm.cc:20: // Number of the cache entry, not scaled.
outdated comment: |offset| is scaled now.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
File src/ic/arm64/stub-cache-arm64.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
src/ic/arm64/stub-cache-arm64.cc:50: __ Mov(scratch, key_offset);
Why is this instruction even needed? Wouldn't it suffice to do "Add(scratch,
key_offset, ...)" below?

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
src/ic/arm64/stub-cache-arm64.cc:138: __ And(scratch, scratch, Operand(extra,
LSL, kCacheIndexShift));
IIUC, "And(scratch, scratch, Operand((kPrimaryTableSize - 1) <<
kCacheIndexShift))" should do the right thing under the hood. Again below.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips/stub-cache-...
File src/ic/mips/stub-cache-mips.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips/stub-cache-...
src/ic/mips/stub-cache-mips.cc:139: __ sll(at, at, kCacheIndexShift);
Why not fold this into the instruction above?

__ li(at, Operand((kPrimaryTableSize - 1) << kCacheIndexShift));

Again below.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips64/stub-cach...
File src/ic/mips64/stub-cache-mips64.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips64/stub-cach...
src/ic/mips64/stub-cache-mips64.cc:143: __ sll(at, at, kCacheIndexShift);
Same comment as for mips32 here.

[Igor Sheludko, 2016-07-20 13:40:26]

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm/stub-cache-a...
File src/ic/arm/stub-cache-arm.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm/stub-cache-a...
src/ic/arm/stub-cache-arm.cc:20: // Number of the cache entry, not scaled.
On 2016/07/20 12:09:45, Jakob wrote:
> outdated comment: |offset| is scaled now.

Done.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
File src/ic/arm64/stub-cache-arm64.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
src/ic/arm64/stub-cache-arm64.cc:50: __ Mov(scratch, key_offset);
On 2016/07/20 12:09:45, Jakob wrote:
> Why is this instruction even needed? Wouldn't it suffice to do "Add(scratch,
> key_offset, ...)" below?

Unfortunately we don't have such an instruction on arm64.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
src/ic/arm64/stub-cache-arm64.cc:138: __ And(scratch, scratch, Operand(extra,
LSL, kCacheIndexShift));
On 2016/07/20 12:09:45, Jakob wrote:
> IIUC, "And(scratch, scratch, Operand((kPrimaryTableSize - 1) <<
> kCacheIndexShift))" should do the right thing under the hood. Again below.

Indeed. Done here and everywhere.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips/stub-cache-...
File src/ic/mips/stub-cache-mips.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips/stub-cache-...
src/ic/mips/stub-cache-mips.cc:139: __ sll(at, at, kCacheIndexShift);
On 2016/07/20 12:09:45, Jakob wrote:
> Why not fold this into the instruction above?
> 
> __ li(at, Operand((kPrimaryTableSize - 1) << kCacheIndexShift));
> 
> Again below.

Done.

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips64/stub-cach...
File src/ic/mips64/stub-cache-mips64.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/mips64/stub-cach...
src/ic/mips64/stub-cache-mips64.cc:143: __ sll(at, at, kCacheIndexShift);
On 2016/07/20 12:09:45, Jakob wrote:
> Same comment as for mips32 here.

Done.

[Igor Sheludko, 2016-07-20 13:40:33]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-20 13:40:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jakob Kummerow, 2016-07-20 13:50:01]

lgtm

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
File src/ic/arm64/stub-cache-arm64.cc (right):

https://codereview.chromium.org/2161153002/diff/60001/src/ic/arm64/stub-cache...
src/ic/arm64/stub-cache-arm64.cc:50: __ Mov(scratch, key_offset);
On 2016/07/20 13:40:26, Igor Sheludko wrote:
> On 2016/07/20 12:09:45, Jakob wrote:
> > Why is this instruction even needed? Wouldn't it suffice to do "Add(scratch,
> > key_offset, ...)" below?
> 
> Unfortunately we don't have such an instruction on arm64. 

Argh, my mistake. Somehow I thought |key_offset| was a register. Never mind.

[commit-bot: I haz the power, 2016-07-20 14:05:00]

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

[commit-bot: I haz the power, 2016-07-20 14:05:01]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-07-20 14:16:39]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2016-07-20 14:16: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-07-20 14:18:52]

Description was changed from

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Stub cache tests now work even when snapshot is enabled.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
==========

to

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Stub cache tests now work even when snapshot is enabled.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
==========

[commit-bot: I haz the power, 2016-07-20 14:18:54]

Committed patchset #2 (id:80001)

[commit-bot: I haz the power, 2016-07-20 14:19:50]

Description was changed from

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Stub cache tests now work even when snapshot is enabled.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701
==========

to

==========
[ic] Fix megamorphic stub cache probing on some platforms.

This CL fixes weird performance implications when changing layout of Code::flags
field:
it happened that the unused ICStateField with MONOMORPHIC value in the handlers'
flags
was accidentally offsetting the underflow bug in stub cache probing code on arm,
arm64,
mips and mips64.

Stub cache tests now work even when snapshot is enabled.

Drive-by-change: Fixed counters manipulation on arm64 and mips64.

BUG=chromium:618701

Committed: https://crrev.com/7da34f8acb99bc513f366aaa8be23742eb30807b
Cr-Commit-Position: refs/heads/master@{#37910}
==========

[commit-bot: I haz the power, 2016-07-20 14:19:51]

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