Skip to content

Commit 4093bda

Browse files
aamCommit Queue
authored andcommitted
[vm] Change try-catch IL representation.
Previously catch blocks were hanging off the function entry blocks being effectively an alternative entry-point into the function. Now catch blocks are hanging off the try-entry blocks. Previously all the variables that are used in the catch block and beyond were declared as parameters to catch block. Now only those that have their definitions not dominating catch entry will become parameters (for example, if a variable is assigned in the try-block, it becomes a parameter to catch block). During OSR, if OSR target entry point is inside some try-blocks, then all of corresponding try-entry/catch-blocks are pulled up to the OSR entry forming a chain that ends with a jump to the OSR target entry. TEST=vm/dart/trycatch*, ci Change-Id: Iae20c6548d5d65c63be6d7a53c1b0d2adac7ac31 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356311 Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Slava Egorov <[email protected]> Reviewed-by: Alexander Markov <[email protected]>
1 parent b3ab7e2 commit 4093bda

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+1549
-497
lines changed

pkg/vm/lib/testing/il_matchers.dart

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,14 @@ class PrettyPrinter {
228228
}
229229
buffer.write('}');
230230
}
231+
if (block['o'] == 'TryEntry') {
232+
if (block['tryBody'] != null) {
233+
buffer.write(' tryBody: B${block["tryBody"]}');
234+
}
235+
if (block['catches'] != null) {
236+
buffer.write(' catches: B${block["catches"]}');
237+
}
238+
}
231239
buffer.writeln();
232240
for (final instr in block['is'] ?? []) {
233241
buffer.write(' ');
@@ -485,6 +493,31 @@ class _BlockMatcher implements Matcher {
485493
}
486494
}
487495

496+
class _TryBlockMatcher implements Matcher {
497+
final String? tryBody;
498+
final String? catches;
499+
_TryBlockMatcher(this.tryBody, this.catches);
500+
501+
@override
502+
MatchStatus match(Env e, covariant Map<String, dynamic> block) {
503+
if (block['o'] != 'TryEntry') {
504+
return MatchStatus.fail('Expected TryEntry block, got ${block['o']} '
505+
'when matching B${block['b']}');
506+
}
507+
if (tryBody != null && "B${block['tryBody']}" != tryBody) {
508+
return MatchStatus.fail(
509+
'Expected tryBody block $tryBody , got B${block['tryBody']} '
510+
'when matching B${block['b']}');
511+
}
512+
if (catches != null && "B${block['catches']}" != catches) {
513+
return MatchStatus.fail(
514+
'Expected catches block $catches , got B${block['catches']} '
515+
'when matching B${block['b']}');
516+
}
517+
return MatchStatus.matched;
518+
}
519+
}
520+
488521
/// A matcher for instruction's named attributes.
489522
///
490523
/// Attributes are resolved to their indices through [Env.descriptors].
@@ -673,6 +706,10 @@ class Matchers {
673706
return _BlockMatcher(kind, List<ElementMatcher>.from(body));
674707
}
675708

709+
_TryBlockMatcher tryBlock({String? tryBody, String? catches}) {
710+
return _TryBlockMatcher(tryBody, catches);
711+
}
712+
676713
final _AnyMatcher any = const _AnyMatcher();
677714

678715
// ignore: non_constant_identifier_names

runtime/tests/vm/dart/block_ordering_il_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ void matchIL$bodyAlwaysThrows(FlowGraph graph) {
138138
graph.match([
139139
match.block('Graph'),
140140
match.block('Function'),
141+
match.tryBlock(),
141142
match.block('CatchBlock'),
142143
match.block('Join'),
143144
], inCodegenBlockOrder: true);

runtime/tests/vm/dart/dynamic_module_pragmas_il_test.dart

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -265,21 +265,21 @@ void matchIL$testCallInTryWithControlFlow(FlowGraph graph) {
265265
'pos' << match.Parameter(index: 1),
266266
match.CheckStackOverflow(),
267267
'cid1' << match.LoadClassId('value'),
268-
match.Branch(match.TestRange('cid1'), ifTrue: 'B9', ifFalse: 'B10'),
268+
match.Branch(match.TestRange('cid1'), ifTrue: 'B10', ifFalse: 'B11'),
269269
]),
270-
'B9' <<
270+
'B10' <<
271271
match.block('Target', [
272272
match.MoveArgument('value'),
273273
'value_length1' << match.DispatchTableCall('cid1'),
274-
match.Goto('B11'),
274+
match.Goto('B12'),
275275
]),
276-
'B10' <<
276+
'B11' <<
277277
match.block('Target', [
278278
match.MoveArgument('value'),
279279
'value_length2' << match.InstanceCall('value'),
280-
match.Goto('B11'),
280+
match.Goto('B12'),
281281
]),
282-
'B11' <<
282+
'B12' <<
283283
match.block('Join', [
284284
'value_length' << match.Phi('value_length1', 'value_length2'),
285285
'value_length_unboxed' << match.UnboxInt64('value_length'),
@@ -290,49 +290,50 @@ void matchIL$testCallInTryWithControlFlow(FlowGraph graph) {
290290
]),
291291
'B3' <<
292292
match.block('Target', [
293-
match.Goto('B8'),
293+
match.Goto('B9'),
294294
]),
295295
'B4' <<
296296
match.block('Target', [
297297
match.Goto('B5'),
298298
]),
299-
'B5' <<
299+
'B5' << match.tryBlock(tryBody: 'B6', catches: 'B8'),
300+
'B6' <<
300301
match.block('Join', [
301302
'pos_boxed' << match.BoxInt64('pos'),
302303
'cid2' << match.LoadClassId('value'),
303-
match.Branch(match.TestRange('cid2'), ifTrue: 'B12', ifFalse: 'B13'),
304+
match.Branch(match.TestRange('cid2'), ifTrue: 'B13', ifFalse: 'B14'),
304305
]),
305-
'B12' <<
306+
'B13' <<
306307
match.block('Target', [
307308
match.MoveArgument('value'),
308309
match.MoveArgument('pos_boxed'),
309310
'value_substring1' << match.DispatchTableCall('cid2'),
310-
match.Goto('B14'),
311+
match.Goto('B15'),
311312
]),
312-
'B13' <<
313+
'B14' <<
313314
match.block('Target', [
314315
match.MoveArgument('value'),
315316
match.MoveArgument('pos_boxed'),
316317
'value_substring2' << match.InstanceCall('value', 'pos_boxed'),
317-
match.Goto('B14'),
318+
match.Goto('B15'),
318319
]),
319-
'B14' <<
320+
'B15' <<
320321
match.block('Join', [
321322
'value_substring' <<
322323
match.Phi('value_substring1', 'value_substring2'),
323324
match.MoveArgument('value_substring'),
324325
match.StaticCall('value_substring'),
325-
match.Goto('B6'),
326+
match.Goto('B7'),
326327
]),
327-
'B7' <<
328+
'B8' <<
328329
match.block('CatchBlock', [
329-
match.Goto('B6'),
330+
match.Goto('B7'),
330331
]),
331-
'B6' <<
332+
'B7' <<
332333
match.block('Join', [
333-
match.Goto('B8'),
334+
match.Goto('B9'),
334335
]),
335-
'B8' <<
336+
'B9' <<
336337
match.block('Join', [
337338
match.DartReturn(match.any),
338339
]),

runtime/tests/vm/dart/eliminate_allocations_il_test.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,15 @@ void matchIL$bar(FlowGraph graph) {
4545
match.block('Function', [
4646
match.Goto('B3', skipUntilMatched: false),
4747
]),
48-
'B3' <<
48+
'B3' << match.tryBlock(tryBody: 'B4', catches: 'B7'),
49+
'B4' <<
4950
match.block('Join', [
50-
match.Goto('B5', skipUntilMatched: false),
51+
match.Goto('B6', skipUntilMatched: false),
5152
]),
52-
'B5' <<
53+
'B6' <<
5354
match.block('Join', [
5455
match.DartReturn('c_42'),
5556
]),
56-
'B6' << match.block('CatchBlock'),
57+
'B7' << match.block('CatchBlock'),
5758
]);
5859
}

runtime/tests/vm/dart/hash_map_probes_limit_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// This test verifies that a relatively large program (kernel compiler)
66
// can be compiled with --hash_map_probes_limit.
77

8-
// VMOptions=--hash_map_probes_limit=1000
8+
// VMOptions=--hash_map_probes_limit=2000
99

1010
import "package:vm/kernel_front_end.dart";
1111

runtime/tests/vm/dart/records_return_value_unboxing_il_test.dart

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,25 +179,26 @@ void matchIL$testUnboxedRecordInTryCatch(FlowGraph graph) {
179179
match.block('Graph'),
180180
match.block('Function', [
181181
match.CheckStackOverflow(),
182-
match.Goto('B1'),
182+
match.Goto('B3'),
183183
]),
184-
'B1' <<
184+
'B3' << match.tryBlock(tryBody: 'B4', catches: 'B6'),
185+
'B4' <<
185186
match.block('Join', [
186187
'v1' << match.StaticCall(),
187188
'v1_a' << match.ExtractNthOutput('v1', index: 0),
188189
match.MoveArgument('v1_a'),
189190
match.StaticCall(),
190-
match.Goto('B3'),
191+
match.Goto('B5'),
191192
]),
192-
'B2' <<
193+
'B6' <<
193194
match.block('CatchBlock', [
194195
'e' << match.Parameter(index: 2),
195196
'st' << match.Parameter(index: 3),
196197
match.MoveArgument('e'),
197198
match.StaticCall(),
198-
match.Goto('B3'),
199+
match.Goto('B5'),
199200
]),
200-
'B3' <<
201+
'B5' <<
201202
match.block('Join', [
202203
match.DartReturn(),
203204
]),

runtime/tests/vm/dart/regress_flutter110715_il_test.dart

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,24 @@ void main() async {
4444
void matchIL$bug1(FlowGraph graph) {
4545
graph.dump();
4646
graph.match([
47-
match.block('Graph'),
47+
match.block('Graph', ['cnull' << match.Constant(value: null)]),
4848
match.block('Function'),
49+
match.block('Try'),
4950
match.block('Join'),
5051
match.block('CatchBlock', [
51-
'v0' << match.Parameter(),
52-
match.Branch(match.StrictCompare('v0', match.any, kind: '==='),
53-
ifTrue: 'B6', ifFalse: 'B7'),
52+
match.Branch(match.StrictCompare(match.any, 'cnull', kind: '==='),
53+
ifTrue: 'B7', ifFalse: 'B8'),
5454
]),
55-
'B6' <<
55+
'B7' <<
5656
match.block('Target', [
57-
match.Goto('B4'),
57+
match.Goto('B5'),
5858
]),
59-
'B7' <<
59+
'B8' <<
6060
match.block('Target', [
6161
match.ClosureCall(),
62-
match.Goto('B4'),
62+
match.Goto('B5'),
6363
]),
64-
'B4' <<
64+
'B5' <<
6565
match.block('Join', [
6666
match.DartReturn(),
6767
]),
@@ -71,24 +71,24 @@ void matchIL$bug1(FlowGraph graph) {
7171
void matchIL$bug2(FlowGraph graph) {
7272
graph.dump();
7373
graph.match([
74-
match.block('Graph'),
74+
match.block('Graph', ['cnull' << match.Constant(value: null)]),
7575
match.block('Function'),
76+
match.tryBlock(),
7677
match.block('Join'),
7778
match.block('CatchBlock', [
78-
'v0' << match.Parameter(),
79-
match.Branch(match.StrictCompare('v0', match.any, kind: '==='),
80-
ifTrue: 'B6', ifFalse: 'B7'),
79+
match.Branch(match.StrictCompare(match.any, 'cnull', kind: '==='),
80+
ifTrue: 'B7', ifFalse: 'B8'),
8181
]),
82-
'B6' <<
82+
'B7' <<
8383
match.block('Target', [
84-
match.Goto('B4'),
84+
match.Goto('B5'),
8585
]),
86-
'B7' <<
86+
'B8' <<
8787
match.block('Target', [
8888
match.ClosureCall(),
89-
match.Goto('B4'),
89+
match.Goto('B5'),
9090
]),
91-
'B4' <<
91+
'B5' <<
9292
match.block('Join', [
9393
match.DartReturn(),
9494
]),
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
//
5+
// Test that aliasing of thrown exception is handled correctly.
6+
//
7+
8+
import 'package:expect/expect.dart';
9+
10+
class X {
11+
int v = 0;
12+
}
13+
14+
class Y {
15+
final x = X();
16+
}
17+
18+
void foo() {
19+
var y = Y();
20+
try {
21+
throw y.x;
22+
} on X catch (e) {
23+
e.v = 10; // e is an alias of y.x
24+
}
25+
Expect.equals(10, y.x.v); // should be 10 not 0.
26+
}
27+
28+
void bar() {
29+
var o = X();
30+
try {
31+
throw o;
32+
} on X catch (e) {
33+
e.v = 10; // e is an alias of o
34+
}
35+
Expect.equals(10, o.v);
36+
}
37+
38+
main() {
39+
foo();
40+
bar();
41+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
//
6+
// Ensure that aliasing calculation takes try-catch into account.
7+
// Otherwise, optimizations like dead store elimination can produce
8+
// incorrect results.
9+
//
10+
11+
import 'package:expect/expect.dart';
12+
13+
class C {
14+
int v = 0;
15+
}
16+
17+
@pragma('vm:never-inline')
18+
void alwaysThrow() {
19+
throw 'a';
20+
}
21+
22+
void foo() {
23+
C? alias;
24+
final alloc = C();
25+
try {
26+
alias = alloc;
27+
alwaysThrow();
28+
alias = null;
29+
} catch (e) {}
30+
// here [alias] will be aliasing [alloc].
31+
// [alias] should be a `Phi(Constant(null), Parameter(...))`
32+
// where `Parameter` arrives from the catch entry.
33+
alias!.v = 42;
34+
// [alloc] should be referencing a AllocateObject
35+
// directly here.
36+
Expect.equals(42, alloc.v);
37+
}
38+
39+
void main() {
40+
foo();
41+
}

0 commit comments

Comments
 (0)