[turbofan] Introduce explicit loop exits markers.

[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).

Committed: https://crrev.com/7a61bbcfd8b1bee2617b32e23a6bbf63cfbc00b3
Cr-Commit-Position: refs/heads/master@{#37769}

[Jarin, 2016-07-13 11:29:21]

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

[commit-bot: I haz the power, 2016-07-13 11:29:30]

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-07-13 11:57:43]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 11:57:44]

Dry run: This issue passed the CQ dry run.

[Jarin, 2016-07-13 12:51:17]

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

[Jarin, 2016-07-13 12:51:18]

Could you take a look, please?

[Jarin, 2016-07-13 12:58:52]

Description was changed from

==========
[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).
==========

to

==========
[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).
==========

[Benedikt Meurer, 2016-07-14 08:12:07]

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

[Benedikt Meurer, 2016-07-14 08:12:07]

bmeurer@chromium.org changed required reviewers:
+ mstarzinger@chromium.org

[Benedikt Meurer, 2016-07-14 08:12:08]

LGTM from my side.

[Michael Starzinger, 2016-07-14 12:13:36]

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.cc:4420: void
AstGraphBuilder::BuildLoopExit(Node* loop, BitVector* assigned_variables,
As discussed offline: Since this is only used by the control builders, I think
we should move this entire logic into the LoopBuilder. We already do similar
mutation of the environment in the TryCatchBuilder (where we push the exception
as an "extra value").

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.h:544: void RenameForLoopExit(Node* loop,
BitVector* assigned_variables);
nit: Needs a short comment explaining purpose of method. Also, can we call it
"PrepareForLoopExit" instead?

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/control-bu...
File src/compiler/control-builders.cc (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/control-bu...
src/compiler/control-builders.cc:88: if (environment()->IsMarkedAsUnreachable())
return;
Please move the unreachability check into Environment::PrepareForLoopExit.

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/opcodes.h
File src/compiler/opcodes.h (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/opcodes.h#...
src/compiler/opcodes.h:61: V(LoopExitMarker)      \
nit: How would you feel about naming this "LoopExitValue" instead?

[Jarin, 2016-07-14 12:45:28]

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

[commit-bot: I haz the power, 2016-07-14 12:45:37]

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

[Jarin, 2016-07-14 12:51:34]

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.cc:4420: void
AstGraphBuilder::BuildLoopExit(Node* loop, BitVector* assigned_variables,
On 2016/07/14 12:13:36, Michael Starzinger wrote:
> As discussed offline: Since this is only used by the control builders, I think
> we should move this entire logic into the LoopBuilder. We already do similar
> mutation of the environment in the TryCatchBuilder (where we push the
exception
> as an "extra value").

Done.

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.h:544: void RenameForLoopExit(Node* loop,
BitVector* assigned_variables);
On 2016/07/14 12:13:36, Michael Starzinger wrote:
> nit: Needs a short comment explaining purpose of method. Also, can we call it
> "PrepareForLoopExit" instead?

Done.

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/control-bu...
File src/compiler/control-builders.cc (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/control-bu...
src/compiler/control-builders.cc:88: if (environment()->IsMarkedAsUnreachable())
return;
On 2016/07/14 12:13:36, Michael Starzinger wrote:
> Please move the unreachability check into Environment::PrepareForLoopExit.

Done.

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/opcodes.h
File src/compiler/opcodes.h (right):

https://codereview.chromium.org/2140673007/diff/20001/src/compiler/opcodes.h#...
src/compiler/opcodes.h:61: V(LoopExitMarker)      \
On 2016/07/14 12:13:36, Michael Starzinger wrote:
> nit: How would you feel about naming this "LoopExitValue" instead?

Done.

[Jarin, 2016-07-14 12:53:03]

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

[commit-bot: I haz the power, 2016-07-14 12:53:11]

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

[Michael Starzinger, 2016-07-14 13:09:40]

LGTM. Thanks!

https://codereview.chromium.org/2140673007/diff/60001/src/compiler/ast-graph-...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/2140673007/diff/60001/src/compiler/ast-graph-...
src/compiler/ast-graph-builder.cc:960: (*values())[i] = rename;
nit: Why not just "values_[i] = rename"?

https://codereview.chromium.org/2140673007/diff/60001/src/compiler/opcodes.h
File src/compiler/opcodes.h (right):

https://codereview.chromium.org/2140673007/diff/60001/src/compiler/opcodes.h#...
src/compiler/opcodes.h:60: V(LoopExit)            \
suggestion: Technically this is a control node, we could move it to
CONTROL_OP_LIST. But I don't mind either way.

[commit-bot: I haz the power, 2016-07-14 13:17:11]

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

[commit-bot: I haz the power, 2016-07-14 13:17:12]

Dry run: This issue passed the CQ dry run.

[Jarin, 2016-07-14 14:57:17]

The CQ bit was checked by jarin@chromium.org

[Jarin, 2016-07-14 14:57:17]

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/2140673007/#ps60001
(title: "Fix")

[commit-bot: I haz the power, 2016-07-14 14:57:29]

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-07-14 14:59:48]

Description was changed from

==========
[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).
==========

to

==========
[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).
==========

[commit-bot: I haz the power, 2016-07-14 14:59:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-14 14:59:55]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 15:02:39]

Description was changed from

==========
[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).
==========

to

==========
[turbofan] Introduce explicit loop exits markers.

This CL introduces explicit LoopExit control nodes at loop exits.
We also attach explicit value renames (LoopExitMarker) and effect
rename (LoopExitEffect) to each loop exit. This is in preparation
to loop peeling, which will replace LoopExit, LoopExitMarker and
LoopExitEffect with Merge, Phi and EffectPhi respectively.

At the moment, we insert loop exit at every return, break,
continue and locally caught throw. We do not yet handle
uncaught throws (including error throws, such as ReferenceError).

Committed: https://crrev.com/7a61bbcfd8b1bee2617b32e23a6bbf63cfbc00b3
Cr-Commit-Position: refs/heads/master@{#37769}
==========

[commit-bot: I haz the power, 2016-07-14 15:02:40]

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