[ic] [stubs] Don't use Code::flags in megamorphic stub cache hash computations.

[ic] [stubs] Don't use Code::flags in megamorphic stub cache hash computations.

This should avoid weird performance issues when changing layout of Code::flags field.

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

[Igor Sheludko, 2016-07-12 11:11:12]

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

[commit-bot: I haz the power, 2016-07-12 11:11:19]

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

[Igor Sheludko, 2016-07-12 11:12:30]

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

[Igor Sheludko, 2016-07-12 11:12:31]

PTAL

[commit-bot: I haz the power, 2016-07-12 11:17:34]

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

[commit-bot: I haz the power, 2016-07-12 11:17:35]

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

[Igor Sheludko, 2016-07-12 12:35:02]

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

[commit-bot: I haz the power, 2016-07-12 12:35:09]

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-12 12:55:50]

LGTM with comments.

https://codereview.chromium.org/2147433002/diff/1/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2147433002/diff/1/src/code-stub-assembler.cc#...
src/code-stub-assembler.cc:2806: // Check that the flags match what we're
looking for.
Any particular reason this isn't behind an #ifdef DEBUG?

https://codereview.chromium.org/2147433002/diff/1/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/2147433002/diff/1/src/code-stubs.h#newcode1389
src/code-stubs.h:1389: ExtraICState GetExtraICState() const override { return
Code::LOAD_IC; }
Code::LOAD_IC is a Code::Kind, not an ExtraICState. Why is this needed?

https://codereview.chromium.org/2147433002/diff/1/src/ic/stub-cache.cc
File src/ic/stub-cache.cc (right):

https://codereview.chromium.org/2147433002/diff/1/src/ic/stub-cache.cc#newcode29
src/ic/stub-cache.cc:29: Code::Flags flags = code->flags();
Just inline this variable into the DCHECK to make the compilers happy.

[commit-bot: I haz the power, 2016-07-12 13:02:13]

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

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

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-07-12 13:06:55]

https://codereview.chromium.org/2147433002/diff/1/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2147433002/diff/1/src/code-stub-assembler.cc#...
src/code-stub-assembler.cc:2806: // Check that the flags match what we're
looking for.
On 2016/07/12 12:55:49, Jakob wrote:
> Any particular reason this isn't behind an #ifdef DEBUG?

TF compiler is good at throwing non used code away. The Assert is a no op in
release mode and therefore TF will be able to throw the whole tree of unused
computations away.

https://codereview.chromium.org/2147433002/diff/1/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/2147433002/diff/1/src/code-stubs.h#newcode1389
src/code-stubs.h:1389: ExtraICState GetExtraICState() const override { return
Code::LOAD_IC; }
On 2016/07/12 12:55:49, Jakob wrote:
> Code::LOAD_IC is a Code::Kind, not an ExtraICState. Why is this needed?

This stub is a handler and handlers store the IC kind in the extra IC state (see
Code::ComputeHandlerFlags()). I had to add this in order to fix the failing
flags.

https://codereview.chromium.org/2147433002/diff/1/src/ic/stub-cache.cc
File src/ic/stub-cache.cc (right):

https://codereview.chromium.org/2147433002/diff/1/src/ic/stub-cache.cc#newcode29
src/ic/stub-cache.cc:29: Code::Flags flags = code->flags();
On 2016/07/12 12:55:50, Jakob wrote:
> Just inline this variable into the DCHECK to make the compilers happy.

Done.

[Igor Sheludko, 2016-07-14 10:06:34]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2016-07-14 10:06:39]

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-14 10:32:04]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-14 10:33:40]

Description was changed from

==========
[ic] [stubs] Don't use Code::flags in megamorphic stub cache hash computations.

This should avoid weird performance issues when changing layout of Code::flags
field.

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

to

==========
[ic] [stubs] Don't use Code::flags in megamorphic stub cache hash computations.

This should avoid weird performance issues when changing layout of Code::flags
field.

BUG=chromium:618701

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

[commit-bot: I haz the power, 2016-07-14 10:33:41]

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

[Igor Sheludko, 2016-07-15 16:07:28]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2147213004/ by ishell@chromium.org.

The reason for reverting is: Unfortunately, the performance issues are still
there on Android..

[Igor Sheludko, 2016-07-19 13:29:56]

Patchset #3 (id:40001) has been deleted

[Igor Sheludko, 2016-07-19 13:30:52]

Description was changed from

==========
[ic] [stubs] Don't use Code::flags in megamorphic stub cache hash computations.

This should avoid weird performance issues when changing layout of Code::flags
field.

BUG=chromium:618701

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

to

==========
[ic] [stubs] Don't use Code::flags in megamorphic stub cache hash computations.

This should avoid weird performance issues when changing layout of Code::flags
field.

BUG=chromium:618701

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

[Igor Sheludko, 2016-07-19 13:31:29]

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

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

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 13:38:03]

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

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

Dry run: 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...)

[Igor Sheludko, 2016-07-19 14:36:24]

Patchset #4 (id:80001) has been deleted

[Igor Sheludko, 2016-07-19 17:06:59]

Patchset #4 (id:100001) has been deleted

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

Patchset #4 (id:120001) has been deleted

[Igor Sheludko, 2016-07-19 17:07:16]

Patchset #4 (id:140001) has been deleted

[Igor Sheludko, 2016-07-19 17:07:26]

Patchset #4 (id:160001) has been deleted