[stubs] Port StoreFastElementsStub to TurboFan.

[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that ensures that the value is kept alive even during GC.

BUG=v8:5269
Committed: https://crrev.com/cce56a3f470d4f8f4c241208b166dea5e09d0620
Cr-Commit-Position: refs/heads/master@{#39407}

[Igor Sheludko, 2016-09-12 15:44:09]

Description was changed from

==========
[stubs] Port StoreFastElementsStub to TurboFan.

cherry-picking missing parts

fixing

Implementing typed array support

remove debug break

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

to

==========
[stubs] Port StoreFastElementsStub to TurboFan.

BUG=v8:5269
==========

[Igor Sheludko, 2016-09-12 15:44:22]

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

[commit-bot: I haz the power, 2016-09-12 15:44:27]

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-09-12 15:44:32]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-09-12 15:45:58]

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

[commit-bot: I haz the power, 2016-09-12 15:45:59]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/12550)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/12545)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/23823)

[Igor Sheludko, 2016-09-12 16:01:22]

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

[commit-bot: I haz the power, 2016-09-12 16:01:29]

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-09-12 16:04:36]

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

[commit-bot: I haz the power, 2016-09-12 16:04:39]

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

[Igor Sheludko, 2016-09-12 20:16:04]

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

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

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-09-12 20:17:37]

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

[commit-bot: I haz the power, 2016-09-12 20:17:38]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/12570)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/8678)

[Igor Sheludko, 2016-09-13 07:09:26]

Patchset #1 (id:20001) has been deleted

[Igor Sheludko, 2016-09-13 07:11:33]

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

[commit-bot: I haz the power, 2016-09-13 07:11: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-09-13 07:25:50]

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

[commit-bot: I haz the power, 2016-09-13 07:25:51]

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

[Igor Sheludko, 2016-09-13 08:20:49]

Patchset #1 (id:40001) has been deleted

[Igor Sheludko, 2016-09-13 08:22:30]

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

[commit-bot: I haz the power, 2016-09-13 08:22:37]

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-09-13 08:26:26]

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

[commit-bot: I haz the power, 2016-09-13 08:26:27]

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

[Igor Sheludko, 2016-09-13 08:39:46]

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

[Igor Sheludko, 2016-09-13 08:39:47]

PTAL

[Benedikt Meurer, 2016-09-13 09:08:00]

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

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4473: namespace {
Nit: add empty line.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4496: }  // namespace
Nit: add empty line.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4554: bailout);
Maybe you want to check the upper bound as well, and not store in that case
(i.e. skip the rest of the function).

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4556: DCHECK(store_mode == STANDARD_STORE);
Nit: DCHECK_EQ

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4565: Node* backing_store =
IntPtrAdd(external_pointer, base_pointer);
You must not have a GC afterwards before you store to this, otherwise you'll get
weird crashers.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4574: value = TryToIntptr(value, bailout);
Maybe add a TODO here, that JS truncation would be fine for word8/16/32 to
truncate the value.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4577: parameter_mode);
You probably need to insert a Retain for the buffer.
Maybe the same for the loads.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4650: if (kind == FAST_SMI_ELEMENTS) {
This shouldn't be necessary, it was only necessary in CS because the storekeyed
could deopt (for smis).

[Igor Sheludko, 2016-09-13 10:33:03]

Patchset #3 (id:100001) has been deleted

[Igor Sheludko, 2016-09-13 10:33:17]

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

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4473: namespace {
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> Nit: add empty line.

Done.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4496: }  // namespace
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> Nit: add empty line.

Done.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4554: bailout);
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> Maybe you want to check the upper bound as well, and not store in that case
> (i.e. skip the rest of the function).

Done. Thanks!

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4556: DCHECK(store_mode == STANDARD_STORE);
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> Nit: DCHECK_EQ

Done.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4565: Node* backing_store =
IntPtrAdd(external_pointer, base_pointer);
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> You must not have a GC afterwards before you store to this, otherwise you'll
get
> weird crashers.

Done.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4574: value = TryToIntptr(value, bailout);
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> Maybe add a TODO here, that JS truncation would be fine for word8/16/32 to
> truncate the value.

Done.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4577: parameter_mode);
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> You probably need to insert a Retain for the buffer.
> Maybe the same for the loads.

I rearranged the code so that the value preparation is done before we access the
buffer and friends.

https://codereview.chromium.org/2330063002/diff/60001/src/code-stub-assembler...
src/code-stub-assembler.cc:4650: if (kind == FAST_SMI_ELEMENTS) {
On 2016/09/13 09:08:00, Benedikt Meurer wrote:
> This shouldn't be necessary, it was only necessary in CS because the
storekeyed
> could deopt (for smis).

Indeed, it's not necessary. Thanks!

[Igor Sheludko, 2016-09-13 10:33:21]

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

[commit-bot: I haz the power, 2016-09-13 10:33:29]

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-09-13 11:00:50]

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

[commit-bot: I haz the power, 2016-09-13 11:00:50]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2016-09-13 11:02:57]

LGTM with comment.

https://codereview.chromium.org/2330063002/diff/120001/src/code-stub-assemble...
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2330063002/diff/120001/src/code-stub-assemble...
src/code-stub-assembler.cc:4768: StoreElement(backing_store, elements_kind, key,
value, parameter_mode);
Can we Retain the buffer for safety reasons for now? And add a TODO.

[Igor Sheludko, 2016-09-14 07:22:32]

Patchset #4 (id:140001) has been deleted

[Igor Sheludko, 2016-09-14 07:24:01]

Description was changed from

==========
[stubs] Port StoreFastElementsStub to TurboFan.

BUG=v8:5269
==========

to

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operator that ensures that the value is kept alive
during GC. 

BUG=v8:5269
==========

[Igor Sheludko, 2016-09-14 07:24:08]

https://codereview.chromium.org/2330063002/diff/120001/src/code-stub-assemble...
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2330063002/diff/120001/src/code-stub-assemble...
src/code-stub-assembler.cc:4768: StoreElement(backing_store, elements_kind, key,
value, parameter_mode);
On 2016/09/13 11:02:57, Benedikt Meurer wrote:
> Can we Retain the buffer for safety reasons for now? And add a TODO.

Done.

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

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

[commit-bot: I haz the power, 2016-09-14 07:24:45]

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-09-14 07:34:53]

Description was changed from

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operator that ensures that the value is kept alive
during GC. 

BUG=v8:5269
==========

to

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that guarantees that the value is kept
alive even during GC. 

BUG=v8:5269
==========

[Igor Sheludko, 2016-09-14 07:35:12]

Description was changed from

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that guarantees that the value is kept
alive even during GC. 

BUG=v8:5269
==========

to

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that ensures that the value is kept alive
even during GC. 

BUG=v8:5269
==========

[Igor Sheludko, 2016-09-14 07:35:26]

Patchset #4 (id:160001) has been deleted

[Igor Sheludko, 2016-09-14 07:36:33]

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

[commit-bot: I haz the power, 2016-09-14 07:36:39]

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-09-14 08:02:34]

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

[commit-bot: I haz the power, 2016-09-14 08:02:36]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-09-14 08:23:16]

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

[commit-bot: I haz the power, 2016-09-14 08:23:24]

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-09-14 08:50:24]

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

[commit-bot: I haz the power, 2016-09-14 08:50:25]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-09-14 09:25:38]

The CQ bit was checked by ishell@chromium.org

[Igor Sheludko, 2016-09-14 09:25:38]

The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/2330063002/#ps200001
(title: "Retain removed, a comment added")

[commit-bot: I haz the power, 2016-09-14 09:25:48]

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-09-14 09:28:09]

Description was changed from

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that ensures that the value is kept alive
even during GC. 

BUG=v8:5269
==========

to

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that ensures that the value is kept alive
even during GC. 

BUG=v8:5269
==========

[commit-bot: I haz the power, 2016-09-14 09:28:10]

Committed patchset #5 (id:200001)

[commit-bot: I haz the power, 2016-09-14 09:29:01]

Description was changed from

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that ensures that the value is kept alive
even during GC. 

BUG=v8:5269
==========

to

==========
[stubs] Port StoreFastElementsStub to TurboFan.

This CL adds CSA::Retain() operation that ensures that the value is kept alive
even during GC.

BUG=v8:5269

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

[commit-bot: I haz the power, 2016-09-14 09:29:02]

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