[compiler][modules] Constant-fold the loads of module cells.

[compiler][modules] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this: if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.
   That is, a JSLoadModule with a Module HeapConstant input becomes
   a LoadField with a Cell HeapConstant input, and similarly for
   JSStoreModule.

BUG=v8:1569
Review-Url: https://codereview.chromium.org/2841613002
Cr-Commit-Position: refs/heads/master@{#45083}
Committed: https://chromium.googlesource.com/v8/v8/+/24d7890151cdaae0bf373a30bc8fbd603bfbfa9a

[neis, 2017-04-24 14:49:21]

Description was changed from

==========
[compiler] Generalize context specialization to allow outer contexts.

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

to

==========
[compiler] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context can be any
outer context of the function, not necessarily the immediate outer context.

2. Based on this, if function specialization is disabled, then specialize for
the module context if there is one.

3. Extend typed lowering of module loads and stores such that if the operand is
a Module constant, we constant-fold the cell load.

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

[neis, 2017-04-24 14:49:54]

Description was changed from

==========
[compiler] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context can be any
outer context of the function, not necessarily the immediate outer context.

2. Based on this, if function specialization is disabled, then specialize for
the module context if there is one.

3. Extend typed lowering of module loads and stores such that if the operand is
a Module constant, we constant-fold the cell load.

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

to

==========
[compiler] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this, if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.

BUG=v8:1569
==========

[neis, 2017-04-24 14:50:12]

Description was changed from

==========
[compiler] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this, if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.

BUG=v8:1569
==========

to

==========
[compiler][modules] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this, if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.

BUG=v8:1569
==========

[neis, 2017-04-24 14:50:17]

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

[commit-bot: I haz the power, 2017-04-24 14:50:19]

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-04-24 14:51:49]

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

[commit-bot: I haz the power, 2017-04-24 14:51:49]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
  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/39648)

[neis, 2017-04-25 07:58:42]

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

[commit-bot: I haz the power, 2017-04-25 07:58:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[neis, 2017-04-25 08:14:44]

Description was changed from

==========
[compiler][modules] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this, if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.

BUG=v8:1569
==========

to

==========
[compiler][modules] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this: if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.
   That is, a JSLoadModule with a Module HeapConstant input becomes
   a LoadField with a Cell HeapConstant input, and similarly for
   JSStoreModule.

BUG=v8:1569
==========

[neis, 2017-04-25 08:14:44]

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

[neis, 2017-04-25 08:15:06]

Patchset #1 (id:1) has been deleted

[neis, 2017-04-25 08:15:14]

Patchset #1 (id:20001) has been deleted

[neis, 2017-04-25 08:15:20]

Patchset #1 (id:40001) has been deleted

[neis, 2017-04-25 08:15:26]

Patchset #1 (id:60001) has been deleted

[neis, 2017-04-25 08:15:34]

Patchset #1 (id:80001) has been deleted

[neis, 2017-04-25 08:15:52]

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

[commit-bot: I haz the power, 2017-04-25 08:15:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[neis, 2017-04-25 08:17:31]

PTAL

[commit-bot: I haz the power, 2017-04-25 08:39:37]

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

[commit-bot: I haz the power, 2017-04-25 08:39:38]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2017-05-03 17:28:16]

Looking good. Just one question about {GetModuleContext} in the pipeline.cc
file.

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-contex...
File src/compiler/js-context-specialization.cc (right):

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-contex...
src/compiler/js-context-specialization.cc:99: MaybeHandle<Context>
GetSpecializationContext(Node* node,
nit: Needs a short comment explaining the semantics of "requested_depth" IMHO.

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-typed-...
File src/compiler/js-typed-lowering.cc (right):

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-typed-...
src/compiler/js-typed-lowering.cc:1407: Cell* cell_constant =
module_constant->GetCell(cell_index);
nit: Let's just make the local variable handlified, for safety, to not have any
raw pointers floating around at all (not matter how short their lifetime). That
way it is super clear that everything here is handlified and safe. I am thinking
along the lines of ...

Handle<Cell> cell(module_constant->GetCell(cell_index), isolate());
return jsgraph()->HeapConstant(cell);

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/pipeline....
src/compiler/pipeline.cc:794: if (current != nullptr &&
current->IsModuleContext()) {
question: Can we ever hit the {nullptr} case here? There should always be a
native context on any context chain AFAICT. What am I missing?

[neis, 2017-05-04 10:36:21]

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-contex...
File src/compiler/js-context-specialization.cc (right):

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-contex...
src/compiler/js-context-specialization.cc:99: MaybeHandle<Context>
GetSpecializationContext(Node* node,
On 2017/05/03 17:28:16, Michael Starzinger wrote:
> nit: Needs a short comment explaining the semantics of "requested_depth" IMHO.

Done.

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-typed-...
File src/compiler/js-typed-lowering.cc (right):

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/js-typed-...
src/compiler/js-typed-lowering.cc:1407: Cell* cell_constant =
module_constant->GetCell(cell_index);
On 2017/05/03 17:28:16, Michael Starzinger wrote:
> nit: Let's just make the local variable handlified, for safety, to not have
any
> raw pointers floating around at all (not matter how short their lifetime).
That
> way it is super clear that everything here is handlified and safe. I am
thinking
> along the lines of ...
> 
> Handle<Cell> cell(module_constant->GetCell(cell_index), isolate());
> return jsgraph()->HeapConstant(cell);

Done.

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/2841613002/diff/120001/src/compiler/pipeline....
src/compiler/pipeline.cc:794: if (current != nullptr &&
current->IsModuleContext()) {
On 2017/05/03 17:28:16, Michael Starzinger wrote:
> question: Can we ever hit the {nullptr} case here? There should always be a
> native context on any context chain AFAICT. What am I missing?

As discussed, I removed this condition.

[Michael Starzinger, 2017-05-04 10:38:41]

LGTM. Nice! Thanks!

[neis, 2017-05-04 10:39:30]

The CQ bit was checked by neis@chromium.org

[commit-bot: I haz the power, 2017-05-04 10:39:35]

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-05-04 11:09:20]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1493894370025310,
"parent_rev": "0733add072bdb3e5e95f8c4dda192fc799e31e04", "commit_rev":
"24d7890151cdaae0bf373a30bc8fbd603bfbfa9a"}

[commit-bot: I haz the power, 2017-05-04 11:09:27]

Description was changed from

==========
[compiler][modules] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this: if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.
   That is, a JSLoadModule with a Module HeapConstant input becomes
   a LoadField with a Cell HeapConstant input, and similarly for
   JSStoreModule.

BUG=v8:1569
==========

to

==========
[compiler][modules] Constant-fold loads of module cells.

1. Generalize context specialization such that the provided context
   can be any outer context of the function, not necessarily the
   immediate outer context.

2. Based on this: if function specialization is disabled, then
   specialize for the module context if there is one.

3. Extend typed lowering of module loads and stores such that if
   the operand is a Module constant, we constant-fold the cell load.
   That is, a JSLoadModule with a Module HeapConstant input becomes
   a LoadField with a Cell HeapConstant input, and similarly for
   JSStoreModule.

BUG=v8:1569

Review-Url: https://codereview.chromium.org/2841613002
Cr-Commit-Position: refs/heads/master@{#45083}
Committed:
https://chromium.googlesource.com/v8/v8/+/24d7890151cdaae0bf373a30bc8fbd603bf...
==========

[commit-bot: I haz the power, 2017-05-04 11:09:28]

Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/v8/v8/+/24d7890151cdaae0bf373a30bc8fbd603bf...