[dart2js] allow-cse and allow-dce annotations

Add `@pragma('dart2js:allow-cse')` and `@pragma('dart2js:allow-dce')`.

These annotations allow the compiler to do common subexpression
elimination and dead code elimination on calls to getters and methods.

Change-Id: Ie4091466e4782a4d28a7f9bfbfdb60a5de45a384
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426360
Reviewed-by: Nate Biggs <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>

Author: rakudrama

pkg/compiler/doc/pragmas.md

@@ -35,6 +35,8 @@ understands potential repercussions.
 | `dart2js:parameter:trust` | TBD |
 | `dart2js:types:check` | TBD |
 | `dart2js:types:trust` | TBD |
+| `dart2js:allow-cse` | [Allow common subexpression elimination (CSE)](#allow-cse) |
+| `dart2js:allow-dce` | [Allow dead code elimination (DCE)](#allow-dce) |
 
 ## Pragmas for internal use
 
@@ -45,8 +47,8 @@ only allowed within the core SDK libraries.
 | --- | --- |
 | `dart2js:assumeDynamic` | TBD |
 | `dart2js:disableFinal` | TBD |
-| `dart2js:noSideEffects` | Requires `dart2js:noInline` to work properly |
-| `dart2js:noThrows` | Requires `dart2js:noInline` to work properly |
+| `dart2js:noSideEffects` | Requires `dart2js:never-inline` to work properly |
+| `dart2js:noThrows` | Requires `dart2js:never-inline` to work properly |
 
 ## Detailed descriptions
 
@@ -125,7 +127,7 @@ This annotation may be placed on a function or method.
 
 Function inlining is disabled at call sites within the annotated function.
 Inlining is disabled even when the call site has a viable inlining candidate
-that is annotated with `@pragma('dart2js:tryInline')`.
+that is annotated with `@pragma('dart2js:prefer-inline')`.
 
 
 ### Annotations related to run-time checks
@@ -157,13 +159,13 @@ casts in the body of the function are checked.
 One use of `dart2js:as:trust` is to construct an `unsafeCast` method.
 
 ```dart
-@pragma('dart2js:tryInline')
+@pragma('dart2js:prefer-inline')
 @pragma('dart2js:as:trust')
 T unsafeCast<T>(Object? o) => o as T;
 ```
 
-The `tryInline` pragma ensures that the function is inlined, removing the cost
-of the call and passing the type parameter `T`, and the `as:trust` pragma
+The `prefer-inline` pragma ensures that the function is inlined, removing the
+cost of the call and passing the type parameter `T`, and the `as:trust` pragma
 removes the code that does the check.
 
 #### Downcasts
@@ -184,7 +186,7 @@ The `unsafeCast` method described above could also be written by trusting
 implicit downcasts.
 
 ```dart
-@pragma('dart2js:tryInline')
+@pragma('dart2js:prefer-inline')
 @pragma('dart2js:downcast:trust')
 T unsafeCast<T>(dynamic o) => o; // implicit downcast `as T`.
 ```
@@ -230,6 +232,41 @@ is opt-in via the following annotation.
 
 This annotation can be placed on a method, class or library.
 
+### Annotations related to compiler optimizations
+
+#### Allow CSE
+
+**EXPERIMENTAL**: This annotation may be removed without notice.
+
+This annotation may be placed on a method or getter. If, after some
+optimization, there are two calls to the same annotated method or getter, and
+the calls have the same input values, and the first call will always have
+happened by the time control flow reaches the second call, the compiler may
+choose to remove the second call and use the value returned by the first call
+instead.
+
+```dart
+@pragma('dart2js:allow-cse')
+```
+
+This annotation is intended for an `external` getter or method that always
+refers to the same thing.
+
+#### Allow DCE
+
+**EXPERIMENTAL**: This annotated may be removed without notice.
+
+This annotation may be placed on a method or getter. If the result of the call
+to the method or getter is unused (or becomes unused due to other
+optimizations), the compiler may choose to remove the call.
+
+```dart
+@pragma('dart2js:allow-dce')
+```
+
+This annotation is intended for and `external` getter or method that makes no
+observable change.
+
 ### Annotations related to deferred library loading
 
 #### Load priority

pkg/compiler/lib/src/js_backend/annotations.dart

@@ -93,7 +93,10 @@ enum PragmaAnnotation {
   loadLibraryPriority('load-priority', hasOption: true),
   resourceIdentifier('resource-identifier'),
 
-  throwWithoutHelperFrame('stack-starts-at-throw');
+  throwWithoutHelperFrame('stack-starts-at-throw'),
+
+  allowCSE('allow-cse'),
+  allowDCE('allow-dce');
 
   final String name;
   final bool forFunctionsOnly;
@@ -364,6 +367,14 @@ abstract class AnnotationsData {
   /// expression generates extra code to avoid having a runtime helper on the
   /// stack?
   bool throwWithoutHelperFrame(ir.TreeNode node);
+
+  /// Returns `true` if [member] has a `@pragma('dart2js:allow-cse')`
+  /// annotation.
+  bool allowCSE(MemberEntity member);
+
+  /// Returns `true` if [member] has a `@pragma('dart2js:allow-dce')`
+  /// annotation.
+  bool allowDCE(MemberEntity member);
 }
 
 class AnnotationsDataImpl implements AnnotationsData {
@@ -668,6 +679,14 @@ class AnnotationsDataImpl implements AnnotationsData {
     }
     return false;
   }
+
+  @override
+  bool allowCSE(MemberEntity member) =>
+      _hasPragma(member, PragmaAnnotation.allowCSE);
+
+  @override
+  bool allowDCE(MemberEntity member) =>
+      _hasPragma(member, PragmaAnnotation.allowDCE);
 }
 
 class AnnotationsDataBuilder {

pkg/compiler/lib/src/ssa/builder.dart

@@ -6804,6 +6804,8 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
       );
     }
     instruction.sideEffects = _inferredData.getSideEffectsOfElement(target);
+    instruction.allowCSE = closedWorld.annotationsData.allowCSE(target);
+    instruction.allowDCE = closedWorld.annotationsData.allowDCE(target);
     push(instruction);
   }
 
@@ -6973,6 +6975,11 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
       }
     }
 
+    if (element != null) {
+      invoke.allowCSE = closedWorld.annotationsData.allowCSE(element);
+      invoke.allowDCE = closedWorld.annotationsData.allowDCE(element);
+    }
+
     if (node is ir.InstanceInvocation ||
         node is ir.FunctionInvocation ||
         node is ir.InstanceGet) {
@@ -7893,6 +7900,14 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
       return false;
     }
 
+    // Don't inline functions marked with 'allow-cse' and 'allow-dce' since we
+    // need the call instruction to do these optimizations. We might be able to
+    // inline simple methods afterwards.
+    if (closedWorld.annotationsData.allowCSE(function) ||
+        closedWorld.annotationsData.allowDCE(function)) {
+      return false;
+    }
+
     bool insideLoop = loopDepth > 0 || graph.calledInLoop;
 
     // Bail out early if the inlining decision is in the cache and we can't

pkg/compiler/lib/src/ssa/nodes.dart

@@ -1175,6 +1175,11 @@ abstract class HInstruction implements SpannableWithEntity {
   SideEffects sideEffects = SideEffects.empty();
   bool _useGvn = false;
 
+  // TODO(sra): Consider whether to reduce instruction size by collecting all
+  // these instruction flags into a bitmask.
+  bool _allowCSE = false;
+  bool _allowDCE = false;
+
   // Main constructor copies the list of inputs to ensure ownership.
   HInstruction(List<HInstruction> initialInputs, this.instructionType)
     : inputs = [...initialInputs];
@@ -1219,6 +1224,26 @@ abstract class HInstruction implements SpannableWithEntity {
         !canThrow(domain);
   }
 
+  /// `true` if an instruction can be eliminated as a common subexpression -
+  /// when the instruction is equivalent to an earlier instruction may be
+  /// replaced by the value of the earlier instruction. Equivalent means that
+  /// the instruction has exactly the same inputs.
+  ///
+  /// This property is set on function invocations on the basis of annotations
+  /// and program analysis.  Usually, `allowCSE` means that the instruction is
+  /// idempotent - a second equivalent instruction returns the same value as the
+  /// first instruction without additional observable effects.
+  ///
+  ///  If an instruction is pure, it should be marked as `useGvn` instead.
+  bool get allowCSE => _allowCSE;
+
+  /// `true` if the instruction may be removed when the value is unused.
+  ///
+  /// This property is set on function invocations on the basis of annotations
+  /// and program analysis.  Usually `allowDCE` means that the instruction has
+  /// no observable effect.
+  bool get allowDCE => _allowDCE;
+
   /// An instruction is an 'allocation' is it is the sole alias for an object.
   /// This applies to instructions that allocate new objects and can be extended
   /// to methods that return other allocations without escaping them.
@@ -1315,11 +1340,11 @@ abstract class HInstruction implements SpannableWithEntity {
   bool isInBasicBlock() => block != null;
 
   bool gvnEquals(HInstruction other) {
-    assert(useGvn() && other.useGvn());
     // Check that the type and the sideEffects match.
     bool hasSameType = typeEquals(other);
     assert(hasSameType == (_gvnType == other._gvnType));
     if (!hasSameType) return false;
+    assert((useGvn() && other.useGvn()) || (allowCSE && other.allowCSE));
     if (sideEffects != other.sideEffects) return false;
     // Check that the inputs match.
     final int inputsLength = inputs.length;
@@ -1344,7 +1369,7 @@ abstract class HInstruction implements SpannableWithEntity {
   }
 
   // These methods should be overwritten by instructions that
-  // participate in global value numbering.
+  // participate in global value numbering or allowCSE.
   _GvnType get _gvnType => _GvnType.undefined;
   bool typeEquals(covariant HInstruction other) => false;
   bool dataEquals(covariant HInstruction other) => false;
@@ -1496,6 +1521,18 @@ abstract class HInstruction implements SpannableWithEntity {
   String toString() => '$runtimeType()';
 }
 
+mixin HasSettableAllowCSE on HInstruction {
+  set allowCSE(bool value) {
+    _allowCSE = value;
+  }
+}
+
+mixin HasSettableAllowDCE on HInstruction {
+  set allowDCE(bool value) {
+    _allowDCE = value;
+  }
+}
+
 /// An interface implemented by certain kinds of [HInstruction]. This makes it
 /// possible to discover which annotations were in force in the code from which
 /// the instruction originated.
@@ -1868,7 +1905,8 @@ class HCreateBox extends HInstruction {
   String toString() => 'HCreateBox()';
 }
 
-abstract class HInvoke extends HInstruction {
+abstract class HInvoke extends HInstruction
+    with HasSettableAllowCSE, HasSettableAllowDCE {
   bool _isAllocation = false;
 
   /// [isInterceptedCall] is true if this invocation uses the interceptor
@@ -2207,6 +2245,10 @@ class HInvokeStatic extends HInvoke {
 
   @override
   _GvnType get _gvnType => _GvnType.invokeStatic;
+  @override
+  bool typeEquals(other) => other is HInvokeStatic;
+  @override
+  bool dataEquals(HInvokeStatic other) => element == other.element;
 
   @override
   String toString() => 'invoke static: $element';
@@ -3938,9 +3980,10 @@ class HNullCheck extends HCheck {
 
   @override
   String toString() {
+    String stickyString = sticky ? 'sticky, ' : '';
     String fieldString = field == null ? '' : ', $field';
     String selectorString = selector == null ? '' : ', $selector';
-    return 'HNullCheck($checkedInput$fieldString$selectorString)';
+    return 'HNullCheck($stickyString$checkedInput$fieldString$selectorString)';
   }
 }