Recognize nested logical operations.

Recognize nested logical operations.
Committed: https://code.google.com/p/dart/source/detail?r=8393

[floitsch, 2012-06-07 09:23:23]

DBC.

https://chromiumcodereview.appspot.com/10539042/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10539042/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:1728: // This method is only called
for phis that are generated at use
Move the comment before the first line of the function.

[ngeoffray, 2012-06-07 10:05:48]

https://chromiumcodereview.appspot.com/10539042/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10539042/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:1728: // This method is only called
for phis that are generated at use
On 2012/06/07 09:23:24, floitsch wrote:
> Move the comment before the first line of the function.

Done.

[Lasse Reichstein Nielsen, 2012-06-07 12:02:46]

LGTM

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
File lib/compiler/implementation/ssa/codegen_helpers.dart (right):

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:157: && instruction !==
block.last.previous) return true;
Putting "&&" at the end of the previous line seems more common. I actually like
this, so keep it :)

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:195: if (elseBlock.first
!== elseBlock.last) return;
Or "if (elseBlock.first is! HGoto) return;"?

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:200: && thenBlock.first ==
thenBlock.last
You could just check that thenBlock.first is HGoto.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:208: // have statements, we
can emit a sequence of logical operation.
"have a statement" to match the call to "hasStatement" below.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:219:
assert(thenInput.usedBy.length == 1);
Can't it also be used internally in the block?

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:227: // and the join branch
to have more than two predecessors.
Then check if that's the case and return if it isn't.
Before setting the generateAtUseSite bit.

[ngeoffray, 2012-06-07 12:39:18]

Thanks Lasse.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
File lib/compiler/implementation/ssa/codegen_helpers.dart (right):

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:195: if (elseBlock.first
!== elseBlock.last) return;
On 2012/06/07 12:02:47, Lasse Reichstein Nielsen wrote:
> Or "if (elseBlock.first is! HGoto) return;"?

Done.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:200: && thenBlock.first ==
thenBlock.last
On 2012/06/07 12:02:47, Lasse Reichstein Nielsen wrote:
> You could just check that thenBlock.first is HGoto.

Done.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:208: // have statements, we
can emit a sequence of logical operation.
On 2012/06/07 12:02:47, Lasse Reichstein Nielsen wrote:
> "have a statement" to match the call to "hasStatement" below.

Done.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:219:
assert(thenInput.usedBy.length == 1);
On 2012/06/07 12:02:47, Lasse Reichstein Nielsen wrote:
> Can't it also be used internally in the block?

No, because otherwise there would be an instruction in-between the instruction
and the control flow instruction, which I guard against in hasStatement. I added
a comment.

http://codereview.chromium.org/10539042/diff/4002/lib/compiler/implementation...
lib/compiler/implementation/ssa/codegen_helpers.dart:227: // and the join branch
to have more than two predecessors.
On 2012/06/07 12:02:47, Lasse Reichstein Nielsen wrote:
> Then check if that's the case and return if it isn't.
> Before setting the generateAtUseSite bit.

Added asserts instead.