Thread maybe-assigned through the bytecodes.

Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-hole-or-immutable.

Review-Url: https://codereview.chromium.org/2655733003
Cr-Commit-Position: refs/heads/master@{#43000}
Committed: https://chromium.googlesource.com/v8/v8/+/17c2dd388697da626224c371c8181d20d2016d82

[Jarin, 2017-01-25 09:38:56]

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

[commit-bot: I haz the power, 2017-01-25 09:39:00]

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

[Jarin, 2017-01-25 09:44:59]

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

[commit-bot: I haz the power, 2017-01-25 09:45:01]

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, 2017-01-25 09:59:12]

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

[commit-bot: I haz the power, 2017-01-25 09:59:13]

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...)

[Jarin, 2017-01-25 11:34:25]

Description was changed from

==========
Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-immutable.
==========

to

==========
Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-hole-immutable.
==========

[Jarin, 2017-01-25 11:34:40]

Description was changed from

==========
Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-hole-immutable.
==========

to

==========
Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-hole-or-immutable.
==========

[Jarin, 2017-02-07 12:58:32]

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

[commit-bot: I haz the power, 2017-02-07 12:58:44]

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

[Jarin, 2017-02-07 13:01:23]

jarin@chromium.org changed reviewers:
+ bmeurer@chromium.org, rmcilroy@chromium.org

[Jarin, 2017-02-07 13:01:23]

Could you take a look, please?

[commit-bot: I haz the power, 2017-02-07 13:02:34]

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

[commit-bot: I haz the power, 2017-02-07 13:02:37]

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...)
  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_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)

[Jarin, 2017-02-07 13:06:32]

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

[commit-bot: I haz the power, 2017-02-07 13:06:35]

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, 2017-02-07 13:17:42]

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

[commit-bot: I haz the power, 2017-02-07 13:17:43]

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...)

[Jarin, 2017-02-07 13:35:16]

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

[commit-bot: I haz the power, 2017-02-07 13:35:18]

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

[neis, 2017-02-07 13:39:24]

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

[neis, 2017-02-07 13:39:25]

lgtm

[Benedikt Meurer, 2017-02-07 13:43:49]

lgtm

[commit-bot: I haz the power, 2017-02-07 13:59:50]

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

[commit-bot: I haz the power, 2017-02-07 13:59:51]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-02-07 14:02:14]

Interpreter LGTM, thanks.

https://codereview.chromium.org/2655733003/diff/60001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2655733003/diff/60001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:791:
environment()->BindAccumulator(node);
nit - you could pull out the shared code to a BuildLdaContextSlot helper
function which both this and VisitLdaContextSlot call to avoid duplicating code.

https://codereview.chromium.org/2655733003/diff/60001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2655733003/diff/60001/src/interpreter/interpr...
src/interpreter/interpreter.cc:564: DoLdaContextSlot(assembler);
Could you add a TODO for Danno to share the machine code for the bytecode
handlers for both this and DoLdaContextSlot

+Danno - FYI.

[rmcilroy, 2017-02-07 14:14:52]

Actually +danno (see comment on interpreter.cc:564).

[Jarin, 2017-02-07 14:31:05]

https://codereview.chromium.org/2655733003/diff/60001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2655733003/diff/60001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:791:
environment()->BindAccumulator(node);
On 2017/02/07 14:02:14, rmcilroy wrote:
> nit - you could pull out the shared code to a BuildLdaContextSlot helper
> function which both this and VisitLdaContextSlot call to avoid duplicating
code.

I thought about this and I'd prefer it this way because the duplication is quite
small and there is really no logic that needs to be kept in sync. I also suspect
a separate BuildLdaContextSlot would end up line-count positive.

https://codereview.chromium.org/2655733003/diff/60001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2655733003/diff/60001/src/interpreter/interpr...
src/interpreter/interpreter.cc:564: DoLdaContextSlot(assembler);
On 2017/02/07 14:02:14, rmcilroy wrote:
> Could you add a TODO for Danno to share the machine code for the bytecode
> handlers for both this and DoLdaContextSlot
> 
> +Danno - FYI.

Done.

[Jarin, 2017-02-07 14:31:13]

The CQ bit was checked by jarin@chromium.org

[Jarin, 2017-02-07 14:31:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, neis@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2655733003/#ps80001
(title: "Comment")

[commit-bot: I haz the power, 2017-02-07 14:31:24]

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, 2017-02-07 14:54:23]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486477873976780,
"parent_rev": "32971301ea388bd66b84d013ac8bdf3263ad77a7", "commit_rev":
"17c2dd388697da626224c371c8181d20d2016d82"}

[commit-bot: I haz the power, 2017-02-07 14:54:31]

Description was changed from

==========
Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-hole-or-immutable.
==========

to

==========
Thread maybe-assigned through the bytecodes.

This introduces LoadImmutableContextSlot and LoadImmutableCurrentContextSlot
bytecodes, which are emitted when reading from never-assigned context slot.

There is a subtlety here: the slot are not immutable, the meaning is
actually undefined-or-hole-or-immutable.

Review-Url: https://codereview.chromium.org/2655733003
Cr-Commit-Position: refs/heads/master@{#43000}
Committed:
https://chromium.googlesource.com/v8/v8/+/17c2dd388697da626224c371c8181d20d20...
==========

[commit-bot: I haz the power, 2017-02-07 14:54:32]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/17c2dd388697da626224c371c8181d20d20...

[Michael Hablich, 2017-02-07 20:19:01]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2680923003/ by hablich@chromium.org.

The reason for reverting is: needed for properly reverting
https://chromium.googlesource.com/v8/v8/+/f3ae5ccf57690d8c2d87c4fe1d10b103ad6....