[vm] Make String.codeUnitAt monomorphic

The new implementation of String.codeUnitAt is always inlined and
doesn't depend on the polymorphic inlining of recognized methods.

String.codeUnitAt now has a custom body in the flow graph builder
which  performs non-speculative bounds check and then branches between
OneByteString and TwoByteString. Corresponding graph intrinsic and
native method are removed.

This change also fixes passing of unboxed arguments to runtime
in the slow path of GenericCheckBound instruction in JIT mode
(when shared stubs are not used).

TEST=ci

Change-Id: Iab2805fc752df84c37089165f828e31aca5f043f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359000
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This commit is contained in:
Alexander Markov 2024-03-24 14:50:17 +00:00 committed by Commit Queue
parent c61aecda75
commit 35d8630176
17 changed files with 1149 additions and 1086 deletions

View file

@ -448,15 +448,6 @@ DEFINE_NATIVE_ENTRY(String_charAt, 0, 2) {
return Symbols::FromCharCode(thread, static_cast<int32_t>(value));
}
// Returns the 16-bit UTF-16 code unit at the given index.
DEFINE_NATIVE_ENTRY(String_codeUnitAt, 0, 2) {
const String& receiver =
String::CheckedHandle(zone, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, index, arguments->NativeArgAt(1));
uint16_t value = StringValueAt(receiver, index);
return Smi::New(static_cast<intptr_t>(value));
}
DEFINE_NATIVE_ENTRY(String_concat, 0, 2) {
const String& receiver =
String::CheckedHandle(zone, arguments->NativeArgAt(0));

View file

@ -135,7 +135,6 @@ namespace dart {
V(String_getHashCode, 1) \
V(String_getLength, 1) \
V(String_charAt, 2) \
V(String_codeUnitAt, 2) \
V(String_concat, 2) \
V(String_fromEnvironment, 3) \
V(String_toLowerCase, 1) \

View file

@ -3156,8 +3156,25 @@ void NullErrorSlowPath::EmitSharedStubCall(FlowGraphCompiler* compiler,
void RangeErrorSlowPath::PushArgumentsForRuntimeCall(
FlowGraphCompiler* compiler) {
LocationSummary* locs = instruction()->locs();
__ PushRegisterPair(locs->in(CheckBoundBaseInstr::kIndexPos).reg(),
locs->in(CheckBoundBaseInstr::kLengthPos).reg());
if (GenericCheckBoundInstr::UseUnboxedRepresentation()) {
// Can't pass unboxed int64 value directly to runtime call, as all
// arguments are expected to be tagged (boxed).
// The unboxed int64 argument is passed through a dedicated slot in Thread.
// TODO(dartbug.com/33549): Clean this up when unboxed values
// could be passed as arguments.
__ StoreToOffset(
locs->in(CheckBoundBaseInstr::kLengthPos).reg(),
compiler::Address(
THR, compiler::target::Thread::unboxed_runtime_arg_offset()));
__ StoreToOffset(
locs->in(CheckBoundBaseInstr::kIndexPos).reg(),
compiler::Address(
THR, compiler::target::Thread::unboxed_runtime_arg_offset() +
kInt64Size));
} else {
__ PushRegisterPair(locs->in(CheckBoundBaseInstr::kIndexPos).reg(),
locs->in(CheckBoundBaseInstr::kLengthPos).reg());
}
}
void RangeErrorSlowPath::EmitSharedStubCall(FlowGraphCompiler* compiler,

View file

@ -273,10 +273,17 @@ class NullErrorSlowPath : public ThrowErrorSlowPathCode {
class RangeErrorSlowPath : public ThrowErrorSlowPathCode {
public:
explicit RangeErrorSlowPath(GenericCheckBoundInstr* instruction)
: ThrowErrorSlowPathCode(instruction, kRangeErrorRuntimeEntry) {}
: ThrowErrorSlowPathCode(
instruction,
GenericCheckBoundInstr::UseUnboxedRepresentation()
? kRangeErrorUnboxedInt64RuntimeEntry
: kRangeErrorRuntimeEntry) {}
virtual const char* name() { return "check bound"; }
virtual intptr_t GetNumberOfArgumentsForRuntimeCall() {
if (GenericCheckBoundInstr::UseUnboxedRepresentation()) {
return 0; // Unboxed arguments are passed through Thread.
}
return 2; // length and index
}

View file

@ -3166,14 +3166,14 @@ static bool InlineStringBaseCharAt(FlowGraph* flow_graph,
return true;
}
static bool InlineStringCodeUnitAt(FlowGraph* flow_graph,
Instruction* call,
Definition* receiver,
intptr_t cid,
GraphEntryInstr* graph_entry,
FunctionEntryInstr** entry,
Instruction** last,
Definition** result) {
static bool InlineStringBaseCodeUnitAt(FlowGraph* flow_graph,
Instruction* call,
Definition* receiver,
intptr_t cid,
GraphEntryInstr* graph_entry,
FunctionEntryInstr** entry,
Instruction** last,
Definition** result) {
if (cid == kDynamicCid) {
ASSERT(call->IsStaticCall());
return false;
@ -3220,8 +3220,11 @@ bool FlowGraphInliner::TryReplaceInstanceCallWithInline(
ASSERT((last != nullptr && result != nullptr) ||
(target.recognized_kind() == MethodRecognizer::kObjectConstructor));
// Determine if inlining instance methods needs a check.
// StringBase.codeUnitAt is monomorphic but its implementation is selected
// based on the receiver cid.
FlowGraph::ToCheck check = FlowGraph::ToCheck::kNoCheck;
if (target.is_polymorphic_target()) {
if (target.is_polymorphic_target() ||
(target.recognized_kind() == MethodRecognizer::kStringBaseCodeUnitAt)) {
check = FlowGraph::ToCheck::kCheckCid;
} else {
check = flow_graph->CheckForInstanceCall(call, target.kind());
@ -4274,10 +4277,10 @@ bool FlowGraphInliner::TryInlineRecognizedMethod(
return InlineSetIndexed(flow_graph, kind, target, call, receiver, source,
exactness, graph_entry, entry, last, result);
}
case MethodRecognizer::kOneByteStringCodeUnitAt:
case MethodRecognizer::kTwoByteStringCodeUnitAt:
return InlineStringCodeUnitAt(flow_graph, call, receiver, receiver_cid,
graph_entry, entry, last, result);
case MethodRecognizer::kStringBaseCodeUnitAt:
return InlineStringBaseCodeUnitAt(flow_graph, call, receiver,
receiver_cid, graph_entry, entry, last,
result);
case MethodRecognizer::kStringBaseCharAt:
return InlineStringBaseCharAt(flow_graph, call, receiver, receiver_cid,
graph_entry, entry, last, result);

View file

@ -397,6 +397,14 @@ Fragment BaseFlowGraphBuilder::LoadIndexed(classid_t class_id,
return Fragment(instr);
}
Fragment BaseFlowGraphBuilder::GenericCheckBound() {
Value* index = Pop();
Value* length = Pop();
auto* instr = new (Z) GenericCheckBoundInstr(length, index, GetNextDeoptId());
Push(instr);
return Fragment(instr);
}
Fragment BaseFlowGraphBuilder::LoadUntagged(intptr_t offset) {
Value* object = Pop();
auto load = new (Z) LoadUntaggedInstr(object, offset);

View file

@ -188,6 +188,7 @@ class BaseFlowGraphBuilder {
intptr_t index_scale = compiler::target::kWordSize,
bool index_unboxed = false,
AlignmentType alignment = kAlignedAccess);
Fragment GenericCheckBound();
Fragment LoadUntagged(intptr_t offset);
Fragment CalculateElementAddress(intptr_t index_scale);

View file

@ -1058,6 +1058,7 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph(
case MethodRecognizer::kFinalizerEntry_allocate:
case MethodRecognizer::kFinalizerEntry_getExternalSize:
case MethodRecognizer::kObjectEquals:
case MethodRecognizer::kStringBaseCodeUnitAt:
case MethodRecognizer::kStringBaseLength:
case MethodRecognizer::kStringBaseIsEmpty:
case MethodRecognizer::kClassIDgetID:
@ -1113,6 +1114,17 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph(
}
}
bool FlowGraphBuilder::IsExpressionTempVarUsedInRecognizedMethodFlowGraph(
const Function& function) {
ASSERT(IsRecognizedMethodForFlowGraph(function));
switch (function.recognized_kind()) {
case MethodRecognizer::kStringBaseCodeUnitAt:
return true;
default:
return false;
}
}
FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
const Function& function) {
ASSERT(IsRecognizedMethodForFlowGraph(function));
@ -1292,6 +1304,51 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
body += LoadLocal(parsed_function_->RawParameterVariable(1));
body += StrictCompare(Token::kEQ_STRICT);
break;
case MethodRecognizer::kStringBaseCodeUnitAt: {
ASSERT_EQUAL(function.NumParameters(), 2);
body += LoadLocal(parsed_function_->RawParameterVariable(0));
body += LoadNativeField(Slot::String_length());
body += LoadLocal(parsed_function_->RawParameterVariable(1));
body += GenericCheckBound();
LocalVariable* safe_index = MakeTemporary();
JoinEntryInstr* done = BuildJoinEntry();
LocalVariable* result = parsed_function_->expression_temp_var();
TargetEntryInstr* one_byte_string;
TargetEntryInstr* two_byte_string;
body += LoadLocal(parsed_function_->RawParameterVariable(0));
body += LoadClassId();
body += IntConstant(kOneByteStringCid);
body += BranchIfEqual(&one_byte_string, &two_byte_string);
body.current = one_byte_string;
body += LoadLocal(parsed_function_->RawParameterVariable(0));
body += LoadLocal(safe_index);
body += LoadIndexed(
kOneByteStringCid,
/*index_scale=*/
compiler::target::Instance::ElementSizeFor(kOneByteStringCid),
/*index_unboxed=*/GenericCheckBoundInstr::UseUnboxedRepresentation());
body += StoreLocal(TokenPosition::kNoSource, result);
body += Drop();
body += Goto(done);
body.current = two_byte_string;
body += LoadLocal(parsed_function_->RawParameterVariable(0));
body += LoadLocal(safe_index);
body += LoadIndexed(
kTwoByteStringCid,
/*index_scale=*/
compiler::target::Instance::ElementSizeFor(kTwoByteStringCid),
/*index_unboxed=*/GenericCheckBoundInstr::UseUnboxedRepresentation());
body += StoreLocal(TokenPosition::kNoSource, result);
body += Drop();
body += Goto(done);
body.current = done;
body += DropTemporary(&safe_index);
body += LoadLocal(result);
} break;
case MethodRecognizer::kStringBaseLength:
case MethodRecognizer::kStringBaseIsEmpty:
ASSERT_EQUAL(function.NumParameters(), 1);

View file

@ -60,6 +60,11 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
// graph building and its body is expressed in a custom-built IL.
static bool IsRecognizedMethodForFlowGraph(const Function& function);
// Returns true if custom flow graph for given [function]
// needs an expression_temp_var().
static bool IsExpressionTempVarUsedInRecognizedMethodFlowGraph(
const Function& function);
private:
BlockEntryInstr* BuildPrologue(BlockEntryInstr* normal_entry,
PrologueInfo* prologue_info);

View file

@ -5,6 +5,7 @@
#include "vm/compiler/frontend/scope_builder.h"
#include "vm/compiler/backend/il.h" // For CompileType.
#include "vm/compiler/frontend/kernel_to_il.h"
#include "vm/compiler/frontend/kernel_translation_helper.h"
namespace dart {
@ -167,6 +168,11 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() {
--depth_.catch_;
}
}
if (FlowGraphBuilder::IsRecognizedMethodForFlowGraph(function) &&
FlowGraphBuilder::IsExpressionTempVarUsedInRecognizedMethodFlowGraph(
function)) {
needs_expr_temp_ = true;
}
intptr_t pos = 0;
if (function.IsClosureFunction()) {
LocalVariable* closure_parameter = MakeVariable(

View file

@ -431,49 +431,6 @@ DEFINE_SIMD_ARRAY_GETTER_SETTER_INTRINSICS(Float64x2Array)
#undef DEFINE_SIMD_ARRAY_GETTER_INTRINSIC
#undef DEFINE_SIMD_ARRAY_SETTER_INTRINSIC
static bool BuildCodeUnitAt(FlowGraph* flow_graph, intptr_t cid) {
GraphEntryInstr* graph_entry = flow_graph->graph_entry();
auto normal_entry = graph_entry->normal_entry();
BlockBuilder builder(flow_graph, normal_entry, /*with_frame=*/false);
Definition* str = builder.AddParameter(0);
Definition* index = builder.AddParameter(1);
VerifyParameterIsBoxed(&builder, 0);
index = CreateBoxedParameterIfNeeded(&builder, index, kUnboxedInt64, 1);
index =
PrepareIndexedOp(flow_graph, &builder, str, index, Slot::String_length());
Definition* load = builder.AddDefinition(new LoadIndexedInstr(
new Value(str), new Value(index), /*index_unboxed=*/false,
target::Instance::ElementSizeFor(cid), cid, kAlignedAccess,
DeoptId::kNone, builder.Source()));
// We don't perform [RangeAnalysis] for graph intrinsics. To inform the
// following boxing instruction about a more precise range we attach it here
// manually.
// http://dartbug.com/36632
auto const rep = RepresentationUtils::RepresentationOfArrayElement(cid);
load->set_range(Range::Full(rep));
Definition* result = CreateBoxedResultIfNeeded(&builder, load, rep);
if (result->IsBoxInteger()) {
result->AsBoxInteger()->ClearEnv();
}
builder.AddReturn(new Value(result));
return true;
}
bool GraphIntrinsifier::Build_OneByteStringCodeUnitAt(FlowGraph* flow_graph) {
return BuildCodeUnitAt(flow_graph, kOneByteStringCid);
}
bool GraphIntrinsifier::Build_TwoByteStringCodeUnitAt(FlowGraph* flow_graph) {
return BuildCodeUnitAt(flow_graph, kTwoByteStringCid);
}
static bool BuildSimdOp(FlowGraph* flow_graph, intptr_t cid, Token::Kind kind) {
if (!FlowGraphCompiler::SupportsUnboxedSimd128()) return false;

View file

@ -124,6 +124,7 @@ namespace dart {
V(::, copyRangeFromUint8ListToOneByteString, \
CopyRangeFromUint8ListToOneByteString, 0xcc42d0a2) \
V(_StringBase, _interpolate, StringBaseInterpolate, 0x8af456e6) \
V(_StringBase, codeUnitAt, StringBaseCodeUnitAt, 0x17ea80f1) \
V(_SuspendState, get:_functionData, SuspendState_getFunctionData, \
0x7281768e) \
V(_SuspendState, set:_functionData, SuspendState_setFunctionData, \
@ -452,8 +453,6 @@ namespace dart {
V(_GrowableList, [], GrowableArrayGetIndexed, 0x78e668b1) \
V(_GrowableList, _setIndexed, GrowableArraySetIndexedUnchecked, 0x513c774f) \
V(_StringBase, get:length, StringBaseLength, 0x5842648b) \
V(_OneByteString, codeUnitAt, OneByteStringCodeUnitAt, 0x17ea7d30) \
V(_TwoByteString, codeUnitAt, TwoByteStringCodeUnitAt, 0x17ea7d30) \
V(_Smi, ~, Smi_bitNegate, 0x82466cfc) \
V(_IntegerImplementation, +, Integer_add, 0x6f06d26c) \
V(_IntegerImplementation, -, Integer_sub, 0x630fe15d) \

File diff suppressed because it is too large Load diff

View file

@ -9240,6 +9240,7 @@ bool Function::RecognizedKindForceOptimize() const {
case MethodRecognizer::kGetNativeField:
case MethodRecognizer::kRecord_fieldNames:
case MethodRecognizer::kRecord_numFields:
case MethodRecognizer::kStringBaseCodeUnitAt:
case MethodRecognizer::kUtf8DecoderScan:
case MethodRecognizer::kDouble_hashCode:
case MethodRecognizer::kTypedList_GetInt8:

View file

@ -146,6 +146,23 @@ DEFINE_RUNTIME_ENTRY(RangeError, 2) {
Exceptions::ThrowByType(Exceptions::kRange, args);
}
DEFINE_RUNTIME_ENTRY(RangeErrorUnboxedInt64, 0) {
int64_t unboxed_length = thread->unboxed_int64_runtime_arg();
int64_t unboxed_index = thread->unboxed_int64_runtime_second_arg();
const auto& length = Integer::Handle(zone, Integer::New(unboxed_length));
const auto& index = Integer::Handle(zone, Integer::New(unboxed_index));
// Throw: new RangeError.range(index, 0, length - 1, "length");
const Array& args = Array::Handle(zone, Array::New(4));
args.SetAt(0, index);
args.SetAt(1, Integer::Handle(zone, Integer::New(0)));
args.SetAt(
2, Integer::Handle(
zone, Integer::Cast(length).ArithmeticOp(
Token::kSUB, Integer::Handle(zone, Integer::New(1)))));
args.SetAt(3, Symbols::Length());
Exceptions::ThrowByType(Exceptions::kRange, args);
}
DEFINE_RUNTIME_ENTRY(WriteError, 0) {
Exceptions::ThrowUnsupportedError("Cannot modify an unmodifiable list");
}

View file

@ -48,6 +48,7 @@ namespace dart {
V(TraceICCall) \
V(PatchStaticCall) \
V(RangeError) \
V(RangeErrorUnboxedInt64) \
V(WriteError) \
V(NullError) \
V(NullErrorWithSelector) \

View file

@ -273,7 +273,11 @@ abstract final class _StringBase implements String {
@pragma("vm:external-name", "String_charAt")
external String operator [](int index);
int codeUnitAt(int index); // Implemented in the subclasses.
@pragma("vm:recognized", "other")
@pragma("vm:prefer-inline")
@pragma("vm:idempotent")
@pragma("vm:exact-result-type", "dart:core#_Smi")
external int codeUnitAt(int index);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:exact-result-type", "dart:core#_Smi")
@ -1010,11 +1014,6 @@ final class _OneByteString extends _StringBase {
@pragma("vm:external-name", "String_getHashCode")
external int get hashCode;
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:exact-result-type", "dart:core#_Smi")
@pragma("vm:external-name", "String_codeUnitAt")
external int codeUnitAt(int index);
bool _isWhitespace(int codeUnit) {
return _StringBase._isOneByteWhitespace(codeUnit);
}
@ -1363,11 +1362,6 @@ final class _TwoByteString extends _StringBase {
return _StringBase._isTwoByteWhitespace(codeUnit);
}
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:exact-result-type", "dart:core#_Smi")
@pragma("vm:external-name", "String_codeUnitAt")
external int codeUnitAt(int index);
@pragma("vm:recognized", "asm-intrinsic")
@pragma("vm:exact-result-type", bool)
// Intrinsic is more efficient than an inlined body even for the small