Skip to content

Commit 56991ad

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[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>
1 parent 03d6e04 commit 56991ad

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
@@ -441,6 +441,7 @@ class FrontendCompiler implements CompilerInterface {
441441
aot: options['aot'],
442442
useGlobalTypeFlowAnalysis: options['tfa'],
443443
environmentDefines: environmentDefines,
444+
enableAsserts: options['enable-asserts'],
444445
useProtobufTreeShaker: options['protobuf-tree-shaker']));
445446
}
446447
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)