Fix argument allocation dangling effect chains

[turbofan] Fix argument allocation dangling effect chains

Argument allocation in typed lowering was producing
dangling effect chains. This patch fixes three sources
of dangling effect chains.

BUG=

Committed: https://crrev.com/0ec6db4750190014d1442a6929808d32ed281f9e
Cr-Commit-Position: refs/heads/master@{#32140}

[sigurds, 2015-11-19 12:26:17]

Patchset #1 (id:1) has been deleted

[sigurds, 2015-11-19 12:33:32]

Description was changed from

==========
Fix some event deps.

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

to

==========
[turbofan] Argument allocation in typed lowering was producing dangling effect
chains. This patch fixes three sources of dangling effect chains. 

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

[sigurds, 2015-11-19 12:33:32]

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

[sigurds, 2015-11-19 12:51:35]

This patch the dangling event chains problem (i.e. all instances I have
encountered).

[Michael Starzinger, 2015-11-20 09:15:56]

Fix in general looks good. Can we change the CL description to talk about
"effect" instead of "event"?

https://codereview.chromium.org/1447323005/diff/40001/src/compiler/js-typed-l...
File src/compiler/js-typed-lowering.cc (right):

https://codereview.chromium.org/1447323005/diff/40001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:1489: AllocationBuilder a(jsgraph(),
allocate_effect, control);
Instead of having the out-parameter passing, could we just pick the correct
effect dependency here with ...

effect = elements->op()->EffectOutputCount() > 0 ? elements : effect;

https://codereview.chromium.org/1447323005/diff/40001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:1533: AllocationBuilder a(jsgraph(),
allocate_effect, control);
Likewise.

[Michael Starzinger, 2015-11-20 09:18:24]

Also many tools will be happier when you limit first line of CL description to
72 characters.

[sigurds, 2015-11-20 10:14:21]

Description was changed from

==========
[turbofan] Argument allocation in typed lowering was producing dangling effect
chains. This patch fixes three sources of dangling effect chains. 

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

to

==========
[turbofan] Argument allocation in typed lowering was
producing dangling effect chains. This patch fixes three
sources of dangling effect chains. 

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

[sigurds, 2015-11-20 10:14:42]

Description was changed from

==========
[turbofan] Argument allocation in typed lowering was
producing dangling effect chains. This patch fixes three
sources of dangling effect chains. 

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

to

==========
[turbofan] Fix argument allocation dangling effect chains

Argument allocation in typed lowering was
producing dangling effect chains. This patch fixes three
sources of dangling effect chains. 

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

[sigurds, 2015-11-20 10:23:48]

Description was changed from

==========
[turbofan] Fix argument allocation dangling effect chains

Argument allocation in typed lowering was
producing dangling effect chains. This patch fixes three
sources of dangling effect chains. 

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

to

==========
[turbofan] Fix argument allocation dangling effect chains

Argument allocation in typed lowering was producing 
dangling effect chains. This patch fixes three sources
of dangling effect chains. 

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

[sigurds, 2015-11-20 10:24:04]

Description was changed from

==========
[turbofan] Fix argument allocation dangling effect chains

Argument allocation in typed lowering was producing 
dangling effect chains. This patch fixes three sources
of dangling effect chains. 

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

to

==========
[turbofan] Fix argument allocation dangling effect chains

Argument allocation in typed lowering was producing 
dangling effect chains. This patch fixes three sources
of dangling effect chains.

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

[sigurds, 2015-11-20 10:24:39]

Addressed your comments.

https://codereview.chromium.org/1447323005/diff/40001/src/compiler/js-typed-l...
File src/compiler/js-typed-lowering.cc (right):

https://codereview.chromium.org/1447323005/diff/40001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:1489: AllocationBuilder a(jsgraph(),
allocate_effect, control);
On 2015/11/20 09:15:56, Michael Starzinger wrote:
> Instead of having the out-parameter passing, could we just pick the correct
> effect dependency here with ...
> 
> effect = elements->op()->EffectOutputCount() > 0 ? elements : effect;

Done.

https://codereview.chromium.org/1447323005/diff/40001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:1533: AllocationBuilder a(jsgraph(),
allocate_effect, control);
On 2015/11/20 09:15:56, Michael Starzinger wrote:
> Likewise.

Done.

[Michael Starzinger, 2015-11-20 10:47:37]

LGTM.

[sigurds, 2015-11-20 10:48:47]

The CQ bit was checked by sigurds@chromium.org

[commit-bot: I haz the power, 2015-11-20 10:48:52]

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

[commit-bot: I haz the power, 2015-11-20 10:50:18]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-11-20 10:50:33]

Patchset 3 (id:??) landed as
https://crrev.com/0ec6db4750190014d1442a6929808d32ed281f9e
Cr-Commit-Position: refs/heads/master@{#32140}