Issue 23063003: Fix issue 12023 by avoiding a critical edge in the SSA graph with a switch statement.

Issue 23063003: Fix issue 12023 by avoiding a critical edge in the SSA graph with a switch statement. (Closed)

Created:
7 years, 4 months ago by ngeoffray

Modified:
7 years, 4 months ago

Reviewers:
karlklose

CC:
reviews_dartlang.org, Lasse Reichstein Nielsen

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

Visibility:
Public.

More Reviews

Description

Fix issue 12023 by avoiding a critical edge in the SSA graph with a switch statement. R=karlklose@google.com Committed: https://code.google.com/p/dart/source/detail?r=26123

Patch Set 3 : #

Created: 7 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+50 lines, -19 lines)			Patch
M	sdk/lib/_internal/compiler/implementation/ssa/builder.dart	View	1 2	4 chunks	+24 lines, -14 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/ssa/codegen.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/ssa/nodes.dart	View		1 chunk	+0 lines, -4 lines	0 comments	Download
A	tests/language/issue12023_test.dart	View		1 chunk	+25 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

karlklose

lgtm LGTM. https://codereview.chromium.org/23063003/diff/2001/sdk/lib/_internal/compiler/implementation/ssa/builder.dart File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right): https://codereview.chromium.org/23063003/diff/2001/sdk/lib/_internal/compiler/implementation/ssa/builder.dart#newcode4745 sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4745: HBasicBlock avoidCriticalEdge = addNewBlock(); Rename variable to ...

7 years, 4 months ago (2013-08-14 14:36:43 UTC) #2

ngeoffray

Thanks Karl. PTAL, As discussed, I had to explicitly add a break to the last ...

7 years, 4 months ago (2013-08-14 14:57:56 UTC) #3

karlklose

SLGTM. https://codereview.chromium.org/23063003/diff/6001/sdk/lib/_internal/compiler/implementation/ssa/builder.dart File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right): https://codereview.chromium.org/23063003/diff/6001/sdk/lib/_internal/compiler/implementation/ssa/builder.dart#newcode4719 sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4719: // sure the last case does not fall ...

7 years, 4 months ago (2013-08-14 15:01:54 UTC) #4

ngeoffray

Thanks Karl. https://codereview.chromium.org/23063003/diff/6001/sdk/lib/_internal/compiler/implementation/ssa/builder.dart File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right): https://codereview.chromium.org/23063003/diff/6001/sdk/lib/_internal/compiler/implementation/ssa/builder.dart#newcode4719 sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4719: // sure the last case does not ...

7 years, 4 months ago (2013-08-14 15:03:53 UTC) #5

ngeoffray

7 years, 4 months ago (2013-08-14 15:13:39 UTC) #6

Message was sent while issue was closed.

Committed patchset #3 manually as r26123 (presubmit successful).

Expand Messages | Collapse Messages