[stubs] Port KeyedStoreIC dispatcher to TurboFan

[stubs] Port KeyedStoreIC dispatcher to TurboFan

BUG=

Committed: https://crrev.com/194c43a103c679b26c58a29e619a80dba1b41e39
Cr-Commit-Position: refs/heads/master@{#40423}

[Jakob Kummerow, 2016-10-18 19:11:10]

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

[Jakob Kummerow, 2016-10-18 19:11:11]

Igor: PTAL.

Benedikt: Please review src/compiler/ (just another TailCallStub overload).

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

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:4731: void
CodeStubAssembler::HandleKeyedStorePolymorphicCase(
This is a more complex version of the method above. We could combine them, but I
suspect readability would suffer. If you think it should be tried, I can do that
in a follow-up CL.

[Jakob Kummerow, 2016-10-18 19:11:16]

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

[commit-bot: I haz the power, 2016-10-18 19:11:25]

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

[Jarin, 2016-10-18 19:13:51]

jarin@chromium.org changed reviewers:
+ jarin@chromium.org

[Jarin, 2016-10-18 19:13:51]

lgtm for src/compiler/*

[commit-bot: I haz the power, 2016-10-18 19:37:12]

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

[commit-bot: I haz the power, 2016-10-18 19:37:13]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-10-18 21:12:20]

lgtm with nits:

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

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:4731: void
CodeStubAssembler::HandleKeyedStorePolymorphicCase(
On 2016/10/18 19:11:11, Jakob Kummerow wrote:
> This is a more complex version of the method above. We could combine them, but
I
> suspect readability would suffer. If you think it should be tried, I can do
that
> in a follow-up CL.

I think it's fine to keep them separate.

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:4735: DCHECK_EQ(MachineRepresentation::kTagged,
var_handler->rep());
Please add the same check for var_transition_map_cell.

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:4743: Goto(&loop);
Consider using BuildFastLoop() machinery.

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:5643: Label if_transition_call(this);
Probably if_transition_handler is a better name.

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:5671: // We might ahve a name in feedback, and a
fixed array in the next slot.
s/ahve/have/

[Benedikt Meurer, 2016-10-19 03:46:18]

LGTM for compiler.

[Jakob Kummerow, 2016-10-19 09:14:46]

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

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:4735: DCHECK_EQ(MachineRepresentation::kTagged,
var_handler->rep());
On 2016/10/18 21:12:20, Igor Sheludko wrote:
> Please add the same check for var_transition_map_cell.

Done.

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:4743: Goto(&loop);
On 2016/10/18 21:12:20, Igor Sheludko wrote:
> Consider using BuildFastLoop() machinery.

In a follow-up CL, as discussed.

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:5643: Label if_transition_call(this);
On 2016/10/18 21:12:20, Igor Sheludko wrote:
> Probably if_transition_handler is a better name.

Done. (Agreed -- "_call" was a legacy name from handwritten stub code.)

https://codereview.chromium.org/2403483002/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:5671: // We might ahve a name in feedback, and a
fixed array in the next slot.
On 2016/10/18 21:12:20, Igor Sheludko wrote:
> s/ahve/have/

Done.

[Jakob Kummerow, 2016-10-19 09:14:51]

The CQ bit was checked by jkummerow@chromium.org

[Jakob Kummerow, 2016-10-19 09:14:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, ishell@chromium.org, jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/2403483002/#ps60001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-10-19 09:15:01]

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-10-19 10:11:30]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-17 22:05:50]

Description was changed from

==========
[stubs] Port KeyedStoreIC dispatcher to TurboFan

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

to

==========
[stubs] Port KeyedStoreIC dispatcher to TurboFan

BUG=

Committed: https://crrev.com/194c43a103c679b26c58a29e619a80dba1b41e39
Cr-Commit-Position: refs/heads/master@{#40423}
==========

[commit-bot: I haz the power, 2016-11-17 22:05:52]

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