Fix bad assumption of environment builder when it comes to nested loops: blocks inside a loop would…

Fix bad assumption of environment builder when it comes to nested loops: blocks inside a loop would not get the loop markers of its header. Instead, stack up loop markers when visiting the graph and pop them when being visited.
Committed: https://code.google.com/p/dart/source/detail?r=15164

[floitsch, 2012-11-20 15:39:31]

LGTM.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right):

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:57: String toString()
=> '${loopMarkers.length}';
Remove or make it at least mention "Environment".

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:253:
List<HBasicBlock> loopMarkers;
Add comment explaining what these contain.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:318: // If the block
is a loop header, we can remove the loop marker,
This sounds too much like an optimization.
In fact without this if the 'loopMarkers' list would never shrink.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:321:
loopMarkers.removeLast();
assert that it is the same as block?

[ngeoffray, 2012-11-20 16:40:37]

Thanks Florian.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right):

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:57: String toString()
=> '${loopMarkers.length}';
On 2012/11/20 15:39:31, floitsch wrote:
> Remove or make it at least mention "Environment".

Removed.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:253:
List<HBasicBlock> loopMarkers;
On 2012/11/20 15:39:31, floitsch wrote:
> Add comment explaining what these contain.

Done.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:318: // If the block
is a loop header, we can remove the loop marker,
On 2012/11/20 15:39:31, floitsch wrote:
> This sounds too much like an optimization.
> In fact without this if the 'loopMarkers' list would never shrink.

Improved comment.

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:321:
loopMarkers.removeLast();
On 2012/11/20 15:39:31, floitsch wrote:
> assert that it is the same as block?

Done.

[ngeoffray, 2012-11-20 17:30:45]

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right):

https://codereview.chromium.org/11411068/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:321:
loopMarkers.removeLast();
On 2012/11/20 16:40:37, ngeoffray wrote:
> On 2012/11/20 15:39:31, floitsch wrote:
> > assert that it is the same as block?
> 
> Done.

Actually, I had to change the data structure to be a set, because the last may
not be the current loop header in case of labeled loops.

[floitsch, 2012-11-20 17:31:32]

Still LGTM.