Issue 12082074: Fix another bailout problem where not all blocks were visiting when propagating bailout information…

Issue 12082074: Fix another bailout problem where not all blocks were visiting when propagating bailout information… (Closed)

Created:
7 years, 10 months ago by ngeoffray

Modified:
7 years, 10 months ago

Reviewers:
Lasse Reichstein Nielsen, kasperl

CC:
reviews_dartlang.org, erikcorry

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Fix another bailout problem where not all blocks were visiting when propagating bailout information to blocks, and fix a braino where we were not getting the right block for the loop header of a do/while loop. Committed: https://code.google.com/p/dart/source/detail?r=17887

Patch Set 1 : #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Total comments: 4

Patch Set 6 : #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+201 lines, -54 lines)			Patch
M	sdk/lib/_internal/compiler/implementation/ssa/bailout.dart	View	1 2 3 4 5	5 chunks	+90 lines, -47 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/ssa/codegen.dart	View	1 2 3 4 5	3 chunks	+8 lines, -7 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/ssa/nodes.dart	View	1 2 3 4 5	2 chunks	+19 lines, -0 lines	0 comments	Download
A	tests/language/bailout6_test.dart	View		1 chunk	+38 lines, -0 lines	0 comments	Download
A	tests/language/bailout7_test.dart	View	1	1 chunk	+46 lines, -0 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

FYI, I plan in a following CL to make sure all blocks get visited in ...

7 years, 10 months ago (2013-01-30 12:20:12 UTC) #1

Lasse Reichstein Nielsen

Looks OK to me, but can't remember enough details to catch any non-obvious bugs. https://codereview.chromium.org/12082074/diff/9002/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart ...

7 years, 10 months ago (2013-01-30 14:00:43 UTC) #2

ngeoffray

https://codereview.chromium.org/12082074/diff/9002/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right): https://codereview.chromium.org/12082074/diff/9002/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart#newcode482 sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:482: return false; On 2013/01/30 14:00:43, Lasse Reichstein Nielsen wrote: ...

7 years, 10 months ago (2013-01-30 14:19:34 UTC) #3

kasperl

LGTM, but this is tricky stuff so a few more comments would be helpful. https://codereview.chromium.org/12082074/diff/15001/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart ...

7 years, 10 months ago (2013-01-31 01:50:06 UTC) #4

ngeoffray

7 years, 10 months ago (2013-01-31 08:17:24 UTC) #5

Thanks Kasper and Lasse.

https://codereview.chromium.org/12082074/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/bailout.dart (right):

https://codereview.chromium.org/12082074/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:451: bool
handleBlockFlow(HBlockFlow blockFlow) {
On 2013/01/31 01:50:06, kasperl wrote:
> This method deserves a comment. 

Done.

https://codereview.chromium.org/12082074/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/bailout.dart:513: void
visitStatements(HSubGraphBlockInformation info, {bool newFlow}) {
On 2013/01/31 01:50:06, kasperl wrote:
> Add a comment that explains how this works and what the semantics of 'newFlow'
> are.

Done.

Expand Messages | Collapse Messages