[dart2js] Lower JS_GET_FLAG in phase 0b kernel transformer
Also adds `JS_FALSE` to replace uses of `JS_GET_FLAG(false)`. See the
doc comment for details.
Change-Id: I33f89f158c1955a19fa299f22c50d51e36ede3d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330171
Commit-Queue: Mayank Patke <fishythefish@google.com>
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
diff --git a/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart b/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart
new file mode 100644
index 0000000..a85842a
--- /dev/null
+++ b/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart
@@ -0,0 +1,80 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:kernel/ast.dart';
+import 'package:kernel/core_types.dart';
+
+import '../../../options.dart';
+
+/// An AST transformation which lowers invocations of `JS_GET_FLAG`.
+///
+/// `JS_GET_FLAG` is defined in `dart:_foreign_helper` and is invoked in runtime
+/// libraries as a way of accessing compiler options in runtime code. The return
+/// value is a de facto constant, so the compiler replaces the invocation by its
+/// result before codegen rather than performing an operation at runtime.
+///
+/// The earlier this lowering is performed, the more optimizations are enabled.
+/// Because `JS_GET_FLAG` invocations are typically used as conditions, SSA is
+/// able to simplify boolean expressions and eliminate dead branches guarded by
+/// `JS_GET_FLAG`. However, if we lower `JS_GET_FLAG` during SSA, the dead code
+/// will still previously have been treated as live. Using a kernel
+/// transformation early allows us to perform treeshaking of the now-dead
+/// references as well.
+///
+/// This transformation is a global transformation because it needs access to
+/// [CompilerOptions], which is not available modularly.
+class JsGetFlagLowering {
+ final CoreTypes _coreTypes;
+ final CompilerOptions _options;
+
+ JsGetFlagLowering(this._coreTypes, this._options);
+
+ TreeNode transformStaticInvocation(StaticInvocation node) {
+ if (node.target != _coreTypes.jsGetFlag) return node;
+ final argument = node.arguments.positional.single;
+
+ String? flag;
+ if (argument is StringLiteral) {
+ flag = argument.value;
+ } else if (argument is ConstantExpression) {
+ final constant = argument.constant;
+ if (constant is StringConstant) {
+ flag = constant.value;
+ }
+ }
+
+ if (flag == null) _unsupportedFlag(argument);
+ final flagValue = _getFlagValue(flag);
+ if (flagValue == null) _unsupportedFlag(flag);
+ return ConstantExpression(BoolConstant(flagValue))
+ ..fileOffset = node.fileOffset;
+ }
+
+ bool? _getFlagValue(String flagName) {
+ switch (flagName) {
+ case 'DEV_COMPILER':
+ return false;
+ case 'MINIFIED':
+ return _options.enableMinification;
+ case 'MUST_RETAIN_METADATA':
+ return false;
+ case 'USE_CONTENT_SECURITY_POLICY':
+ return _options.features.useContentSecurityPolicy.isEnabled;
+ case 'VARIANCE':
+ return _options.enableVariance;
+ case 'LEGACY':
+ return _options.useLegacySubtyping;
+ case 'EXTRA_NULL_SAFETY_CHECKS':
+ // TODO(fishythefish): Handle this flag as needed.
+ return false;
+ case 'PRINT_LEGACY_STARS':
+ return _options.printLegacyStars;
+ default:
+ return null;
+ }
+ }
+
+ Never _unsupportedFlag(Object? flag) =>
+ throw UnsupportedError('Unexpected JS_GET_FLAG argument: $flag');
+}
diff --git a/pkg/compiler/lib/src/kernel/transformations/global/lowering.dart b/pkg/compiler/lib/src/kernel/transformations/global/lowering.dart
index e2d3a68..7e4aa1b 100644
--- a/pkg/compiler/lib/src/kernel/transformations/global/lowering.dart
+++ b/pkg/compiler/lib/src/kernel/transformations/global/lowering.dart
@@ -3,19 +3,34 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:kernel/ast.dart';
+import 'package:kernel/core_types.dart';
+import '../../../options.dart';
import 'clone_mixin_methods_with_super.dart' as transformMixins;
+import 'js_get_flag_lowering.dart';
-void transformLibraries(List<Library> libraries) {
- final transformer = _Lowering();
+void transformLibraries(
+ List<Library> libraries, CoreTypes coreTypes, CompilerOptions options) {
+ final transformer = _Lowering(coreTypes, options);
libraries.forEach(transformer.visitLibrary);
}
class _Lowering extends Transformer {
+ final JsGetFlagLowering _jsGetFlagLowering;
+
+ _Lowering(CoreTypes coreTypes, CompilerOptions options)
+ : _jsGetFlagLowering = JsGetFlagLowering(coreTypes, options);
+
@override
Class visitClass(Class node) {
node.transformChildren(this);
transformMixins.transformClass(node);
return node;
}
+
+ @override
+ TreeNode visitStaticInvocation(StaticInvocation node) {
+ node.transformChildren(this);
+ return _jsGetFlagLowering.transformStaticInvocation(node);
+ }
}
diff --git a/pkg/compiler/lib/src/phase/load_kernel.dart b/pkg/compiler/lib/src/phase/load_kernel.dart
index 87abcb5..9d58953 100644
--- a/pkg/compiler/lib/src/phase/load_kernel.dart
+++ b/pkg/compiler/lib/src/phase/load_kernel.dart
@@ -124,12 +124,13 @@
// serialized components and components from source.
void _doTransformsOnKernelLoad(Component component, CompilerOptions options) {
if (options.stage.shouldRunGlobalTransforms) {
- globalTransforms.transformLibraries(component.libraries);
+ ir.CoreTypes coreTypes = ir.CoreTypes(component);
+ globalTransforms.transformLibraries(
+ component.libraries, coreTypes, options);
// referenceFromIndex is only necessary in the case where a module
// containing a stub definition is invalidated, and then reloaded, because
// we need to keep existing references to that stub valid. Here, we have the
// whole program, and therefore do not need it.
- ir.CoreTypes coreTypes = ir.CoreTypes(component);
StaticInteropClassEraser(coreTypes, null,
additionalCoreLibraries: {'_js_types', 'js_interop'})
.visitComponent(component);
diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart
index 122978e..c2c2186 100644
--- a/pkg/compiler/lib/src/ssa/builder.dart
+++ b/pkg/compiler/lib/src/ssa/builder.dart
@@ -376,39 +376,6 @@
open(newBlock);
}
- /// Helper to implement JS_GET_FLAG.
- ///
- /// The concrete SSA graph builder will extract a flag parameter from the
- /// JS_GET_FLAG call and then push a boolean result onto the stack. This
- /// function provides the boolean value corresponding to the given [flagName].
- /// If [flagName] is not recognized, this function returns `null` and the
- /// concrete SSA builder reports an error.
- bool? _getFlagValue(String flagName) {
- switch (flagName) {
- case 'FALSE':
- return false;
- case 'DEV_COMPILER':
- return false;
- case 'MINIFIED':
- return options.enableMinification;
- case 'MUST_RETAIN_METADATA':
- return false;
- case 'USE_CONTENT_SECURITY_POLICY':
- return options.features.useContentSecurityPolicy.isEnabled;
- case 'VARIANCE':
- return options.enableVariance;
- case 'LEGACY':
- return options.useLegacySubtyping;
- case 'EXTRA_NULL_SAFETY_CHECKS':
- // TODO(fishythefish): Handle this flag as needed.
- return false;
- case 'PRINT_LEGACY_STARS':
- return options.printLegacyStars;
- default:
- return null;
- }
- }
-
StaticType _getStaticType(ir.Expression node) {
// TODO(johnniwinther): Substitute the type by the this type and type
// arguments of the current frame.
@@ -4474,8 +4441,8 @@
_handleForeignJsEmbeddedGlobal(invocation);
} else if (name == 'JS_BUILTIN') {
_handleForeignJsBuiltin(invocation);
- } else if (name == 'JS_GET_FLAG') {
- _handleForeignJsGetFlag(invocation);
+ } else if (name == 'JS_FALSE') {
+ _handleForeignJsFalse(invocation);
} else if (name == 'JS_EFFECT') {
stack.add(graph.addConstantNull(closedWorld));
} else if (name == 'JS_INTERCEPTOR_CONSTANT') {
@@ -4987,24 +4954,9 @@
}
}
- void _handleForeignJsGetFlag(ir.StaticInvocation invocation) {
- if (_unexpectedForeignArguments(invocation,
- minPositional: 1, maxPositional: 1)) {
- stack.add(
- // Result expected on stack.
- graph.addConstantBool(false, closedWorld));
- return;
- }
- String name = _foreignConstantStringArgument(invocation, 0, 'JS_GET_FLAG')!;
- final value = _getFlagValue(name);
- if (value == null) {
- reporter.reportErrorMessage(
- _elementMap.getSpannable(targetElement, invocation),
- MessageKind.GENERIC,
- {'text': 'Error: Unknown internal flag "$name".'});
- } else {
- stack.add(graph.addConstantBool(value, closedWorld));
- }
+ void _handleForeignJsFalse(ir.StaticInvocation invocation) {
+ _unexpectedForeignArguments(invocation, minPositional: 0, maxPositional: 0);
+ stack.add(graph.addConstantBool(false, closedWorld));
}
void _handleJsInterceptorConstant(ir.StaticInvocation invocation) {
diff --git a/pkg/kernel/lib/core_types.dart b/pkg/kernel/lib/core_types.dart
index b8aad69..cecfdca 100644
--- a/pkg/kernel/lib/core_types.dart
+++ b/pkg/kernel/lib/core_types.dart
@@ -353,6 +353,9 @@
late final Field enumNameField =
index.getField('dart:core', '_Enum', '_name');
+ late final Procedure jsGetFlag =
+ index.getTopLevelProcedure('dart:_foreign_helper', 'JS_GET_FLAG');
+
InterfaceType get objectLegacyRawType {
return _objectLegacyRawType ??= _legacyRawTypes[objectClass] ??=
new InterfaceType(objectClass, Nullability.legacy, const <DartType>[]);
diff --git a/sdk/lib/_internal/js_runtime/lib/async_patch.dart b/sdk/lib/_internal/js_runtime/lib/async_patch.dart
index f68c1c7..a412c7b 100644
--- a/sdk/lib/_internal/js_runtime/lib/async_patch.dart
+++ b/sdk/lib/_internal/js_runtime/lib/async_patch.dart
@@ -15,7 +15,7 @@
unwrapException;
import 'dart:_foreign_helper'
- show JS, JS_GET_FLAG, JS_RAW_EXCEPTION, RAW_DART_FUNCTION_REF;
+ show JS, JS_FALSE, JS_RAW_EXCEPTION, RAW_DART_FUNCTION_REF;
import 'dart:_async_status_codes' as async_status_codes;
@@ -597,7 +597,7 @@
}
bool moveNext() {
- if (JS_GET_FLAG('FALSE')) _modelGeneratedCode();
+ if (JS_FALSE()) _modelGeneratedCode();
Object? errorValue;
int errorCode = async_status_codes.SUCCESS;
while (true) {
diff --git a/sdk/lib/_internal/js_runtime/lib/foreign_helper.dart b/sdk/lib/_internal/js_runtime/lib/foreign_helper.dart
index 549e485..c0bc40c 100644
--- a/sdk/lib/_internal/js_runtime/lib/foreign_helper.dart
+++ b/sdk/lib/_internal/js_runtime/lib/foreign_helper.dart
@@ -232,6 +232,11 @@
/// when the program has been analyzed.
external bool JS_GET_FLAG(String name);
+/// Always returns `false`. Because this method is opaque until SSA, it can be
+/// used to guard code which should be treated as live but not emitted by
+/// default.
+external bool JS_FALSE();
+
/// Returns the underlying JavaScript exception. Must be used in a catch block.
///
/// try {
diff --git a/sdk/lib/_internal/js_runtime/lib/interceptors.dart b/sdk/lib/_internal/js_runtime/lib/interceptors.dart
index cd4d96c..54db90f 100644
--- a/sdk/lib/_internal/js_runtime/lib/interceptors.dart
+++ b/sdk/lib/_internal/js_runtime/lib/interceptors.dart
@@ -8,7 +8,7 @@
show DISPATCH_PROPERTY_NAME, TYPE_TO_INTERCEPTOR_MAP;
import 'dart:collection' hide LinkedList, LinkedListEntry;
-import 'dart:_foreign_helper' show JS_GET_FLAG, TYPE_REF;
+import 'dart:_foreign_helper' show JS_FALSE, JS_GET_FLAG, TYPE_REF;
import 'dart:_internal' hide Symbol;
import "dart:_internal" as _symbol_dev show Symbol;
import 'dart:_js_helper'
diff --git a/sdk/lib/_internal/js_runtime/lib/js_number.dart b/sdk/lib/_internal/js_runtime/lib/js_number.dart
index 27dc231..cdd8fec 100644
--- a/sdk/lib/_internal/js_runtime/lib/js_number.dart
+++ b/sdk/lib/_internal/js_runtime/lib/js_number.dart
@@ -340,7 +340,7 @@
int operator ~/(num other) {
if (other is! num) throw argumentErrorValue(other);
- if (JS_GET_FLAG('FALSE')) _tdivFast(other); // Ensure resolution.
+ if (JS_FALSE()) _tdivFast(other); // Ensure resolution.
if (_isInt32(this)) {
if (other >= 1 || other < -1) {
return JS('int', r'(# / #) | 0', this, other);
@@ -400,7 +400,7 @@
num operator >>(num other) {
if (other is! num) throw argumentErrorValue(other);
if (other < 0) throw argumentErrorValue(other);
- if (JS_GET_FLAG('FALSE')) _shrReceiverPositive(other);
+ if (JS_FALSE()) _shrReceiverPositive(other);
return _shrOtherPositive(other);
}
