[turbofan] Remove unused diamonds during control reduction.

[turbofan] Remove unused diamonds during control reduction.

A diamond is unused if the Merge node has no Phi/EffectPhi uses, exactly
two inputs, one IfTrue and one IfFalse, which have the same Branch
control input and no other uses except for the Merge. In this case the
diamond can safely be removed.

R=jarin@chromium.org

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

[Benedikt Meurer, 2015-03-12 07:30:45]

PTAL

[Jarin, 2015-03-12 08:45:05]

lgtm

[Benedikt Meurer, 2015-03-12 08:50:27]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2015-03-12 08:50:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1000883003/1

[commit-bot: I haz the power, 2015-03-12 09:02:13]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-03-12 09:02:34]

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

[brucedawson, 2015-03-13 18:38:20]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2015-03-13 18:38:21]

I opened a bug for this issue: crbug/467099

Also, but all-purpose bug for /analyze is 427616 so any fix should be annotated
with that bug also.

https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reduce...
File src/compiler/control-reducer.cc (right):

https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reduce...
src/compiler/control-reducer.cc:551: node1->opcode() == IrOpcode::kIfFalse)) &&
This test looks wrong. This showed up as a new VC++ /analyze warning:

src\v8\src\objects.cc(2128) : warning C6237: (<zero> && <expression>) is always
zero.  <expression> is never evaluated and might have side effects.

I'm not sure what the  correct check is -- presumably either testing for both
being kIfTrue or both being kIfFalse, or else checking for node0/1 being
kIfTrue/kIfFalse or node0/1 being kIfFalse/kIfTrue. Or maybe something else.

[Jarin, 2015-03-14 19:17:34]

https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reduce...
File src/compiler/control-reducer.cc (right):

https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reduce...
src/compiler/control-reducer.cc:551: node1->opcode() == IrOpcode::kIfFalse)) &&
On 2015/03/13 18:38:21, brucedawson wrote:
> This test looks wrong. This showed up as a new VC++ /analyze warning:
> 
> src\v8\src\objects.cc(2128) : warning C6237: (<zero> && <expression>) is
always
> zero.  <expression> is never evaluated and might have side effects.
> 
> I'm not sure what the  correct check is -- presumably either testing for both
> being kIfTrue or both being kIfFalse, or else checking for node0/1 being
> kIfTrue/kIfFalse or node0/1 being kIfFalse/kIfTrue. Or maybe something else.

Good catch. This should test that node0/1 is kIfTrue/kIfFalse or
kIfFalse/kIfTrue.

[Benedikt Meurer, 2015-03-16 06:24:18]

On 2015/03/14 19:17:34, jarin wrote:
>
https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reduce...
> File src/compiler/control-reducer.cc (right):
> 
>
https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reduce...
> src/compiler/control-reducer.cc:551: node1->opcode() == IrOpcode::kIfFalse))
&&
> On 2015/03/13 18:38:21, brucedawson wrote:
> > This test looks wrong. This showed up as a new VC++ /analyze warning:
> > 
> > src\v8\src\objects.cc(2128) : warning C6237: (<zero> && <expression>) is
> always
> > zero.  <expression> is never evaluated and might have side effects.
> > 
> > I'm not sure what the  correct check is -- presumably either testing for
both
> > being kIfTrue or both being kIfFalse, or else checking for node0/1 being
> > kIfTrue/kIfFalse or node0/1 being kIfFalse/kIfTrue. Or maybe something else.
> 
> Good catch. This should test that node0/1 is kIfTrue/kIfFalse or
> kIfFalse/kIfTrue.

Indeed, great catch. Thanks.