[turbofan] Effect linearization after representation inference.

[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect chain according to the schedule. It also connects allocating representation changes to the effect chain, and removes the BeginRegion and EndRegion nodes - they should not be needed anymore because all effectful nodes should be already wired-in.

This is an intermediate CL - the next step is to move lowering of the Change*ToTaggedEffect nodes to StateEffectIntroduction so that we do not have to introduce the effectful versions of nodes.

Committed: https://crrev.com/b9e287c6d97e02ee46daa89c8f1b698d567cac6c
Cr-Commit-Position: refs/heads/master@{#35565}

[Jarin, 2016-04-03 11:12:24]

Description was changed from

==========
[turbofan] State effect introduction (WIP).
==========

to

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we can do not
even have to introduce the nodes. We could also try inserting the changes to the
control chain so that we help the scheduler with floating control.
==========

[Jarin, 2016-04-03 11:16:51]

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

[commit-bot: I haz the power, 2016-04-03 11:16:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849603002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849603002/80001

[commit-bot: I haz the power, 2016-04-03 11:20:49]

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

[commit-bot: I haz the power, 2016-04-03 11:20:50]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
  v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/3709)
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
  v8_linux_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/3722)
  v8_win_compile_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/1...)

[Jarin, 2016-04-03 20:26:37]

Description was changed from

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we can do not
even have to introduce the nodes. We could also try inserting the changes to the
control chain so that we help the scheduler with floating control.
==========

to

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we do not have to
introduce the effectful versions of nodes.
==========

[Jarin, 2016-04-03 20:26:37]

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

[Jarin, 2016-04-03 20:27:41]

Could you take a look, please?

[Jarin, 2016-04-16 07:38:01]

Could you take another look, please?

[Jarin, 2016-04-16 08:02:19]

On 2016/04/16 07:38:01, Jarin wrote:
> Could you take another look, please?

As far I can see, compilation time goes up by 10% in --turbo-stats, but overall
Octane score appears to be unaffected (strange).

[Benedikt Meurer, 2016-04-18 04:34:18]

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
File src/compiler/effect-control-linearizer.cc (right):

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.cc:105: // The control node should be the
first.
DCHECK this assumption.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.cc:149: // End block is a just dummy, no
effect needed.
a just dummy -> just a dummy

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.cc:151: DCHECK_EQ(1, block->size());
DCHECK_EQ(1u, block->size())

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
File src/compiler/effect-control-linearizer.h (right):

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.h:5: #ifndef
V8_COMPILER_EFFECT_CONTROL_LINEARIZER_H_
Please rename to effect-control-linearization.h/.cc

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.h:26: class EffectControlLinearizer {
Please rename to EffectControlLinearization to match the naming scheme.

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

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/pipeline....
src/compiler/pipeline.cc:804: struct FixLowLevelEffectsPhase {
Can we have a single ControlEffectLinearizationPhase here, which does both the
scheduling and the actual linearization? using a temporary zone for the
schedule.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/pipeline....
src/compiler/pipeline.cc:848: struct EffectScheduleTrimmingPhase {
Can you put a TODO on this guy, that we only need to do this because the
scheduler is still confused by dead uses.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/pipeline....
src/compiler/pipeline.cc:1285: if (!info()->shared_info()->asm_function()) {
How bad is this actually for asm.js functions?

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/schedule.cc
File src/compiler/schedule.cc (right):

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/schedule....
src/compiler/schedule.cc:223: DCHECK(IsPotentiallyThrowingCall(call->opcode()));
Can you just run JSGenericLowering first so that the Scheduler doesn't need to
know about JS operators? Or at least put a TODO here that we fix that later.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/scheduler.cc
File src/compiler/scheduler.cc (right):

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/scheduler...
src/compiler/scheduler.cc:327: #define BUILD_BLOCK_JS_CASE(Name) case
IrOpcode::k##Name:
Same here, see comment in header.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/scheduler...
src/compiler/scheduler.cc:371: #define CONNECT_BLOCK_JS_CASE(Name) case
IrOpcode::k##Name:
Same here, see comment above.

[Jarin, 2016-04-18 07:53:33]

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
File src/compiler/effect-control-linearizer.cc (right):

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.cc:105: // The control node should be the
first.
On 2016/04/18 04:34:18, Benedikt Meurer wrote:
> DCHECK this assumption.

Done.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.cc:149: // End block is a just dummy, no
effect needed.
On 2016/04/18 04:34:18, Benedikt Meurer wrote:
> a just dummy -> just a dummy

Done.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/effect-co...
src/compiler/effect-control-linearizer.cc:151: DCHECK_EQ(1, block->size());
On 2016/04/18 04:34:18, Benedikt Meurer wrote:
> DCHECK_EQ(1u, block->size())

Done.

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

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/pipeline....
src/compiler/pipeline.cc:804: struct FixLowLevelEffectsPhase {
On 2016/04/18 04:34:18, Benedikt Meurer wrote:
> Can we have a single ControlEffectLinearizationPhase here, which does both the
> scheduling and the actual linearization? using a temporary zone for the
> schedule.

Done.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/pipeline....
src/compiler/pipeline.cc:848: struct EffectScheduleTrimmingPhase {
On 2016/04/18 04:34:18, Benedikt Meurer wrote:
> Can you put a TODO on this guy, that we only need to do this because the
> scheduler is still confused by dead uses.

Done.

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/schedule.cc
File src/compiler/schedule.cc (right):

https://codereview.chromium.org/1849603002/diff/280001/src/compiler/schedule....
src/compiler/schedule.cc:223: DCHECK(IsPotentiallyThrowingCall(call->opcode()));
On 2016/04/18 04:34:18, Benedikt Meurer wrote:
> Can you just run JSGenericLowering first so that the Scheduler doesn't need to
> know about JS operators? Or at least put a TODO here that we fix that later.

I think the scheduler should be able to schedule any graph. Arguably, we might
want to have a better mechanism than explicit opcode check here (such as
operator properties or some such).

[Benedikt Meurer, 2016-04-18 07:58:11]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-04-18 07:58:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849603002/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849603002/340001

[commit-bot: I haz the power, 2016-04-18 07:58:19]

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

[commit-bot: I haz the power, 2016-04-18 07:58:20]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[Jarin, 2016-04-18 08:05:00]

The CQ bit was checked by jarin@chromium.org

[commit-bot: I haz the power, 2016-04-18 08:05:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849603002/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849603002/340001

[commit-bot: I haz the power, 2016-04-18 08:05:08]

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

[commit-bot: I haz the power, 2016-04-18 08:05:10]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[Benedikt Meurer, 2016-04-18 08:11:41]

lgtm

[Benedikt Meurer, 2016-04-18 08:11:54]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-04-18 08:11:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849603002/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849603002/340001

[commit-bot: I haz the power, 2016-04-18 08:28:40]

Description was changed from

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we do not have to
introduce the effectful versions of nodes.
==========

to

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we do not have to
introduce the effectful versions of nodes.
==========

[commit-bot: I haz the power, 2016-04-18 08:28:41]

Committed patchset #18 (id:340001)

[commit-bot: I haz the power, 2016-04-18 08:30:04]

Description was changed from

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we do not have to
introduce the effectful versions of nodes.
==========

to

==========
[turbofan] Effect linearization after representation inference.

This introduces a compiler pass that schedules the graph and re-wires effect
chain according to the schedule. It also connects allocating representation
changes to the effect chain, and removes the BeginRegion and EndRegion nodes -
they should not be needed anymore because all effectful nodes should be already
wired-in.

This is an intermediate CL - the next step is to move lowering of the
Change*ToTaggedEffect nodes to StateEffectIntroduction so that we do not have to
introduce the effectful versions of nodes.

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

[commit-bot: I haz the power, 2016-04-18 08:30:06]

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