[turbofan] Wire in floating control during effect linearization phase.

[turbofan] Wire in floating control during effect linearization phase.

Committed: https://crrev.com/9cae24b8cc7a1ed14eaf305b7ce8bbc8f8175634
Cr-Commit-Position: refs/heads/master@{#35747}

[Jarin, 2016-04-24 18:04:30]

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

[Jarin, 2016-04-24 18:04:30]

Could you take a look, please?

[Benedikt Meurer, 2016-04-24 18:12:50]

Looks nice. Need to understand the IfSuccess magic still.

https://codereview.chromium.org/1921483002/diff/20001/src/compiler/effect-con...
File src/compiler/effect-control-linearizer.cc (right):

https://codereview.chromium.org/1921483002/diff/20001/src/compiler/effect-con...
src/compiler/effect-control-linearizer.cc:294: for (Edge edge :
success_input->use_edges()) {
Not sure I really get this one: This tries to update all "other" control uses of
the call and thread the IfSuccess in instead? Can we somehow document this this
with some ascii art? And maybe we could even put this whole thing into a
dedicated ProcessIfSuccess() method to highlight the fact that this is tricky
stuff.

[Benedikt Meurer, 2016-04-24 18:15:07]

As discussed, LGTM once IfSuccess black magic is addressed :-)

[Jarin, 2016-04-24 19:21:23]

I rewrote the IfSuccess part.

This version should be more like what we agreed to on Friday, and I have to
admit this looks better (even though it is still a bit subtle because of the
exceptional/non-exceptional call distinction).

WDYT?

https://codereview.chromium.org/1921483002/diff/20001/src/compiler/effect-con...
File src/compiler/effect-control-linearizer.cc (right):

https://codereview.chromium.org/1921483002/diff/20001/src/compiler/effect-con...
src/compiler/effect-control-linearizer.cc:294: for (Edge edge :
success_input->use_edges()) {
On 2016/04/24 18:12:49, Benedikt Meurer wrote:
> Not sure I really get this one: This tries to update all "other" control uses
of
> the call and thread the IfSuccess in instead? Can we somehow document this
this
> with some ascii art? And maybe we could even put this whole thing into a
> dedicated ProcessIfSuccess() method to highlight the fact that this is tricky
> stuff.

I have changed this to keep the wiring of IfSuccess while we connect the call
node. That seems more understandable (and less brittle) than wedging-in the
IfSuccess node.

[Benedikt Meurer, 2016-04-25 04:01:18]

Yep, this is quite nice and easier to understand. LGTM.

[Jarin, 2016-04-25 04:25:30]

The CQ bit was checked by jarin@chromium.org

[commit-bot: I haz the power, 2016-04-25 04:25:39]

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

[commit-bot: I haz the power, 2016-04-25 04:27:38]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-25 04:29:33]

Description was changed from

==========
[turbofan] Wire in floating control during effect linearization phase.
==========

to

==========
[turbofan] Wire in floating control during effect linearization phase.

Committed: https://crrev.com/9cae24b8cc7a1ed14eaf305b7ce8bbc8f8175634
Cr-Commit-Position: refs/heads/master@{#35747}
==========

[commit-bot: I haz the power, 2016-04-25 04:29:34]

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