Skip to content

Commit 230f48d

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[vm,aot] Simple unreachable code elimination before type-flow analysis, take 3
This is a reland of 56991ad Original change's description: > [vm,aot] Simple unreachable code elimination before type-flow analysis, take 2 > > This is the relanding of https://dart-review.googlesource.com/c/sdk/+/121500 > with fixes. > > Original CL description: > > This change adds transformation for very early cleanup of unreachable > code such as code guarded by if statements with constant conditions or > code used in assert statements when assertions are disabled. > > The advantage of cleaning such code early is that type-flow analysis > won't be looking at it and TFA-based tree shaker is able to remove > more code. > > flutter_gallery_total_size -0.5663% (arm), -0.5409% (arm64) > build_bench_total_size -2.533% (arm), -2.449% (arm64) > gesture_detector_total_size -4.183% (arm), -4.072% (arm64) > > Change-Id: Ief8ebd0cd828c0e7a847a824f44d2d97a3595b87 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121901 > Reviewed-by: Ryan Macnak <rmacnak@google.com> > Reviewed-by: Martin Kustermann <kustermann@google.com> > Commit-Queue: Alexander Markov <alexmarkov@google.com> Change-Id: I82ea0f70845851f85ec159c366f408aa7f9e5788 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/122166 Reviewed-by: Martin Kustermann <kustermann@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
1 parent e8fa489 commit 230f48d

File tree

7 files changed

+441
-2
lines changed

7 files changed

+441
-2
lines changed

pkg/frontend_server/lib/frontend_server.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ class FrontendCompiler implements CompilerInterface {
435435
aot: options['aot'],
436436
useGlobalTypeFlowAnalysis: options['tfa'],
437437
environmentDefines: environmentDefines,
438+
enableAsserts: options['enable-asserts'],
438439
useProtobufTreeShaker: options['protobuf-tree-shaker']));
439440
}
440441
if (results.component != null) {

pkg/vm/lib/kernel_front_end.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ import 'transformations/type_flow/transformer.dart' as globalTypeFlow
5959
import 'transformations/obfuscation_prohibitions_annotator.dart'
6060
as obfuscationProhibitions;
6161
import 'transformations/call_site_annotator.dart' as call_site_annotator;
62+
import 'transformations/unreachable_code_elimination.dart'
63+
as unreachable_code_elimination;
6264

6365
/// Declare options consumed by [runCompiler].
6466
void declareCompilerOptions(ArgParser args) {
@@ -217,6 +219,7 @@ Future<int> runCompiler(ArgResults options, String usage) async {
217219
aot: aot,
218220
useGlobalTypeFlowAnalysis: tfa,
219221
environmentDefines: environmentDefines,
222+
enableAsserts: enableAsserts,
220223
genBytecode: genBytecode,
221224
bytecodeOptions: bytecodeOptions,
222225
dropAST: dropAST && !splitOutputByPackages,
@@ -285,6 +288,7 @@ Future<KernelCompilationResults> compileToKernel(
285288
{bool aot: false,
286289
bool useGlobalTypeFlowAnalysis: false,
287290
Map<String, String> environmentDefines,
291+
bool enableAsserts: true,
288292
bool genBytecode: false,
289293
BytecodeOptions bytecodeOptions,
290294
bool dropAST: false,
@@ -308,6 +312,7 @@ Future<KernelCompilationResults> compileToKernel(
308312
component,
309313
useGlobalTypeFlowAnalysis,
310314
environmentDefines,
315+
enableAsserts,
311316
useProtobufTreeShaker,
312317
errorDetector);
313318
}
@@ -362,6 +367,7 @@ Future _runGlobalTransformations(
362367
Component component,
363368
bool useGlobalTypeFlowAnalysis,
364369
Map<String, String> environmentDefines,
370+
bool enableAsserts,
365371
bool useProtobufTreeShaker,
366372
ErrorDetector errorDetector) async {
367373
if (errorDetector.hasCompilationErrors) return;
@@ -376,6 +382,10 @@ Future _runGlobalTransformations(
376382
// when building a platform dill file for VM/JIT case.
377383
mixin_deduplication.transformComponent(component);
378384

385+
// Unreachable code elimination transformation should be performed
386+
// before type flow analysis so TFA won't take unreachable code into account.
387+
unreachable_code_elimination.transformComponent(component, enableAsserts);
388+
379389
if (useGlobalTypeFlowAnalysis) {
380390
globalTypeFlow.transformComponent(
381391
compilerOptions.target, coreTypes, component);
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Copyright (c) 2019, 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+
import 'package:kernel/ast.dart';
6+
7+
/// Simple unreachable code elimination: removes asserts and if statements
8+
/// with constant conditions. Does a very limited constant folding of
9+
/// logical expressions.
10+
Component transformComponent(Component component, bool enableAsserts) {
11+
new SimpleUnreachableCodeElimination(enableAsserts).visitComponent(component);
12+
return component;
13+
}
14+
15+
class SimpleUnreachableCodeElimination extends Transformer {
16+
final bool enableAsserts;
17+
18+
SimpleUnreachableCodeElimination(this.enableAsserts);
19+
20+
bool _isBoolConstant(Expression node) =>
21+
node is BoolLiteral ||
22+
(node is ConstantExpression && node.constant is BoolConstant);
23+
24+
bool _getBoolConstantValue(Expression node) {
25+
if (node is BoolLiteral) {
26+
return node.value;
27+
}
28+
if (node is ConstantExpression) {
29+
final constant = node.constant;
30+
if (constant is BoolConstant) {
31+
return constant.value;
32+
}
33+
}
34+
throw 'Expected bool constant: $node';
35+
}
36+
37+
Expression _createBoolLiteral(bool value, int fileOffset) =>
38+
new BoolLiteral(value)..fileOffset = fileOffset;
39+
40+
Statement _makeEmptyBlockIfNull(Statement node) =>
41+
node == null ? Block(<Statement>[]) : node;
42+
43+
@override
44+
TreeNode visitIfStatement(IfStatement node) {
45+
node.transformChildren(this);
46+
final condition = node.condition;
47+
if (_isBoolConstant(condition)) {
48+
final value = _getBoolConstantValue(condition);
49+
return value ? node.then : node.otherwise;
50+
}
51+
node.then = _makeEmptyBlockIfNull(node.then);
52+
return node;
53+
}
54+
55+
@override
56+
visitConditionalExpression(ConditionalExpression node) {
57+
node.transformChildren(this);
58+
final condition = node.condition;
59+
if (_isBoolConstant(condition)) {
60+
final value = _getBoolConstantValue(condition);
61+
return value ? node.then : node.otherwise;
62+
}
63+
return node;
64+
}
65+
66+
@override
67+
TreeNode visitNot(Not node) {
68+
node.transformChildren(this);
69+
final operand = node.operand;
70+
if (_isBoolConstant(operand)) {
71+
return _createBoolLiteral(
72+
!_getBoolConstantValue(operand), node.fileOffset);
73+
}
74+
return node;
75+
}
76+
77+
@override
78+
TreeNode visitLogicalExpression(LogicalExpression node) {
79+
node.transformChildren(this);
80+
final left = node.left;
81+
final right = node.right;
82+
final operator = node.operator;
83+
if (_isBoolConstant(left)) {
84+
final leftValue = _getBoolConstantValue(left);
85+
if (_isBoolConstant(right)) {
86+
final rightValue = _getBoolConstantValue(right);
87+
if (operator == '||') {
88+
return _createBoolLiteral(leftValue || rightValue, node.fileOffset);
89+
} else if (operator == '&&') {
90+
return _createBoolLiteral(leftValue && rightValue, node.fileOffset);
91+
} else {
92+
throw 'Unexpected LogicalExpression operator ${operator}: $node';
93+
}
94+
} else {
95+
if (leftValue && operator == '||') {
96+
return _createBoolLiteral(true, node.fileOffset);
97+
} else if (!leftValue && operator == '&&') {
98+
return _createBoolLiteral(false, node.fileOffset);
99+
}
100+
}
101+
}
102+
return node;
103+
}
104+
105+
@override
106+
visitStaticGet(StaticGet node) {
107+
node.transformChildren(this);
108+
final target = node.target;
109+
if (target is Field && target.isConst) {
110+
throw 'StaticGet from const field $target should be evaluated by front-end: $node';
111+
}
112+
return node;
113+
}
114+
115+
@override
116+
TreeNode visitAssertStatement(AssertStatement node) {
117+
if (!enableAsserts) {
118+
return null;
119+
}
120+
return super.visitAssertStatement(node);
121+
}
122+
123+
@override
124+
TreeNode visitAssertBlock(AssertBlock node) {
125+
if (!enableAsserts) {
126+
return null;
127+
}
128+
return super.visitAssertBlock(node);
129+
}
130+
131+
@override
132+
TreeNode visitAssertInitializer(AssertInitializer node) {
133+
if (!enableAsserts) {
134+
return null;
135+
}
136+
return super.visitAssertInitializer(node);
137+
}
138+
139+
// Make sure we're not generating `null` bodies.
140+
// Try/catch, try/finally and switch/case statements
141+
// always have a Block in a body, so there is no
142+
// need to guard against null.
143+
144+
@override
145+
TreeNode visitWhileStatement(WhileStatement node) {
146+
node.transformChildren(this);
147+
node.body = _makeEmptyBlockIfNull(node.body);
148+
return node;
149+
}
150+
151+
@override
152+
TreeNode visitDoStatement(DoStatement node) {
153+
node.transformChildren(this);
154+
node.body = _makeEmptyBlockIfNull(node.body);
155+
return node;
156+
}
157+
158+
@override
159+
TreeNode visitForStatement(ForStatement node) {
160+
node.transformChildren(this);
161+
node.body = _makeEmptyBlockIfNull(node.body);
162+
return node;
163+
}
164+
165+
@override
166+
TreeNode visitForInStatement(ForInStatement node) {
167+
node.transformChildren(this);
168+
node.body = _makeEmptyBlockIfNull(node.body);
169+
return node;
170+
}
171+
}

pkg/vm/test/common_test_utils.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,18 @@ class TestingVmTarget extends VmTarget {
3333
}
3434

3535
Future<Component> compileTestCaseToKernelProgram(Uri sourceUri,
36-
{Target target, bool enableSuperMixins: false}) async {
36+
{Target target,
37+
bool enableSuperMixins = false,
38+
Map<String, String> environmentDefines}) async {
3739
final platformKernel =
3840
computePlatformBinariesLocation().resolve('vm_platform_strong.dill');
3941
target ??= new TestingVmTarget(new TargetFlags())
4042
..enableSuperMixins = enableSuperMixins;
43+
environmentDefines ??= <String, String>{};
4144
final options = new CompilerOptions()
4245
..target = target
4346
..linkedDependencies = <Uri>[platformKernel]
44-
..environmentDefines = <String, String>{}
47+
..environmentDefines = environmentDefines
4548
..onDiagnostic = (DiagnosticMessage message) {
4649
fail("Compilation error: ${message.plainTextFormatted.join('\n')}");
4750
};
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) 2019, 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+
import 'dart:io';
6+
7+
import 'package:kernel/target/targets.dart';
8+
import 'package:kernel/ast.dart';
9+
import 'package:kernel/kernel.dart';
10+
import 'package:test/test.dart';
11+
import 'package:vm/transformations/unreachable_code_elimination.dart'
12+
show transformComponent;
13+
14+
import '../common_test_utils.dart';
15+
16+
final String pkgVmDir = Platform.script.resolve('../..').toFilePath();
17+
18+
runTestCase(Uri source) async {
19+
final target = new TestingVmTarget(new TargetFlags());
20+
Component component = await compileTestCaseToKernelProgram(source,
21+
target: target,
22+
environmentDefines: {
23+
'test.define.isTrue': 'true',
24+
'test.define.isFalse': 'false'
25+
});
26+
27+
component = transformComponent(component, /* enableAsserts = */ false);
28+
29+
final actual = kernelLibraryToString(component.mainMethod.enclosingLibrary);
30+
31+
compareResultWithExpectationsFile(source, actual);
32+
}
33+
34+
main() {
35+
group('unreachable-code-elimination', () {
36+
final testCasesDir = new Directory(
37+
pkgVmDir + '/testcases/transformations/unreachable_code_elimination');
38+
39+
for (var entry in testCasesDir
40+
.listSync(recursive: true, followLinks: false)
41+
.reversed) {
42+
if (entry.path.endsWith(".dart")) {
43+
test(entry.path, () => runTestCase(entry.uri));
44+
}
45+
}
46+
});
47+
}

0 commit comments

Comments
 (0)