Change labeled statement to use visitSubGraph for its body instead of marking its "exit block" spec…

Change labeled statement to use visitSubGraph for its body instead of marking its "exit block" specially.

Committed: https://code.google.com/p/dart/source/detail?r=5014

[ngeoffray, 2012-03-05 14:12:48]

LGTM!

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2261: handler.close();
Any reason why the close is between the creation and setter of the blockInfo?

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/codegen.dart
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/codegen.da...
frog/leg/ssa/codegen.dart:271: iterateBasicBlock(HBasicBlock node) {
Was this change (adding iterateBasicBlock) necessary? If not, I prefer keeping
one method for now.

[Lasse Reichstein Nielsen, 2012-03-05 14:16:49]

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2261: handler.close();
No. It just needs to be after handler.labels() is called.
I'll move it further down.

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/codegen.dart
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/1/frog/leg/ssa/codegen.da...
frog/leg/ssa/codegen.dart:271: iterateBasicBlock(HBasicBlock node) {
Sadly, yes, something is necessary, but I'm not entirely sure what.
visitBasicBlock calls handleLabeledBlock, which calls visitSubGraph, which calls
back to visitBlock.
To break this loop, I split the main visitBasicBlock into the part that
dispatches on the block annotation and the part that does the real work. But
thinking more about it, I'm not sure it's good enough. If a labeled statement is
a branch of an if, it'll fail. Back to the drawing board. :(

[floitsch, 2012-03-05 14:21:17]

Waiting for the corrected version, but so far looks good.

[Lasse Reichstein Nielsen, 2012-03-06 08:58:08]

Updated, please take a new look.

[floitsch, 2012-03-06 10:12:13]

LGTM.

https://chromiumcodereview.appspot.com/9601009/diff/1005/frog/leg/ssa/codegen...
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/1005/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:107: // Restriction on the block traversal.
That comment doesn't work for me. Maybe (if that's what it does): The [subGraph]
variable is used to delimit the traversal for some instructions, like Ifs.

[Lasse Reichstein Nielsen, 2012-03-06 10:20:43]

https://chromiumcodereview.appspot.com/9601009/diff/1005/frog/leg/ssa/codegen...
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/1005/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:107: // Restriction on the block traversal.
Reworded.

[ngeoffray, 2012-03-06 11:17:37]

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:2261: handler.close();
Move the close around?

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/codegen...
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:105: // Used to break bad recursion.
What's a bad recursion?

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:267: // don't handle it again.
How can you end up doing this? Should we re-design the subgraph to make sure we
never encounter this case?

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/tests/leg_only/...
File frog/tests/leg_only/src/BreakTest.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/tests/leg_only/...
frog/tests/leg_only/src/BreakTest.dart:122: }
indentation is still off :)

[Lasse Reichstein Nielsen, 2012-03-08 09:02:02]

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:2261: handler.close();
I did. On another computer. Bad sync!

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/codegen...
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:105: // Used to break bad recursion.
Infinite recursion, recursing on the same input again. Will reword.

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:267: // don't handle it again.
For a labeled expression, the "HLabeledBlockInformation" is put on the first
block of the body, which is also the first block of the subgraph.
Any suggestions for redesign are appreciated.

VisitBasicBlock is called by visitSubGraph, and the label/loop tests are
necessary for that (they can be branches of an if, which also uses
visitSubGraph).

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/tests/leg_only/...
File frog/tests/leg_only/src/BreakTest.dart (right):

https://chromiumcodereview.appspot.com/9601009/diff/7002/frog/tests/leg_only/...
frog/tests/leg_only/src/BreakTest.dart:122: }
That's actually deliberate, because I indent relative to the if. I should
probably indent relative to the foo: label.