[dart2js] Use indexes for operation names and verbs

Change-Id: Ic954a7e20062aa8bf1c0f622ee9b0656bc563ba5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375024 Commit-Queue: Stephen Adams <[email protected]> Reviewed-by: Mayank Patke <[email protected]>
dart-lang · Jul 15, 2024 · 15792df · 15792df
1 parent 88b6ee2
commit 15792df
Show file tree

Hide file tree

Showing 15 changed files with 429 additions and 68 deletions.
diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart
@@ -5424,7 +5424,7 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
   void _handleArrayFlagsCheck(
       ir.StaticInvocation invocation, SourceInformation? sourceInformation) {
     if (_unexpectedForeignArguments(invocation,
-        minPositional: 4, maxPositional: 4, typeArgumentCount: 1)) {
+        minPositional: 4, maxPositional: 5, typeArgumentCount: 1)) {
       // Result expected on stack.
       stack.add(graph.addConstantNull(closedWorld));
       return;
@@ -5434,6 +5434,7 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
     final arrayFlags = inputs[1];
     final checkFlags = inputs[2];
     final operation = inputs[3];
+    final verb = inputs.length > 4 ? inputs[4] : null;
 
     // TODO(sra): Use the flags to improve in the AbstractValue, which may
     // contain powerset domain bits outside of the conventional type
@@ -5444,7 +5445,7 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
     instructionType ??= _abstractValueDomain.dynamicType;
 
     push(HArrayFlagsCheck(
-        array, arrayFlags, checkFlags, operation, instructionType)
+        array, arrayFlags, checkFlags, operation, verb, instructionType)
       ..sourceInformation = sourceInformation);
   }
 

diff --git a/pkg/compiler/lib/src/ssa/codegen.dart b/pkg/compiler/lib/src/ssa/codegen.dart
@@ -3486,18 +3486,21 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
       test = js.js('# & #', [arrayFlags, checkFlags]);
     }
 
-    final operation = node.operation;
-    // Most common operation is "[]=", so 'pass' that by leaving it out.
-    if (operation
-        case HConstant(constant: StringConstantValue(stringValue: '[]='))) {
-      _pushCallStatic(_commonElements.throwUnsupportedOperation, [array],
-          node.sourceInformation);
-    } else {
-      use(operation);
-      _pushCallStatic(_commonElements.throwUnsupportedOperation, [array, pop()],
-          node.sourceInformation);
+    List<js.Expression> arguments = [array];
+
+    if (node.hasOperation) {
+      use(node.operation);
+      arguments.add(pop());
     }
 
+    if (node.hasVerb) {
+      use(node.verb);
+      arguments.add(pop());
+    }
+
+    _pushCallStatic(_commonElements.throwUnsupportedOperation, arguments,
+        node.sourceInformation);
+
     js.Statement check;
     if (test == null) {
       check = js.js.statement('#;', pop());

diff --git a/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart b/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart
@@ -246,10 +246,11 @@ class IndexAssignSpecializer extends InvokeDynamicSpecializer {
       HInstruction mask =
           graph.addConstantInt(ArrayFlags.unmodifiableCheck, closedWorld);
       HInstruction name = graph.addConstantString('[]=', closedWorld);
+      HInstruction verb = graph.addConstantString('modify', closedWorld);
       final instructionType = receiver.instructionType;
-      final checkFlags =
-          HArrayFlagsCheck(receiver, getFlags, mask, name, instructionType)
-            ..sourceInformation = instruction.sourceInformation;
+      final checkFlags = HArrayFlagsCheck(
+          receiver, getFlags, mask, name, verb, instructionType)
+        ..sourceInformation = instruction.sourceInformation;
       instruction.block!.addBefore(instruction, checkFlags);
       checkFlags.instructionType = checkFlags.computeInstructionType(
           instructionType, abstractValueDomain);

diff --git a/pkg/compiler/lib/src/ssa/nodes.dart b/pkg/compiler/lib/src/ssa/nodes.dart
@@ -1401,6 +1401,12 @@ abstract class HInstruction implements SpannableWithEntity {
     replacement.usedBy.add(this);
   }
 
+  /// Remove a single input.
+  void removeInput(int index) {
+    inputs[index].usedBy.remove(this);
+    inputs.removeAt(index);
+  }
+
   void replaceAllUsersDominatedBy(
       HInstruction cursor, HInstruction newInstruction) {
     DominatedUses.of(this, cursor).replaceWith(newInstruction);
@@ -4707,21 +4713,37 @@ class HTypeBind extends HInstruction implements HRtiInstruction {
 ///
 ///     a = ...
 ///     f = HArrayFlagsGet(a);
-///     a2 = HArrayFlagsCheck(a, f, ArrayFlags.unmodifiableCheck, "[]=")
+///     a2 = HArrayFlagsCheck(a, f, ArrayFlags.unmodifiableCheck, "[]=", "modify")
 ///     a2[i] = 0
 ///
 /// HArrayFlagsGet is a separate instruction so that 'loading' the flags from
 /// the Array can by hoisted.
 class HArrayFlagsCheck extends HCheck {
-  HArrayFlagsCheck(HInstruction array, HInstruction arrayFlags,
-      HInstruction checkFlags, HInstruction operation, AbstractValue type)
-      : super([array, arrayFlags, checkFlags, operation], type);
+  HArrayFlagsCheck(
+      HInstruction array,
+      HInstruction arrayFlags,
+      HInstruction checkFlags,
+      HInstruction? operation,
+      HInstruction? verb,
+      AbstractValue type)
+      : super([
+          array,
+          arrayFlags,
+          checkFlags,
+          if (operation != null) operation,
+          if (verb != null) verb,
+        ], type);
 
   HInstruction get array => inputs[0];
   HInstruction get arrayFlags => inputs[1];
   HInstruction get checkFlags => inputs[2];
+
+  bool get hasOperation => inputs.length > 3;
   HInstruction get operation => inputs[3];
 
+  bool get hasVerb => inputs.length > 4;
+  HInstruction get verb => inputs[4];
+
   // The checked type is the input type, refined to match the flags.
   AbstractValue computeInstructionType(
       AbstractValue inputType, AbstractValueDomain domain) {

diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart
@@ -2675,6 +2675,57 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
       }
     }
 
+    // The 'operation' and 'verb' strings can be replaced with an index into a
+    // small table to known operations or verbs. This makes the call-sites
+    // smaller, so is worthwhile for calls to HArrayFlagsCheck that are inlined
+    // into multiple places.
+    //
+    // A trailing zero index (verb, or verb and operation) can be omitted from
+    // the instruction.
+    //
+    // When both indexes are replaced by indexes, the indexes are combined into
+    // a single value.
+    //
+    //     finalIndex = verbIndex * numberOfOperationIndexes + operationIndex
+
+    int? verbIndex; // Verb index if nonzero.
+
+    if (node.hasVerb) {
+      if (node.verb
+          case HConstant(constant: StringConstantValue(:final stringValue))) {
+        final index = ArrayFlags.verbToIndex[stringValue];
+        if (index != null) {
+          if (index == 0) {
+            node.removeInput(4);
+          } else {
+            final replacement = _graph.addConstantInt(index, _closedWorld);
+            node.replaceInput(4, replacement);
+            verbIndex = index;
+          }
+        }
+      }
+    }
+
+    if (node.hasOperation) {
+      if (node.operation
+          case HConstant(constant: StringConstantValue(:final stringValue))) {
+        var index = ArrayFlags.operationNameToIndex[stringValue];
+        if (index != null) {
+          if (index == 0 && !node.hasVerb) {
+            node.removeInput(3);
+          } else {
+            if (verbIndex != null) {
+              // Encode combined indexes and remove 'verb' input.
+              index += verbIndex * ArrayFlags.operationNameToIndex.length;
+              node.removeInput(4);
+            }
+            final replacement = _graph.addConstantInt(index, _closedWorld);
+            node.replaceInput(3, replacement);
+          }
+        }
+      }
+    }
+
     return node;
   }
 

diff --git a/pkg/compiler/test/codegen/data/array_flags_optimizations.dart b/pkg/compiler/test/codegen/data/array_flags_optimizations.dart
@@ -0,0 +1,90 @@
+// Copyright (c) 2024, 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.
+
+// Test of various space-saving contractions of calls to
+// `throwUnsupportedOperation`.
+
+import 'dart:_foreign_helper' show ArrayFlags, HArrayFlagsCheck;
+
+@pragma('dart2js:never-inline')
+// The operation and verb are both elided.
+/*member: indexSetter:function() {
+  A.throwUnsupportedOperation(B.List_empty);
+}*/
+indexSetter() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      '[]=', 'modify');
+}
+
+@pragma('dart2js:never-inline')
+// The operation is reduced to an index but cannot be elided because it is
+// followed by a non-elided verb.
+/*member: indexSetterUnusualVerb:function() {
+  A.throwUnsupportedOperation(B.List_empty, 0, "change contents of");
+}*/
+indexSetterUnusualVerb() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      '[]=', 'change contents of');
+}
+
+@pragma('dart2js:never-inline')
+// The verb is elided.
+/*member: unusualOperationModify:function() {
+  A.throwUnsupportedOperation(B.List_empty, "rub");
+}*/
+unusualOperationModify() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      'rub', 'modify');
+}
+
+@pragma('dart2js:never-inline')
+// The operation is left as a string and verb is
+/*member: unusualOperationRemoveFrom:function() {
+  A.throwUnsupportedOperation(B.List_empty, "rub", 1);
+}*/
+unusualOperationRemoveFrom() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      'rub', 'remove from');
+}
+
+@pragma('dart2js:never-inline')
+// The operation and verb are left as strings.
+/*member: unusualOperationAndVerb:function() {
+  A.throwUnsupportedOperation(B.List_empty, "rub", "burnish");
+}*/
+unusualOperationAndVerb() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      'rub', 'burnish');
+}
+
+@pragma('dart2js:never-inline')
+// The operation is reduced to an index and the verb is elided.
+/*member: knownOperationModify:function() {
+  A.throwUnsupportedOperation(B.List_empty, 10);
+}*/
+knownOperationModify() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      'setUint16', 'modify');
+}
+
+@pragma('dart2js:never-inline')
+// The operation and verb are combined to a single number.
+/*member: knownOperationAndVerb:function() {
+  A.throwUnsupportedOperation(B.List_empty, 16);
+}*/
+knownOperationAndVerb() {
+  HArrayFlagsCheck(const [], ArrayFlags.constant, ArrayFlags.unmodifiableCheck,
+      'removeWhere', 'remove from');
+}
+
+/*member: main:ignore*/
+main() {
+  indexSetter();
+  indexSetterUnusualVerb();
+  unusualOperationModify();
+  unusualOperationRemoveFrom();
+  unusualOperationAndVerb();
+  knownOperationModify();
+  knownOperationAndVerb();
+}
diff --git a/pkg/compiler/test/codegen/data/unmodifiable_bytedata.dart b/pkg/compiler/test/codegen/data/unmodifiable_bytedata.dart
@@ -52,7 +52,7 @@ returnModifiable2() {
 /*member: guaranteedFail:function() {
   var a = new DataView(new ArrayBuffer(10));
   a.$flags = 3;
-  A.throwUnsupportedOperation(a, "setInt32");
+  A.throwUnsupportedOperation(a, 8);
   a.setInt32(0, 100, false);
   a.setUint32(4, 2000, false);
   return a;
@@ -67,7 +67,7 @@ guaranteedFail() {
 
 @pragma('dart2js:never-inline')
 /*member: multipleWrites:function(data) {
-  data.$flags & 2 && A.throwUnsupportedOperation(data, "setFloat64");
+  data.$flags & 2 && A.throwUnsupportedOperation(data, 13);
   data.setFloat64(0, 1.23, false);
   data.setFloat32(8, 1.23, false);
   return data;
@@ -83,7 +83,7 @@ multipleWrites(ByteData data) {
 /*member: hoistedLoad:function(data) {
   var t1, i;
   for (t1 = data.$flags | 0, i = 0; i < data.byteLength; i += 2) {
-    t1 & 2 && A.throwUnsupportedOperation(data, "setUint16");
+    t1 & 2 && A.throwUnsupportedOperation(data, 10);
     data.setUint16(i, 100, true);
   }
   return data;

diff --git a/pkg/compiler/test/rti/emission/list.dart b/pkg/compiler/test/rti/emission/list.dart
@@ -2,7 +2,7 @@
 // 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.
 
-/*spec.class: global#JSArray:checkedInstance,checks=[$isIterable],instance*/
+/*spec.class: global#JSArray:checkedInstance,checks=[$isIterable,$isList],instance*/
 /*prod.class: global#JSArray:checks=[$isIterable],instance*/
 
 /*class: global#Iterable:checkedInstance*/

diff --git a/pkg/js_runtime/lib/synced/array_flags.dart b/pkg/js_runtime/lib/synced/array_flags.dart
@@ -60,4 +60,64 @@ class ArrayFlags {
 
   /// Bit to check for constant JSArray.
   static const int constantCheck = 4;
+
+  /// A List of operation names for HArrayFlagsCheck, encoded in a string of
+  /// names separated by semicolons.
+  ///
+  /// The optimizer may replace the string with an index into this
+  /// table. Generally this makes the generated code smaller. A name does not
+  /// need to be in this table. It makes sense for this table to include only
+  /// names that occur in more than one HArrayFlagsCheck, either as written in
+  /// js_runtime, or occuring multiple times via inlining.
+  static const operationNames = //
+      '[]=' // This is the most common operation so it comes first.
+      ';add'
+      ';removeWhere'
+      ';retainWhere'
+      ';removeRange'
+      ';setRange'
+      ';setInt8'
+      ';setInt16'
+      ';setInt32'
+      ';setUint8'
+      ';setUint16'
+      ';setUint32'
+      ';setFloat32'
+      ';setFloat64'
+      //
+      ;
+
+  /// A list of 'verbs' for HArrayFlagsCheck, encoded as a string of phrases
+  /// separated by semicolons. The 'verb' fills a span of the error message, for
+  /// example, "remove from" in the message
+  ///
+  ///     Cannot remove from an unmodifiable list.
+  ///            ^^^^^^^^^^^
+  ///
+  /// The optimizer may replace the string with an index into this
+  /// table. Generally this makes the program smaller. A 'verb' does not need to
+  /// be in this table. It makes sense to only have verbs that are used by
+  /// several calls to HArrayFlagsCheck, either as written in js_runtime, or
+  /// occuring multiple times via inlining.
+  static const verbs = //
+      'modify' // This is the verb for '[]=' so it comes first.
+      ';remove from'
+      ';add to'
+      //
+      ;
+
+  /// A view of [operationNames] as a map from the name to the index of the name
+  /// in the list.
+  static final Map<String, int> operationNameToIndex = _invert(operationNames);
+
+  /// A view of [verbs] as a map from the verb to the index of the verb in the
+  /// list.
+  static final Map<String, int> verbToIndex = _invert(verbs);
+
+  static Map<String, int> _invert(String joinedStrings) {
+    return {
+      for (final entry in joinedStrings.split(';').asMap().entries)
+        entry.value: entry.key,
+    };
+  }
 }
diff --git a/sdk/lib/_internal/js_runtime/lib/foreign_helper.dart b/sdk/lib/_internal/js_runtime/lib/foreign_helper.dart
@@ -336,11 +336,15 @@ external int HArrayFlagsGet(Object array);
 /// intervening HArrayFlagsSet that might invalidate the check.
 @pragma('dart2js:as:trust')
 T HArrayFlagsCheck<T>(
-    Object array, int arrayFlags, int checkFlags, String operation) {
+    Object array, int arrayFlags, int checkFlags, String operation,
+    [String verb = 'modify']) {
   // This body is unused but serves as a model for global for impacts and
   // analysis.
   if (arrayFlags & checkFlags != 0) {
-    throwUnsupportedOperation(array, operation);
+    if (operation == '[]=') throwUnsupportedOperation(array);
+    if (operation == 'setUint32') throwUnsupportedOperation(array, 1);
+    if (verb == 'remove from') throwUnsupportedOperation(array, operation, 1);
+    throwUnsupportedOperation(array, operation, verb);
   }
   return array as T;
 }