From d3da4311c09c7269b35aa8288408a6e2defb9e2b Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Fri, 5 May 2023 15:27:11 +0000 Subject: [PATCH] [dart2js] Lower String.codeUnitAt to charCodeAt Change-Id: Id4408a761304fc2fcb7ce3bd353c626958971ffb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300500 Reviewed-by: Sigmund Cherem Commit-Queue: Stephen Adams --- pkg/compiler/lib/src/ssa/builder.dart | 13 ++++ pkg/compiler/lib/src/ssa/codegen.dart | 9 +++ .../src/ssa/invoke_dynamic_specializers.dart | 75 ++++++++++--------- pkg/compiler/lib/src/ssa/nodes.dart | 46 +++++++++++- pkg/compiler/lib/src/ssa/optimize.dart | 8 ++ pkg/compiler/lib/src/ssa/tracer.dart | 7 ++ .../test/codegen/data/codeUnitAt_folding.dart | 73 ++++++++++++++++-- 7 files changed, 183 insertions(+), 48 deletions(-) diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart index 26e3918eba44..ffeacb24552e 100644 --- a/pkg/compiler/lib/src/ssa/builder.dart +++ b/pkg/compiler/lib/src/ssa/builder.dart @@ -4500,6 +4500,8 @@ class KernelSsaGraphBuilder extends ir.Visitor 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 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 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)) { diff --git a/pkg/compiler/lib/src/ssa/codegen.dart b/pkg/compiler/lib/src/ssa/codegen.dart index 00f97419ac29..1195670d0fce 100644 --- a/pkg/compiler/lib/src/ssa/codegen.dart +++ b/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); diff --git a/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart b/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart index 76ffc8c7dcc1..948c3652f0d6 100644 --- a/pkg/compiler/lib/src/ssa/invoke_dynamic_specializers.dart +++ b/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(); diff --git a/pkg/compiler/lib/src/ssa/nodes.dart b/pkg/compiler/lib/src/ssa/nodes.dart index 02d47e3f63b6..446b1d2ad1aa 100644 --- a/pkg/compiler/lib/src/ssa/nodes.dart +++ b/pkg/compiler/lib/src/ssa/nodes.dart @@ -47,6 +47,7 @@ abstract class HVisitor { 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 extends HGraphVisitor implements HVisitor { 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,34 @@ 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; + + @override + HInstruction getDartReceiver(JClosedWorld closedWorld) => receiver; + @override + bool onlyThrowsNSM() => true; + @override + bool canThrow(AbstractValueDomain domain) => + 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(HVisitor 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; @@ -3526,6 +3559,13 @@ class HIndexAssign extends HInstruction { @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 @@ -3534,7 +3574,7 @@ class HIndexAssign extends HInstruction { /// 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); diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart index 623e87f5c28a..c1f3ee68085b 100644 --- a/pkg/compiler/lib/src/ssa/optimize.dart +++ b/pkg/compiler/lib/src/ssa/optimize.dart @@ -1575,6 +1575,14 @@ class SsaInstructionSimplifier extends HBaseVisitor 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) { diff --git a/pkg/compiler/lib/src/ssa/tracer.dart b/pkg/compiler/lib/src/ssa/tracer.dart index 5618557339eb..bac4d5202a2b 100644 --- a/pkg/compiler/lib/src/ssa/tracer.dart +++ b/pkg/compiler/lib/src/ssa/tracer.dart @@ -250,6 +250,13 @@ class HInstructionStringifier implements HVisitor { 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}"; diff --git a/pkg/compiler/test/codegen/data/codeUnitAt_folding.dart b/pkg/compiler/test/codegen/data/codeUnitAt_folding.dart index 993e670c26f3..b6df6605da54 100644 --- a/pkg/compiler/test/codegen/data/codeUnitAt_folding.dart +++ b/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'); }