From 646fdacaa790fa29f0a7224a513c6514ecb9f627 Mon Sep 17 00:00:00 2001 From: jgruber Date: Mon, 15 May 2017 00:51:15 -0700 Subject: [PATCH] [csa] Add ToLength and ToString variants with inlined fast checks Smis can easily be handled outside the stub call without adding much to code size. The ToString inlining adds overhead of repeated instance type loads and checks, but under the assumption that it is called with mostly string values it should speed things up (a local RegExp.p[@@replace] microbenchmark shows consistent 1.6% improvements). Drive-by-fix: Remove duplication in ToString implementations. BUG= Review-Url: https://codereview.chromium.org/2874423003 Cr-Commit-Position: refs/heads/master@{#45287} --- src/builtins/builtins-array-gen.cc | 6 ++--- src/builtins/builtins-conversion-gen.cc | 30 +------------------------ src/builtins/builtins-regexp-gen.cc | 10 ++++----- src/builtins/builtins-string-gen.cc | 28 +++++++++-------------- src/code-stub-assembler.cc | 25 +++++++++++++++++++++ src/code-stub-assembler.h | 4 ++++ 6 files changed, 46 insertions(+), 57 deletions(-) diff --git a/src/builtins/builtins-array-gen.cc b/src/builtins/builtins-array-gen.cc index ca553b73ce..f9afa159cc 100644 --- a/src/builtins/builtins-array-gen.cc +++ b/src/builtins/builtins-array-gen.cc @@ -303,8 +303,7 @@ class ArrayBuiltinCodeStubAssembler : public CodeStubAssembler { BIND(¬_js_array); Node* len_property = GetProperty(context(), o(), isolate()->factory()->length_string()); - merged_length.Bind( - CallStub(CodeFactory::ToLength(isolate()), context(), len_property)); + merged_length.Bind(ToLength_Inline(context(), len_property)); Goto(&has_length); BIND(&has_length); len_ = merged_length.value(); @@ -2197,8 +2196,7 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) { { Node* length = GetProperty(context, array, factory()->length_string()); - Callable to_length = CodeFactory::ToLength(isolate()); - var_length.Bind(CallStub(to_length, context, length)); + var_length.Bind(ToLength_Inline(context, length)); Goto(&done); } diff --git a/src/builtins/builtins-conversion-gen.cc b/src/builtins/builtins-conversion-gen.cc index 21d59346b5..5fe2cb03bd 100644 --- a/src/builtins/builtins-conversion-gen.cc +++ b/src/builtins/builtins-conversion-gen.cc @@ -133,35 +133,7 @@ TF_BUILTIN(ToString, CodeStubAssembler) { Node* context = Parameter(Descriptor::kContext); Node* input = Parameter(Descriptor::kArgument); - Label is_number(this); - Label runtime(this); - - GotoIf(TaggedIsSmi(input), &is_number); - - Node* input_map = LoadMap(input); - Node* input_instance_type = LoadMapInstanceType(input_map); - - Label not_string(this); - GotoIfNot(IsStringInstanceType(input_instance_type), ¬_string); - Return(input); - - Label not_heap_number(this); - - BIND(¬_string); - { Branch(IsHeapNumberMap(input_map), &is_number, ¬_heap_number); } - - BIND(&is_number); - { Return(NumberToString(context, input)); } - - BIND(¬_heap_number); - { - GotoIf(Word32NotEqual(input_instance_type, Int32Constant(ODDBALL_TYPE)), - &runtime); - Return(LoadObjectField(input, Oddball::kToStringOffset)); - } - - BIND(&runtime); - { Return(CallRuntime(Runtime::kToString, context, input)); } + Return(ToString(context, input)); } // 7.1.1.1 OrdinaryToPrimitive ( O, hint ) diff --git a/src/builtins/builtins-regexp-gen.cc b/src/builtins/builtins-regexp-gen.cc index 4abd9c8b05..04a35bd000 100644 --- a/src/builtins/builtins-regexp-gen.cc +++ b/src/builtins/builtins-regexp-gen.cc @@ -577,8 +577,7 @@ Node* RegExpBuiltinsAssembler::RegExpPrototypeExecBodyWithoutResult( BIND(&call_tolength); { - var_lastindex.Bind( - CallBuiltin(Builtins::kToLength, context, regexp_lastindex)); + var_lastindex.Bind(ToLength_Inline(context, regexp_lastindex)); Goto(&next); } @@ -1941,7 +1940,7 @@ void RegExpBuiltinsAssembler::RegExpPrototypeMatchBody(Node* const context, if (is_fastpath) { CSA_ASSERT(this, TaggedIsPositiveSmi(last_index)); } else { - last_index = CallBuiltin(Builtins::kToLength, context, last_index); + last_index = ToLength_Inline(context, last_index); } Node* const new_last_index = @@ -2809,8 +2808,7 @@ TF_BUILTIN(RegExpReplace, RegExpBuiltinsAssembler) { // 3. Does ToString({replace_value}) contain '$'? BIND(&checkreplacestring); { - Node* const replace_string = - CallBuiltin(Builtins::kToString, context, replace_value); + Node* const replace_string = ToString_Inline(context, replace_value); // ToString(replaceValue) could potentially change the shape of the RegExp // object. Recheck that we are still on the fast path and bail to runtime @@ -2898,7 +2896,7 @@ TF_BUILTIN(RegExpPrototypeReplace, RegExpBuiltinsAssembler) { Node* const receiver = maybe_receiver; // Convert {maybe_string} to a String. - Node* const string = CallBuiltin(Builtins::kToString, context, maybe_string); + Node* const string = ToString_Inline(context, maybe_string); // Fast-path checks: 1. Is the {receiver} an unmodified JSRegExp instance? Label stub(this), runtime(this, Label::kDeferred); diff --git a/src/builtins/builtins-string-gen.cc b/src/builtins/builtins-string-gen.cc index 18fba261e8..6fb91ca807 100644 --- a/src/builtins/builtins-string-gen.cc +++ b/src/builtins/builtins-string-gen.cc @@ -767,7 +767,7 @@ TF_BUILTIN(StringPrototypeConcat, CodeStubAssembler) { arguments.ForEach( CodeStubAssembler::VariableList({&var_result}, zone()), [this, context, &var_result](Node* arg) { - arg = CallStub(CodeFactory::ToString(isolate()), context, arg); + arg = ToString_Inline(context, arg); var_result.Bind(CallStub(CodeFactory::StringAdd(isolate()), context, var_result.value(), arg)); }); @@ -1184,9 +1184,7 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) { MaybeCallFunctionAtSymbol( context, search, isolate()->factory()->replace_symbol(), [=]() { - Callable tostring_callable = CodeFactory::ToString(isolate()); - Node* const subject_string = - CallStub(tostring_callable, context, receiver); + Node* const subject_string = ToString_Inline(context, receiver); Callable replace_callable = CodeFactory::RegExpReplace(isolate()); return CallStub(replace_callable, context, search, subject_string, @@ -1199,11 +1197,10 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) { // Convert {receiver} and {search} to strings. - Callable tostring_callable = CodeFactory::ToString(isolate()); Callable indexof_callable = CodeFactory::StringIndexOf(isolate()); - Node* const subject_string = CallStub(tostring_callable, context, receiver); - Node* const search_string = CallStub(tostring_callable, context, search); + Node* const subject_string = ToString_Inline(context, receiver); + Node* const search_string = ToString_Inline(context, search); Node* const subject_length = LoadStringLength(subject_string); Node* const search_length = LoadStringLength(search_string); @@ -1257,7 +1254,7 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) { // TODO(jgruber): Could introduce ToStringSideeffectsStub which only // performs observable parts of ToString. - CallStub(tostring_callable, context, replace); + ToString_Inline(context, replace); Goto(&return_subject); BIND(&return_subject); @@ -1300,8 +1297,7 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) { Node* const replacement = CallJS(call_callable, context, replace, UndefinedConstant(), search_string, match_start_index, subject_string); - Node* const replacement_string = - CallStub(tostring_callable, context, replacement); + Node* const replacement_string = ToString_Inline(context, replacement); var_result.Bind(CallStub(stringadd_callable, context, var_result.value(), replacement_string)); Goto(&out); @@ -1309,7 +1305,7 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) { BIND(&if_notcallablereplace); { - Node* const replace_string = CallStub(tostring_callable, context, replace); + Node* const replace_string = ToString_Inline(context, replace); Node* const replacement = GetSubstitution(context, subject_string, match_start_index, match_end_index, replace_string); @@ -1429,9 +1425,7 @@ TF_BUILTIN(StringPrototypeSplit, StringBuiltinsAssembler) { MaybeCallFunctionAtSymbol( context, separator, isolate()->factory()->split_symbol(), [=]() { - Callable tostring_callable = CodeFactory::ToString(isolate()); - Node* const subject_string = - CallStub(tostring_callable, context, receiver); + Node* const subject_string = ToString_Inline(context, receiver); Callable split_callable = CodeFactory::RegExpSplit(isolate()); return CallStub(split_callable, context, separator, subject_string, @@ -1447,14 +1441,12 @@ TF_BUILTIN(StringPrototypeSplit, StringBuiltinsAssembler) { // but AFAIK there should not be a difference since arrays are capped at Smi // lengths. - Callable tostring_callable = CodeFactory::ToString(isolate()); - Node* const subject_string = CallStub(tostring_callable, context, receiver); + Node* const subject_string = ToString_Inline(context, receiver); Node* const limit_number = Select(IsUndefined(limit), [=]() { return SmiConstant(Smi::kMaxValue); }, [=]() { return ToUint32(context, limit); }, MachineRepresentation::kTagged); - Node* const separator_string = - CallStub(tostring_callable, context, separator); + Node* const separator_string = ToString_Inline(context, separator); // Shortcut for {limit} == 0. { diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index 73c6f2c2b4..08952874c7 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -4480,6 +4480,22 @@ Node* CodeStubAssembler::ToString(Node* context, Node* input) { return result.value(); } +Node* CodeStubAssembler::ToString_Inline(Node* const context, + Node* const input) { + VARIABLE(var_result, MachineRepresentation::kTagged, input); + Label stub_call(this, Label::kDeferred), out(this); + + GotoIf(TaggedIsSmi(input), &stub_call); + Branch(IsString(input), &out, &stub_call); + + BIND(&stub_call); + var_result.Bind(CallBuiltin(Builtins::kToString, context, input)); + Goto(&out); + + BIND(&out); + return var_result.value(); +} + Node* CodeStubAssembler::JSReceiverToPrimitive(Node* context, Node* input) { Label if_isreceiver(this, Label::kDeferred), if_isnotreceiver(this); VARIABLE(result, MachineRepresentation::kTagged); @@ -4557,6 +4573,15 @@ Node* CodeStubAssembler::ToSmiLength(Node* input, Node* const context, return result.value(); } +Node* CodeStubAssembler::ToLength_Inline(Node* const context, + Node* const input) { + Node* const smi_zero = SmiConstant(0); + return Select( + TaggedIsSmi(input), [=] { return SmiMax(input, smi_zero); }, + [=] { return CallBuiltin(Builtins::kToLength, context, input); }, + MachineRepresentation::kTagged); +} + Node* CodeStubAssembler::ToInteger(Node* context, Node* input, ToIntegerTruncationMode mode) { // We might need to loop once for ToNumber conversion. diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index 01417f5f5e..55fd4f3e18 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -856,6 +856,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler { // Convert any object to a String. Node* ToString(Node* context, Node* input); + Node* ToString_Inline(Node* const context, Node* const input); // Convert any object to a Primitive. Node* JSReceiverToPrimitive(Node* context, Node* input); @@ -871,6 +872,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler { // ES6 7.1.15 ToLength, but jumps to range_error if the result is not a Smi. Node* ToSmiLength(Node* input, Node* const context, Label* range_error); + // ES6 7.1.15 ToLength, but with inlined fast path. + Node* ToLength_Inline(Node* const context, Node* const input); + // Convert any object to an Integer. Node* ToInteger(Node* context, Node* input, ToIntegerTruncationMode mode = kNoTruncation);