[interpreter] Add string type feedback to add
Adds string type feedback to Ignition's AddWithFeedback code stub, for now only adding a special case for when both lhs and rhs are strings. This improves octane's splay by >100%. BUG=v8:5400 Review-Url: https://codereview.chromium.org/2392533002 Cr-Commit-Position: refs/heads/master@{#39987}
This commit is contained in:
leszeks
Commit bot
parent
8b682e1a5a
commit
fb4ae2239d
@ -1032,7 +1032,7 @@ compiler::Node* AddWithFeedbackStub::Generate(
|
||||
|
||||
// Shared entry for floating point addition.
|
||||
Label do_fadd(assembler), end(assembler),
|
||||
call_add_stub(assembler, Label::kDeferred);
|
||||
do_add_any(assembler, Label::kDeferred), call_add_stub(assembler);
|
||||
Variable var_fadd_lhs(assembler, MachineRepresentation::kFloat64),
|
||||
var_fadd_rhs(assembler, MachineRepresentation::kFloat64),
|
||||
var_type_feedback(assembler, MachineRepresentation::kWord32),
|
||||
@ -1080,8 +1080,7 @@ compiler::Node* AddWithFeedbackStub::Generate(
|
||||
Node* rhs_map = assembler->LoadMap(rhs);
|
||||
|
||||
// Check if the {rhs} is a HeapNumber.
|
||||
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map),
|
||||
&call_add_stub);
|
||||
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), &do_add_any);
|
||||
|
||||
var_fadd_lhs.Bind(assembler->SmiToFloat64(lhs));
|
||||
var_fadd_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
|
||||
@ -1091,12 +1090,14 @@ compiler::Node* AddWithFeedbackStub::Generate(
|
||||
|
||||
assembler->Bind(&if_lhsisnotsmi);
|
||||
{
|
||||
Label check_string(assembler);
|
||||
|
||||
// Load the map of {lhs}.
|
||||
Node* lhs_map = assembler->LoadMap(lhs);
|
||||
|
||||
// Check if {lhs} is a HeapNumber.
|
||||
Label if_lhsisnumber(assembler), if_lhsisnotnumber(assembler);
|
||||
assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), &call_add_stub);
|
||||
assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), &check_string);
|
||||
|
||||
// Check if the {rhs} is Smi.
|
||||
Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler);
|
||||
@ -1115,13 +1116,34 @@ compiler::Node* AddWithFeedbackStub::Generate(
|
||||
Node* rhs_map = assembler->LoadMap(rhs);
|
||||
|
||||
// Check if the {rhs} is a HeapNumber.
|
||||
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map),
|
||||
&call_add_stub);
|
||||
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), &do_add_any);
|
||||
|
||||
var_fadd_lhs.Bind(assembler->LoadHeapNumberValue(lhs));
|
||||
var_fadd_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
|
||||
assembler->Goto(&do_fadd);
|
||||
}
|
||||
|
||||
assembler->Bind(&check_string);
|
||||
{
|
||||
// Check if the {rhs} is a smi, and exit the string check early if it is.
|
||||
assembler->GotoIf(assembler->WordIsSmi(rhs), &do_add_any);
|
||||
|
||||
Node* lhs_instance_type = assembler->LoadMapInstanceType(lhs_map);
|
||||
|
||||
// Exit unless {lhs} is a string
|
||||
assembler->GotoUnless(assembler->IsStringInstanceType(lhs_instance_type),
|
||||
&do_add_any);
|
||||
|
||||
Node* rhs_instance_type = assembler->LoadInstanceType(rhs);
|
||||
|
||||
// Exit unless {rhs} is a string
|
||||
assembler->GotoUnless(assembler->IsStringInstanceType(rhs_instance_type),
|
||||
&do_add_any);
|
||||
|
||||
var_type_feedback.Bind(
|
||||
assembler->Int32Constant(BinaryOperationFeedback::kString));
|
||||
assembler->Goto(&call_add_stub);
|
||||
}
|
||||
}
|
||||
|
||||
assembler->Bind(&do_fadd);
|
||||
@ -1135,10 +1157,15 @@ compiler::Node* AddWithFeedbackStub::Generate(
|
||||
assembler->Goto(&end);
|
||||
}
|
||||
|
||||
assembler->Bind(&call_add_stub);
|
||||
assembler->Bind(&do_add_any);
|
||||
{
|
||||
var_type_feedback.Bind(
|
||||
assembler->Int32Constant(BinaryOperationFeedback::kAny));
|
||||
assembler->Goto(&call_add_stub);
|
||||
}
|
||||
|
||||
assembler->Bind(&call_add_stub);
|
||||
{
|
||||
Callable callable = CodeFactory::Add(assembler->isolate());
|
||||
var_result.Bind(assembler->CallStub(callable, context, lhs, rhs));
|
||||
assembler->Goto(&end);
|
||||
|
||||
@ -1209,9 +1209,16 @@ inline uint32_t ObjectHash(Address address) {
|
||||
// at different points by performing an 'OR' operation. Type feedback moves
|
||||
// to a more generic type when we combine feedback.
|
||||
// kSignedSmall -> kNumber -> kAny
|
||||
// kString -> kAny
|
||||
class BinaryOperationFeedback {
|
||||
public:
|
||||
enum { kNone = 0x00, kSignedSmall = 0x01, kNumber = 0x3, kAny = 0x7 };
|
||||
enum {
|
||||
kNone = 0x0,
|
||||
kSignedSmall = 0x1,
|
||||
kNumber = 0x3,
|
||||
kString = 0x4,
|
||||
kAny = 0xF
|
||||
};
|
||||
};
|
||||
|
||||
// TODO(epertoso): consider unifying this with BinaryOperationFeedback.
|
||||
|
||||
@ -129,6 +129,8 @@ BinaryOperationHint BinaryOperationHintFromFeedback(int type_feedback) {
|
||||
return BinaryOperationHint::kSignedSmall;
|
||||
case BinaryOperationFeedback::kNumber:
|
||||
return BinaryOperationHint::kNumberOrOddball;
|
||||
case BinaryOperationFeedback::kString:
|
||||
return BinaryOperationHint::kString;
|
||||
case BinaryOperationFeedback::kAny:
|
||||
default:
|
||||
return BinaryOperationHint::kAny;
|
||||
|
||||
@ -386,28 +386,36 @@ TEST(InterpreterStringAdd) {
|
||||
Handle<Object> lhs;
|
||||
Handle<Object> rhs;
|
||||
Handle<Object> expected_value;
|
||||
int32_t expected_feedback;
|
||||
} test_cases[] = {
|
||||
{factory->NewStringFromStaticChars("a"),
|
||||
factory->NewStringFromStaticChars("b"),
|
||||
factory->NewStringFromStaticChars("ab")},
|
||||
factory->NewStringFromStaticChars("ab"),
|
||||
BinaryOperationFeedback::kString},
|
||||
{factory->NewStringFromStaticChars("aaaaaa"),
|
||||
factory->NewStringFromStaticChars("b"),
|
||||
factory->NewStringFromStaticChars("aaaaaab")},
|
||||
factory->NewStringFromStaticChars("aaaaaab"),
|
||||
BinaryOperationFeedback::kString},
|
||||
{factory->NewStringFromStaticChars("aaa"),
|
||||
factory->NewStringFromStaticChars("bbbbb"),
|
||||
factory->NewStringFromStaticChars("aaabbbbb")},
|
||||
factory->NewStringFromStaticChars("aaabbbbb"),
|
||||
BinaryOperationFeedback::kString},
|
||||
{factory->NewStringFromStaticChars(""),
|
||||
factory->NewStringFromStaticChars("b"),
|
||||
factory->NewStringFromStaticChars("b")},
|
||||
factory->NewStringFromStaticChars("b"),
|
||||
BinaryOperationFeedback::kString},
|
||||
{factory->NewStringFromStaticChars("a"),
|
||||
factory->NewStringFromStaticChars(""),
|
||||
factory->NewStringFromStaticChars("a")},
|
||||
factory->NewStringFromStaticChars("a"),
|
||||
BinaryOperationFeedback::kString},
|
||||
{factory->NewStringFromStaticChars("1.11"), factory->NewHeapNumber(2.5),
|
||||
factory->NewStringFromStaticChars("1.112.5")},
|
||||
factory->NewStringFromStaticChars("1.112.5"),
|
||||
BinaryOperationFeedback::kAny},
|
||||
{factory->NewStringFromStaticChars("-1.11"), factory->NewHeapNumber(2.56),
|
||||
factory->NewStringFromStaticChars("-1.112.56")},
|
||||
factory->NewStringFromStaticChars("-1.112.56"),
|
||||
BinaryOperationFeedback::kAny},
|
||||
{factory->NewStringFromStaticChars(""), factory->NewHeapNumber(2.5),
|
||||
factory->NewStringFromStaticChars("2.5")},
|
||||
factory->NewStringFromStaticChars("2.5"), BinaryOperationFeedback::kAny},
|
||||
};
|
||||
|
||||
for (size_t i = 0; i < arraysize(test_cases); i++) {
|
||||
@ -429,6 +437,11 @@ TEST(InterpreterStringAdd) {
|
||||
auto callable = tester.GetCallable<>();
|
||||
Handle<Object> return_value = callable().ToHandleChecked();
|
||||
CHECK(return_value->SameValue(*test_cases[i].expected_value));
|
||||
|
||||
Object* feedback = vector->Get(slot);
|
||||
CHECK(feedback->IsSmi());
|
||||
CHECK_EQ(test_cases[i].expected_feedback,
|
||||
static_cast<Smi*>(feedback)->value());
|
||||
}
|
||||
}
|
||||
|
||||
@ -1703,7 +1716,7 @@ TEST(InterpreterSmiComparisons) {
|
||||
CompareC(comparison, inputs[i], inputs[j]));
|
||||
Object* feedback = vector->Get(slot);
|
||||
CHECK(feedback->IsSmi());
|
||||
CHECK_EQ(BinaryOperationFeedback::kSignedSmall,
|
||||
CHECK_EQ(CompareOperationFeedback::kSignedSmall,
|
||||
static_cast<Smi*>(feedback)->value());
|
||||
}
|
||||
}
|
||||
@ -1750,7 +1763,7 @@ TEST(InterpreterHeapNumberComparisons) {
|
||||
CompareC(comparison, inputs[i], inputs[j]));
|
||||
Object* feedback = vector->Get(slot);
|
||||
CHECK(feedback->IsSmi());
|
||||
CHECK_EQ(BinaryOperationFeedback::kNumber,
|
||||
CHECK_EQ(CompareOperationFeedback::kNumber,
|
||||
static_cast<Smi*>(feedback)->value());
|
||||
}
|
||||
}
|
||||
@ -1796,7 +1809,7 @@ TEST(InterpreterStringComparisons) {
|
||||
CompareC(comparison, inputs[i], inputs[j]));
|
||||
Object* feedback = vector->Get(slot);
|
||||
CHECK(feedback->IsSmi());
|
||||
CHECK_EQ(BinaryOperationFeedback::kAny,
|
||||
CHECK_EQ(CompareOperationFeedback::kAny,
|
||||
static_cast<Smi*>(feedback)->value());
|
||||
}
|
||||
}
|
||||
@ -1862,7 +1875,7 @@ TEST(InterpreterMixedComparisons) {
|
||||
CompareC(comparison, lhs, rhs, true));
|
||||
Object* feedback = vector->Get(slot);
|
||||
CHECK(feedback->IsSmi());
|
||||
CHECK_EQ(BinaryOperationFeedback::kAny,
|
||||
CHECK_EQ(CompareOperationFeedback::kAny,
|
||||
static_cast<Smi*>(feedback)->value());
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user