Revision: 17290
Author: [email protected]
Date: Mon Oct 21 12:42:08 2013 UTC
Log: Inline number to string conversion for string addition into
BinaryOp(Stub).
This fixes a performance regression that was caused by converting the
BinaryOpStub to a Hydrogen code stub. It also fixes a leftover TODO wrt.
the handling of Number*String or String*Number versions of the stub.
[email protected]
Review URL: https://codereview.chromium.org/27674002
http://code.google.com/p/v8/source/detail?r=17290
Modified:
/branches/bleeding_edge/src/arm/code-stubs-arm.cc
/branches/bleeding_edge/src/code-stubs-hydrogen.cc
/branches/bleeding_edge/src/code-stubs.cc
/branches/bleeding_edge/src/code-stubs.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/hydrogen.h
/branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
/branches/bleeding_edge/src/isolate.cc
/branches/bleeding_edge/src/x64/code-stubs-x64.cc
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Fri Oct 18 14:55:21
2013 UTC
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Mon Oct 21 12:42:08
2013 UTC
@@ -65,7 +65,8 @@
static Register registers[] = { r0 };
descriptor->register_param_count_ = 1;
descriptor->register_params_ = registers;
- descriptor->deoptimization_handler_ = NULL;
+ descriptor->deoptimization_handler_ =
+ Runtime::FunctionForId(Runtime::kNumberToString)->entry;
}
=======================================
--- /branches/bleeding_edge/src/code-stubs-hydrogen.cc Fri Oct 18 14:55:21
2013 UTC
+++ /branches/bleeding_edge/src/code-stubs-hydrogen.cc Mon Oct 21 12:42:08
2013 UTC
@@ -351,7 +351,7 @@
HValue* CodeStubGraphBuilder<NumberToStringStub>::BuildCodeStub() {
info()->MarkAsSavesCallerDoubles();
HValue* number = GetParameter(NumberToStringStub::kNumber);
- return BuildNumberToString(number);
+ return BuildNumberToString(number, handle(Type::Number(), isolate()));
}
@@ -881,35 +881,52 @@
if (stub->operation() == Token::ADD &&
(left_type->Maybe(Type::String()) ||
right_type->Maybe(Type::String())) &&
!left_type->Is(Type::String()) && !right_type->Is(Type::String())) {
- // For the generic add stub a fast case for String add is performance
+ // For the generic add stub a fast case for string addition is
performance
// critical.
if (left_type->Maybe(Type::String())) {
- IfBuilder left_string(this);
- left_string.If<HIsStringAndBranch>(left);
- left_string.Then();
- Push(Add<HStringAdd>(left, right, STRING_ADD_CHECK_RIGHT));
- left_string.Else();
- Push(AddInstruction(BuildBinaryOperation(stub->operation(),
- left, right, left_type, right_type, result_type,
- stub->fixed_right_arg(), true)));
- left_string.End();
+ IfBuilder if_leftisstring(this);
+ if_leftisstring.If<HIsStringAndBranch>(left);
+ if_leftisstring.Then();
+ {
+ Push(AddInstruction(BuildBinaryOperation(
+ stub->operation(), left, right,
+ handle(Type::String(), isolate()), right_type,
+ result_type, stub->fixed_right_arg(), true)));
+ }
+ if_leftisstring.Else();
+ {
+ Push(AddInstruction(BuildBinaryOperation(
+ stub->operation(), left, right,
+ left_type, right_type, result_type,
+ stub->fixed_right_arg(), true)));
+ }
+ if_leftisstring.End();
result = Pop();
} else {
- IfBuilder right_string(this);
- right_string.If<HIsStringAndBranch>(right);
- right_string.Then();
- Push(Add<HStringAdd>(left, right, STRING_ADD_CHECK_LEFT));
- right_string.Else();
- Push(AddInstruction(BuildBinaryOperation(stub->operation(),
- left, right, left_type, right_type, result_type,
- stub->fixed_right_arg(), true)));
- right_string.End();
+ IfBuilder if_rightisstring(this);
+ if_rightisstring.If<HIsStringAndBranch>(right);
+ if_rightisstring.Then();
+ {
+ Push(AddInstruction(BuildBinaryOperation(
+ stub->operation(), left, right,
+ left_type, handle(Type::String(), isolate()),
+ result_type, stub->fixed_right_arg(), true)));
+ }
+ if_rightisstring.Else();
+ {
+ Push(AddInstruction(BuildBinaryOperation(
+ stub->operation(), left, right,
+ left_type, right_type, result_type,
+ stub->fixed_right_arg(), true)));
+ }
+ if_rightisstring.End();
result = Pop();
}
} else {
- result = AddInstruction(BuildBinaryOperation(stub->operation(),
- left, right, left_type, right_type, result_type,
- stub->fixed_right_arg(), true));
+ result = AddInstruction(BuildBinaryOperation(
+ stub->operation(), left, right,
+ left_type, right_type, result_type,
+ stub->fixed_right_arg(), true));
}
// If we encounter a generic argument, the number conversion is
=======================================
--- /branches/bleeding_edge/src/code-stubs.cc Fri Oct 18 14:55:21 2013 UTC
+++ /branches/bleeding_edge/src/code-stubs.cc Mon Oct 21 12:42:08 2013 UTC
@@ -570,14 +570,8 @@
State max_input = Max(left_state_, right_state_);
- // TODO(olivf) Instead of doing this normalization we should have a
Hydrogen
- // version of the LookupNumberStringCache to avoid a converting
StringAddStub.
- if (left_state_ == STRING && right_state_ < STRING) {
- right_state_ = GENERIC;
- } else if (right_state_ == STRING && left_state_ < STRING) {
- left_state_ = GENERIC;
- } else if (!has_int_result() && op_ != Token::SHR &&
- max_input <= NUMBER && max_input > result_state_) {
+ if (!has_int_result() && op_ != Token::SHR &&
+ max_input <= NUMBER && max_input > result_state_) {
result_state_ = max_input;
}
@@ -1125,6 +1119,12 @@
ArrayNArgumentsConstructorStub stub3(GetInitialFastElementsKind());
InstallDescriptor(isolate, &stub3);
}
+
+
+void NumberToStringStub::InstallDescriptors(Isolate* isolate) {
+ NumberToStringStub stub;
+ InstallDescriptor(isolate, &stub);
+}
void FastNewClosureStub::InstallDescriptors(Isolate* isolate) {
=======================================
--- /branches/bleeding_edge/src/code-stubs.h Fri Oct 18 14:55:21 2013 UTC
+++ /branches/bleeding_edge/src/code-stubs.h Mon Oct 21 12:42:08 2013 UTC
@@ -477,6 +477,8 @@
Isolate* isolate,
CodeStubInterfaceDescriptor* descriptor) V8_OVERRIDE;
+ static void InstallDescriptors(Isolate* isolate);
+
// Parameters accessed via CodeStubGraphBuilder::GetParameter()
static const int kNumber = 0;
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Mon Oct 21 12:32:38 2013 UTC
+++ /branches/bleeding_edge/src/hydrogen.cc Mon Oct 21 12:42:08 2013 UTC
@@ -1283,9 +1283,10 @@
}
-HValue* HGraphBuilder::BuildLookupNumberStringCache(
- HValue* object,
- HIfContinuation* continuation) {
+HValue* HGraphBuilder::BuildNumberToString(HValue* object,
+ Handle<Type> type) {
+ NoObservableSideEffectsScope scope(this);
+
// Create a joinable continuation.
HIfContinuation found(graph()->CreateBasicBlock(),
graph()->CreateBasicBlock());
@@ -1294,7 +1295,7 @@
HValue* number_string_cache =
Add<HLoadRoot>(Heap::kNumberStringCacheRootIndex);
- // Make the hash maks from the length of the number string cache. It
+ // Make the hash mask from the length of the number string cache. It
// contains two elements (number and string) for each cache entry.
HValue* mask = AddLoadFixedArrayLength(number_string_cache);
mask->set_type(HType::Smi());
@@ -1317,7 +1318,7 @@
// Check if object == key.
IfBuilder if_objectiskey(this);
- if_objectiskey.If<HCompareObjectEqAndBranch>(key, object);
+ if_objectiskey.If<HCompareObjectEqAndBranch>(object, key);
if_objectiskey.Then();
{
// Make the key_index available.
@@ -1327,92 +1328,84 @@
}
if_objectissmi.Else();
{
- // Check if object is a heap number.
- IfBuilder if_objectisnumber(this);
- if_objectisnumber.If<HCompareMap>(
- object, isolate()->factory()->heap_number_map());
- if_objectisnumber.Then();
- {
- // Compute hash for heap number similar to double_get_hash().
- HValue* low = Add<HLoadNamedField>(
- object, HObjectAccess::ForHeapNumberValueLowestBits());
- HValue* high = Add<HLoadNamedField>(
- object, HObjectAccess::ForHeapNumberValueHighestBits());
- HValue* hash = Add<HBitwise>(Token::BIT_XOR, low, high);
- hash = Add<HBitwise>(Token::BIT_AND, hash, mask);
+ if (type->Is(Type::Smi())) {
+ if_objectissmi.Deopt("Excepted smi");
+ } else {
+ // Check if the object is a heap number.
+ IfBuilder if_objectisnumber(this);
+ if_objectisnumber.If<HCompareMap>(
+ object, isolate()->factory()->heap_number_map());
+ if_objectisnumber.Then();
+ {
+ // Compute hash for heap number similar to double_get_hash().
+ HValue* low = Add<HLoadNamedField>(
+ object, HObjectAccess::ForHeapNumberValueLowestBits());
+ HValue* high = Add<HLoadNamedField>(
+ object, HObjectAccess::ForHeapNumberValueHighestBits());
+ HValue* hash = Add<HBitwise>(Token::BIT_XOR, low, high);
+ hash = Add<HBitwise>(Token::BIT_AND, hash, mask);
- // Load the key.
- HValue* key_index = Add<HShl>(hash, graph()->GetConstant1());
- HValue* key = Add<HLoadKeyed>(number_string_cache, key_index,
- static_cast<HValue*>(NULL),
- FAST_ELEMENTS, ALLOW_RETURN_HOLE);
+ // Load the key.
+ HValue* key_index = Add<HShl>(hash, graph()->GetConstant1());
+ HValue* key = Add<HLoadKeyed>(number_string_cache, key_index,
+ static_cast<HValue*>(NULL),
+ FAST_ELEMENTS, ALLOW_RETURN_HOLE);
- // Check if key is a heap number.
- IfBuilder if_keyisnumber(this);
- if_keyisnumber.IfNot<HIsSmiAndBranch>(key);
- if_keyisnumber.AndIf<HCompareMap>(
- key, isolate()->factory()->heap_number_map());
- if_keyisnumber.Then();
- {
- // Check if values of key and object match.
- IfBuilder if_keyeqobject(this);
- if_keyeqobject.If<HCompareNumericAndBranch>(
- Add<HLoadNamedField>(key, HObjectAccess::ForHeapNumberValue()),
- Add<HLoadNamedField>(object,
HObjectAccess::ForHeapNumberValue()),
- Token::EQ);
- if_keyeqobject.Then();
+ // Check if key is a heap number (the number string cache contains
only
+ // SMIs and heap number, so it is sufficient to do a SMI check
here).
+ IfBuilder if_keyisnotsmi(this);
+ if_keyisnotsmi.IfNot<HIsSmiAndBranch>(key);
+ if_keyisnotsmi.Then();
{
- // Make the key_index available.
- Push(key_index);
+ // Check if values of key and object match.
+ IfBuilder if_keyeqobject(this);
+ if_keyeqobject.If<HCompareNumericAndBranch>(
+ Add<HLoadNamedField>(key,
HObjectAccess::ForHeapNumberValue()),
+ Add<HLoadNamedField>(object,
HObjectAccess::ForHeapNumberValue()),
+ Token::EQ);
+ if_keyeqobject.Then();
+ {
+ // Make the key_index available.
+ Push(key_index);
+ }
+ if_keyeqobject.JoinContinuation(&found);
}
- if_keyeqobject.JoinContinuation(&found);
+ if_keyisnotsmi.JoinContinuation(&found);
}
- if_keyisnumber.JoinContinuation(&found);
+ if_objectisnumber.Else();
+ {
+ if (type->Is(Type::Number())) {
+ if_objectisnumber.Deopt("Expected heap number");
+ }
+ }
+ if_objectisnumber.JoinContinuation(&found);
}
- if_objectisnumber.JoinContinuation(&found);
}
- if_objectissmi.End();
+ if_objectissmi.JoinContinuation(&found);
// Check for cache hit.
IfBuilder if_found(this, &found);
if_found.Then();
+ {
+ // Count number to string operation in native code.
+ AddIncrementCounter(isolate()->counters()->number_to_string_native());
- // Load the value in case of cache hit.
- HValue* key_index = Pop();
- HValue* value_index = Add<HAdd>(key_index, graph()->GetConstant1());
- HValue* value = Add<HLoadKeyed>(number_string_cache, value_index,
- static_cast<HValue*>(NULL),
- FAST_ELEMENTS, ALLOW_RETURN_HOLE);
- AddIncrementCounter(isolate()->counters()->number_to_string_native());
-
- if_found.CaptureContinuation(continuation);
-
- // The value is only available in true branch of continuation.
- return value;
-}
-
-
-HValue* HGraphBuilder::BuildNumberToString(HValue* number) {
- NoObservableSideEffectsScope scope(this);
-
- // Lookup the number in the number string cache.
- HIfContinuation continuation;
- HValue* value = BuildLookupNumberStringCache(number, &continuation);
- IfBuilder if_found(this, &continuation);
- if_found.Then();
-
- // Cache hit.
- Push(value);
-
+ // Load the value in case of cache hit.
+ HValue* key_index = Pop();
+ HValue* value_index = Add<HAdd>(key_index, graph()->GetConstant1());
+ Push(Add<HLoadKeyed>(number_string_cache, value_index,
+ static_cast<HValue*>(NULL),
+ FAST_ELEMENTS, ALLOW_RETURN_HOLE));
+ }
if_found.Else();
-
- // Cache miss, fallback to runtime.
- Add<HPushArgument>(number);
- Push(Add<HCallRuntime>(
- isolate()->factory()->empty_string(),
- Runtime::FunctionForId(Runtime::kNumberToStringSkipCache),
- 1));
-
+ {
+ // Cache miss, fallback to runtime.
+ Add<HPushArgument>(object);
+ Push(Add<HCallRuntime>(
+ isolate()->factory()->empty_string(),
+ Runtime::FunctionForId(Runtime::kNumberToStringSkipCache),
+ 1));
+ }
if_found.End();
return Pop();
@@ -7849,6 +7842,42 @@
if (!maybe_string_add) right = TruncateToNumber(right, &right_type);
right_rep = Representation::FromType(right_type);
}
+
+ // Special case for string addition here.
+ if (op == Token::ADD &&
+ (left_type->Is(Type::String()) || right_type->Is(Type::String()))) {
+ if (left_type->Is(Type::String())) {
+ IfBuilder if_isstring(this);
+ if_isstring.If<HIsStringAndBranch>(left);
+ if_isstring.Then();
+ if_isstring.ElseDeopt("Expected string for LHS of binary operation");
+ } else if (left_type->Is(Type::Number())) {
+ left = BuildNumberToString(left, left_type);
+ } else {
+ ASSERT(right_type->Is(Type::String()));
+ HValue* function = AddLoadJSBuiltin(Builtins::STRING_ADD_RIGHT);
+ Add<HPushArgument>(left);
+ Add<HPushArgument>(right);
+ return NewUncasted<HInvokeFunction>(function, 2);
+ }
+
+ if (right_type->Is(Type::String())) {
+ IfBuilder if_isstring(this);
+ if_isstring.If<HIsStringAndBranch>(right);
+ if_isstring.Then();
+ if_isstring.ElseDeopt("Expected string for RHS of binary operation");
+ } else if (right_type->Is(Type::Number())) {
+ right = BuildNumberToString(right, right_type);
+ } else {
+ ASSERT(left_type->Is(Type::String()));
+ HValue* function = AddLoadJSBuiltin(Builtins::STRING_ADD_LEFT);
+ Add<HPushArgument>(left);
+ Add<HPushArgument>(right);
+ return NewUncasted<HInvokeFunction>(function, 2);
+ }
+
+ return NewUncasted<HStringAdd>(left, right, STRING_ADD_CHECK_NONE);
+ }
if (binop_stub) {
left = EnforceNumberType(left, left_type);
@@ -7859,15 +7888,12 @@
bool is_non_primitive = (left_rep.IsTagged() && !left_rep.IsSmi()) ||
(right_rep.IsTagged() && !right_rep.IsSmi());
- bool is_string_add = op == Token::ADD &&
- (left_type->Is(Type::String()) ||
- right_type->Is(Type::String()));
HInstruction* instr = NULL;
// Only the stub is allowed to call into the runtime, since otherwise we
would
// inline several instructions (including the two pushes) for every
tagged
// operation in optimized code, which is more expensive, than a stub
call.
- if (binop_stub && is_non_primitive && !is_string_add) {
+ if (binop_stub && is_non_primitive) {
HValue* function = AddLoadJSBuiltin(BinaryOpIC::TokenToJSBuiltin(op));
Add<HPushArgument>(left);
Add<HPushArgument>(right);
@@ -7875,23 +7901,7 @@
} else {
switch (op) {
case Token::ADD:
- if (is_string_add) {
- StringAddFlags flags = STRING_ADD_CHECK_BOTH;
- if (left_type->Is(Type::String())) {
- BuildCheckHeapObject(left);
- AddInstruction(HCheckInstanceType::NewIsString(left, zone()));
- flags = STRING_ADD_CHECK_RIGHT;
- }
- if (right_type->Is(Type::String())) {
- BuildCheckHeapObject(right);
- AddInstruction(HCheckInstanceType::NewIsString(right, zone()));
- flags = (flags == STRING_ADD_CHECK_BOTH)
- ? STRING_ADD_CHECK_LEFT : STRING_ADD_CHECK_NONE;
- }
- instr = NewUncasted<HStringAdd>(left, right, flags);
- } else {
- instr = NewUncasted<HAdd>(left, right);
- }
+ instr = NewUncasted<HAdd>(left, right);
break;
case Token::SUB:
instr = NewUncasted<HSub>(left, right);
@@ -9090,7 +9100,8 @@
ASSERT_EQ(1, call->arguments()->length());
CHECK_ALIVE(VisitForValue(call->arguments()->at(0)));
HValue* number = Pop();
- HValue* result = BuildNumberToString(number);
+ HValue* result = BuildNumberToString(
+ number, handle(Type::Number(), isolate()));
return ast_context()->ReturnValue(result);
}
=======================================
--- /branches/bleeding_edge/src/hydrogen.h Wed Oct 16 08:10:36 2013 UTC
+++ /branches/bleeding_edge/src/hydrogen.h Mon Oct 21 12:42:08 2013 UTC
@@ -1234,14 +1234,7 @@
ElementsKind to_kind,
bool is_jsarray);
- // Do lookup in the number string cache. If the object is not found
- // in the cache, the false branch of the continuation is taken;
- // otherwise the true branch is taken and the returned value contains
- // the cache value for the object. The returned value must NOT be used
- // on the false branch.
- HValue* BuildLookupNumberStringCache(HValue* object,
- HIfContinuation* continuation);
- HValue* BuildNumberToString(HValue* number);
+ HValue* BuildNumberToString(HValue* object, Handle<Type> type);
HInstruction* BuildUncheckedMonomorphicElementAccess(
HValue* checked_object,
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Fri Oct 18 14:55:21
2013 UTC
+++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Mon Oct 21 12:42:08
2013 UTC
@@ -70,7 +70,8 @@
static Register registers[] = { eax };
descriptor->register_param_count_ = 1;
descriptor->register_params_ = registers;
- descriptor->deoptimization_handler_ = NULL;
+ descriptor->deoptimization_handler_ =
+ Runtime::FunctionForId(Runtime::kNumberToString)->entry;
}
=======================================
--- /branches/bleeding_edge/src/isolate.cc Fri Oct 4 08:17:11 2013 UTC
+++ /branches/bleeding_edge/src/isolate.cc Mon Oct 21 12:42:08 2013 UTC
@@ -2327,6 +2327,7 @@
ArrayConstructorStubBase::InstallDescriptors(this);
InternalArrayConstructorStubBase::InstallDescriptors(this);
FastNewClosureStub::InstallDescriptors(this);
+ NumberToStringStub::InstallDescriptors(this);
}
if (FLAG_sweeper_threads > 0) {
=======================================
--- /branches/bleeding_edge/src/x64/code-stubs-x64.cc Fri Oct 18 14:55:21
2013 UTC
+++ /branches/bleeding_edge/src/x64/code-stubs-x64.cc Mon Oct 21 12:42:08
2013 UTC
@@ -66,7 +66,8 @@
static Register registers[] = { rax };
descriptor->register_param_count_ = 1;
descriptor->register_params_ = registers;
- descriptor->deoptimization_handler_ = NULL;
+ descriptor->deoptimization_handler_ =
+ Runtime::FunctionForId(Runtime::kNumberToString)->entry;
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
