[dart2js] Lower String.codeUnitAt to charCodeAt

Change-Id: Id4408a761304fc2fcb7ce3bd353c626958971ffb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300500
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Stephen Adams <sra@google.com>

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

@@ -4500,6 +4500,8 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
       _handleLateWriteOnceCheck(invocation);
     } else if (name == '_lateInitializeOnceCheck') {
       _handleLateInitializeOnceCheck(invocation);
+    } else if (name == 'HCharCodeAt') {
+      _handleCharCodeAt(invocation);
     } else {
       reporter.internalError(
           _elementMap.getSpannable(targetElement, invocation),
@@ -5170,6 +5172,17 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
     push(HStringConcat(inputs[0], inputs[1], _abstractValueDomain.stringType));
   }
 
+  void _handleCharCodeAt(ir.StaticInvocation invocation) {
+    if (_unexpectedForeignArguments(invocation,
+        minPositional: 2, maxPositional: 2)) {
+      // Result expected on stack.
+      stack.add(graph.addConstantNull(closedWorld));
+      return;
+    }
+    List<HInstruction> inputs = _visitPositionalArguments(invocation.arguments);
+    push(HCharCodeAt(inputs[0], inputs[1], _abstractValueDomain.uint31Type));
+  }
+
   void _handleForeignTypeRef(ir.StaticInvocation invocation) {
     if (_unexpectedForeignArguments(invocation,
         minPositional: 0, maxPositional: 0, typeArgumentCount: 1)) {

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

@@ -2940,6 +2940,15 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
         .withSourceInformation(node.sourceInformation));
   }
 
+  @override
+  void visitCharCodeAt(HCharCodeAt node) {
+    use(node.receiver);
+    js.Expression receiver = pop();
+    use(node.index);
+    push(js.js('#.charCodeAt(#)', [receiver, pop()]).withSourceInformation(
+        node.sourceInformation));
+  }
+
   void checkTypeOf(HInstruction input, String cmp, String typeName,
       SourceInformation? sourceInformation) {
     use(input);

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

@@ -322,6 +322,44 @@ class IndexSpecializer extends InvokeDynamicSpecializer {
   }
 }
 
+class CodeUnitAtSpecializer extends InvokeDynamicSpecializer {
+  const CodeUnitAtSpecializer();
+
+  @override
+  constant_system.BinaryOperation operation() {
+    return constant_system.codeUnitAt;
+  }
+
+  @override
+  HInstruction? tryConvertToBuiltin(
+      HInvokeDynamic instruction,
+      HGraph graph,
+      GlobalTypeInferenceResults results,
+      JCommonElements commonElements,
+      JClosedWorld closedWorld,
+      OptimizationTestLog? log) {
+    final abstractValueDomain = closedWorld.abstractValueDomain;
+    HInstruction receiver = instruction.getDartReceiver(closedWorld);
+    if (receiver.isStringOrNull(abstractValueDomain).isPotentiallyFalse) {
+      return null;
+    }
+    HInstruction index = instruction.inputs.last;
+    if (index.isInteger(abstractValueDomain).isPotentiallyFalse) {
+      return null;
+    }
+
+    HInstruction checkedIndex = index;
+    if (requiresBoundsCheck(instruction, closedWorld)) {
+      checkedIndex =
+          insertBoundsCheck(instruction, receiver, index, closedWorld, log);
+    }
+    final converted =
+        HCharCodeAt(receiver, checkedIndex, abstractValueDomain.uint31Type);
+    log?.registerCodeUnitAt(instruction);
+    return converted;
+  }
+}
+
 class RemoveLastSpecializer extends InvokeDynamicSpecializer {
   const RemoveLastSpecializer();
 
@@ -1436,43 +1474,6 @@ class LessEqualSpecializer extends RelationalSpecializer {
   }
 }
 
-class CodeUnitAtSpecializer extends InvokeDynamicSpecializer {
-  const CodeUnitAtSpecializer();
-
-  @override
-  constant_system.BinaryOperation operation() {
-    return constant_system.codeUnitAt;
-  }
-
-  @override
-  HInstruction? tryConvertToBuiltin(
-      HInvokeDynamic instruction,
-      HGraph graph,
-      GlobalTypeInferenceResults results,
-      JCommonElements commonElements,
-      JClosedWorld closedWorld,
-      OptimizationTestLog? log) {
-    // TODO(sra): Implement a builtin HCodeUnitAt instruction and the same index
-    // bounds checking optimizations as for HIndex.
-    HInstruction receiver = instruction.getDartReceiver(closedWorld);
-    if (receiver
-        .isStringOrNull(closedWorld.abstractValueDomain)
-        .isDefinitelyTrue) {
-      // Even if there is no builtin equivalent instruction, we know
-      // String.codeUnitAt does not have any side effect (other than throwing),
-      // and that it can be GVN'ed.
-      clearAllSideEffects(instruction);
-      if (instruction.inputs.last
-          .isPositiveInteger(closedWorld.abstractValueDomain)
-          .isDefinitelyTrue) {
-        redirectSelector(instruction, '_codeUnitAt', commonElements);
-      }
-      log?.registerCodeUnitAt(instruction);
-    }
-    return null;
-  }
-}
-
 class CompareToSpecializer extends InvokeDynamicSpecializer {
   const CompareToSpecializer();
 

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

@@ -47,6 +47,7 @@ abstract class HVisitor<R> {
   R visitBitXor(HBitXor node);
   R visitBoundsCheck(HBoundsCheck node);
   R visitBreak(HBreak node);
+  R visitCharCodeAt(HCharCodeAt node);
   R visitConstant(HConstant node);
   R visitContinue(HContinue node);
   R visitCreate(HCreate node);
@@ -467,6 +468,8 @@ class HBaseVisitor<R> extends HGraphVisitor implements HVisitor<R> {
   R visitBreak(HBreak node) => visitJump(node);
   @override
   R visitContinue(HContinue node) => visitJump(node);
+  @override
+  R visitCharCodeAt(HCharCodeAt node) => visitInstruction(node);
   R visitCheck(HCheck node) => visitInstruction(node);
   @override
   R visitConstant(HConstant node) => visitInstruction(node);
@@ -1128,6 +1131,8 @@ abstract class HInstruction implements SpannableWithEntity {
   static const int LATE_WRITE_ONCE_CHECK_TYPECODE = 62;
   static const int LATE_INITIALIZE_ONCE_CHECK_TYPECODE = 63;
 
+  static const int CHAR_CODE_AT_TYPECODE = 64;
+
   HInstruction(this.inputs, this.instructionType);
   @override
   Entity? get sourceEntity => sourceElement;
@@ -3475,7 +3480,8 @@ class HIndex extends HInstruction {
   HInstruction get index => inputs[1];
 
   // Implicit dependency on HBoundsCheck or constraints on index.
-  // TODO(27272): Make HIndex dependent on bounds checking.
+  // TODO(27272): Make HIndex dependent on positions of eliminated bounds
+  // checks.
   @override
   bool get isMovable => false;
 
@@ -3515,7 +3521,7 @@ class HIndexAssign extends HInstruction {
   HInstruction get value => inputs[2];
 
   // Implicit dependency on HBoundsCheck or constraints on index.
-  // TODO(27272): Make HIndex dependent on bounds checking.
+  // TODO(27272): Make HIndex dependent on eliminated bounds checks.
   @override
   bool get isMovable => false;
 
@@ -3528,13 +3534,47 @@ class HIndexAssign extends HInstruction {
       receiver.isNull(domain).isPotentiallyTrue;
 }
 
+class HCharCodeAt extends HInstruction {
+  HCharCodeAt(HInstruction receiver, HInstruction index, AbstractValue type)
+      : super([receiver, index], type);
+
+  @override
+  String toString() => 'HCharCodeAt';
+  @override
+  R accept<R>(HVisitor<R> visitor) => visitor.visitCharCodeAt(this);
+
+  HInstruction get receiver => inputs[0];
+  HInstruction get index => inputs[1];
+
+  // Implicit dependency on HBoundsCheck or constraints on index.
+  // TODO(27272): Make HCharCodeAt dependent on positions of eliminated bounds
+  // checks.
+  @override
+  bool get isMovable => false;
+
+  @override
+  HInstruction getDartReceiver(JClosedWorld closedWorld) => receiver;
+  @override
+  bool onlyThrowsNSM() => true;
+  @override
+  bool canThrow(AbstractValueDomain domain) =>
+      receiver.isNull(domain).isPotentiallyTrue;
+
+  @override
+  int typeCode() => HInstruction.CHAR_CODE_AT_TYPECODE;
+  @override
+  bool typeEquals(other) => other is HCharCodeAt;
+  @override
+  bool dataEquals(HCharCodeAt other) => true;
+}
+
 /// HLateValue is a late-stage instruction that can be used to force a value
 /// into a temporary.
 ///
 /// HLateValue is useful for naming values that would otherwise be generated at
 /// use site, for example, if 'this' is used many times, replacing uses of
 /// 'this' with HLateValue(HThis) will have the effect of copying 'this' to a
-/// temporary will reduce the size of minified code.
+/// temporary which will reduce the size of minified code.
 class HLateValue extends HLateInstruction {
   HLateValue(HInstruction target) : super([target], target.instructionType);
 

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

@@ -1575,6 +1575,14 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
     return node;
   }
 
+  @override
+  HInstruction visitCharCodeAt(HCharCodeAt node) {
+    final folded =
+        foldBinary(constant_system.codeUnitAt, node.receiver, node.index);
+    if (folded != null) return folded;
+    return node;
+  }
+
   /// Returns the guarded receiver.
   HInstruction maybeGuardWithNullCheck(
       HInstruction receiver, HInvokeDynamic node, FieldEntity? field) {

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

@@ -250,6 +250,13 @@ class HInstructionStringifier implements HVisitor<String> {
     return "Break: (B${target.id})";
   }
 
+  @override
+  String visitCharCodeAt(HCharCodeAt node) {
+    String receiver = temporaryId(node.receiver);
+    String index = temporaryId(node.index);
+    return "CharCodeAt: $receiver.charCodeAt($index)";
+  }
+
   @override
   String visitConstant(HConstant constant) => "Constant: ${constant.constant}";
 

pkg/compiler/test/codegen/data/codeUnitAt_folding.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.
 
-@pragma('dart2js:noInline')
+@pragma('dart2js:never-inline')
 /*member: foo1:function() {
   return 72;
 }*/
@@ -13,29 +13,76 @@ foo1() {
   // Constant folds to 'return 72;'
 }
 
-@pragma('dart2js:noInline')
+@pragma('dart2js:never-inline')
 /*spec|canary.member: foo2:function() {
-  return B.JSString_methods.codeUnitAt$1("Hello", A._asInt(1.5));
+  return B.JSString_methods.codeUnitAt$1("Hello", A._asInt("x"));
 }*/
 /*prod.member: foo2:function() {
-  return B.JSString_methods.codeUnitAt$1("Hello", 1.5);
+  return B.JSString_methods.codeUnitAt$1("Hello", "x");
 }*/
 foo2() {
   var a = 'Hello';
-  dynamic b = 1.5;
+  dynamic b = 'x';
   return a.codeUnitAt(b);
   // No folding of index type error.
 }
 
-@pragma('dart2js:noInline')
+@pragma('dart2js:never-inline')
 /*member: foo3:function() {
-  return B.JSString_methods._codeUnitAt$1("Hello", 55);
+  return A.ioore("Hello", 55);
+  return "Hello".charCodeAt(55);
 }*/
 foo3() {
   var a = 'Hello';
   dynamic b = 55;
   return a.codeUnitAt(b);
-  // No folding of index range error.
+  // Index always out of range.
+  // The code after the always-fail check is unfortunate.
+}
+
+@pragma('dart2js:never-inline')
+/*member: foo4:function(i) {
+  if (!(i >= 0 && i < 5))
+    return A.ioore("Hello", i);
+  return "Hello".charCodeAt(i);
+}*/
+foo4(int i) {
+  return 'Hello'.codeUnitAt(i);
+  // Normal bounds check.
+}
+
+@pragma('dart2js:never-inline')
+/*member: foo5:function(i) {
+  if (!(i < 5))
+    return A.ioore("Hello", i);
+  return "Hello".charCodeAt(i);
+}*/
+foo5(int i) {
+  return 'Hello'.codeUnitAt(i);
+  // High-only bounds check.
+}
+
+@pragma('dart2js:never-inline')
+@pragma('dart2js:index-bounds:trust')
+/*member: foo6:function(i) {
+  return "Hello".charCodeAt(i);
+}*/
+foo6(int i) {
+  return 'Hello'.codeUnitAt(i);
+  // No bound check, as requested.
+}
+
+@pragma('dart2js:never-inline')
+@pragma('dart2js:index-bounds:trust')
+/*spec|canary.member: foo7:function(i) {
+  return "Hello".charCodeAt(A._asInt(i));
+}*/
+/*prod.member: foo7:function(i) {
+  return B.JSString_methods.codeUnitAt$1("Hello", i);
+}*/
+foo7(dynamic i) {
+  return 'Hello'.codeUnitAt(i);
+  // No folding of index type error even when bounds check removed.
 }
 
 /*member: main:ignore*/
@@ -43,4 +90,14 @@ main() {
   foo1();
   foo2();
   foo3();
+  foo4(-9);
+  foo4(0);
+  foo4(100);
+  foo5(0);
+  foo5(100);
+  foo6(-9);
+  foo6(0);
+  foo6(100);
+  foo7(0);
+  foo7('x');
 }