Skip to content

Commit b3c8b0c

Browse files
LeszekSwirskiCommit bot
LeszekSwirski
authored and
Commit bot
committed
[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 Committed: https://crrev.com/fb4ae2239d37adaf0321165034050316914de708 Committed: https://crrev.com/bf1a94f1b269914856a8c8763fd282367f066c67 Review-Url: https://codereview.chromium.org/2392533002 Cr-Original-Original-Commit-Position: refs/heads/master@{#39987} Cr-Original-Commit-Position: refs/heads/master@{#39996} Cr-Commit-Position: refs/heads/master@{#40015}
1 parent 45b64d1 commit b3c8b0c

4 files changed

Lines changed: 69 additions & 20 deletions

File tree

‎src/code-stubs.cc‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,7 +1034,7 @@ compiler::Node* AddWithFeedbackStub::Generate(
10341034

10351035
// Shared entry for floating point addition.
10361036
Label do_fadd(assembler), end(assembler),
1037-
call_add_stub(assembler, Label::kDeferred);
1037+
do_add_any(assembler, Label::kDeferred), call_add_stub(assembler);
10381038
Variable var_fadd_lhs(assembler, MachineRepresentation::kFloat64),
10391039
var_fadd_rhs(assembler, MachineRepresentation::kFloat64),
10401040
var_type_feedback(assembler, MachineRepresentation::kWord32),
@@ -1082,8 +1082,7 @@ compiler::Node* AddWithFeedbackStub::Generate(
10821082
Node* rhs_map = assembler->LoadMap(rhs);
10831083

10841084
// Check if the {rhs} is a HeapNumber.
1085-
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map),
1086-
&call_add_stub);
1085+
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), &do_add_any);
10871086

10881087
var_fadd_lhs.Bind(assembler->SmiToFloat64(lhs));
10891088
var_fadd_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
@@ -1093,12 +1092,14 @@ compiler::Node* AddWithFeedbackStub::Generate(
10931092

10941093
assembler->Bind(&if_lhsisnotsmi);
10951094
{
1095+
Label check_string(assembler);
1096+
10961097
// Load the map of {lhs}.
10971098
Node* lhs_map = assembler->LoadMap(lhs);
10981099

10991100
// Check if {lhs} is a HeapNumber.
11001101
Label if_lhsisnumber(assembler), if_lhsisnotnumber(assembler);
1101-
assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), &call_add_stub);
1102+
assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), &check_string);
11021103

11031104
// Check if the {rhs} is Smi.
11041105
Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler);
@@ -1117,13 +1118,34 @@ compiler::Node* AddWithFeedbackStub::Generate(
11171118
Node* rhs_map = assembler->LoadMap(rhs);
11181119

11191120
// Check if the {rhs} is a HeapNumber.
1120-
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map),
1121-
&call_add_stub);
1121+
assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), &do_add_any);
11221122

11231123
var_fadd_lhs.Bind(assembler->LoadHeapNumberValue(lhs));
11241124
var_fadd_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
11251125
assembler->Goto(&do_fadd);
11261126
}
1127+
1128+
assembler->Bind(&check_string);
1129+
{
1130+
// Check if the {rhs} is a smi, and exit the string check early if it is.
1131+
assembler->GotoIf(assembler->WordIsSmi(rhs), &do_add_any);
1132+
1133+
Node* lhs_instance_type = assembler->LoadMapInstanceType(lhs_map);
1134+
1135+
// Exit unless {lhs} is a string
1136+
assembler->GotoUnless(assembler->IsStringInstanceType(lhs_instance_type),
1137+
&do_add_any);
1138+
1139+
Node* rhs_instance_type = assembler->LoadInstanceType(rhs);
1140+
1141+
// Exit unless {rhs} is a string
1142+
assembler->GotoUnless(assembler->IsStringInstanceType(rhs_instance_type),
1143+
&do_add_any);
1144+
1145+
var_type_feedback.Bind(
1146+
assembler->Int32Constant(BinaryOperationFeedback::kString));
1147+
assembler->Goto(&call_add_stub);
1148+
}
11271149
}
11281150

11291151
assembler->Bind(&do_fadd);
@@ -1137,10 +1159,15 @@ compiler::Node* AddWithFeedbackStub::Generate(
11371159
assembler->Goto(&end);
11381160
}
11391161

1140-
assembler->Bind(&call_add_stub);
1162+
assembler->Bind(&do_add_any);
11411163
{
11421164
var_type_feedback.Bind(
11431165
assembler->Int32Constant(BinaryOperationFeedback::kAny));
1166+
assembler->Goto(&call_add_stub);
1167+
}
1168+
1169+
assembler->Bind(&call_add_stub);
1170+
{
11441171
Callable callable = CodeFactory::Add(assembler->isolate());
11451172
var_result.Bind(assembler->CallStub(callable, context, lhs, rhs));
11461173
assembler->Goto(&end);

‎src/globals.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1209,9 +1209,16 @@ inline uint32_t ObjectHash(Address address) {
12091209
// at different points by performing an 'OR' operation. Type feedback moves
12101210
// to a more generic type when we combine feedback.
12111211
// kSignedSmall -> kNumber -> kAny
1212+
// kString -> kAny
12121213
class BinaryOperationFeedback {
12131214
public:
1214-
enum { kNone = 0x00, kSignedSmall = 0x01, kNumber = 0x3, kAny = 0x7 };
1215+
enum {
1216+
kNone = 0x0,
1217+
kSignedSmall = 0x1,
1218+
kNumber = 0x3,
1219+
kString = 0x4,
1220+
kAny = 0xF
1221+
};
12151222
};
12161223

12171224
// TODO(epertoso): consider unifying this with BinaryOperationFeedback.

‎src/type-feedback-vector-inl.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ BinaryOperationHint BinaryOperationHintFromFeedback(int type_feedback) {
129129
return BinaryOperationHint::kSignedSmall;
130130
case BinaryOperationFeedback::kNumber:
131131
return BinaryOperationHint::kNumberOrOddball;
132+
case BinaryOperationFeedback::kString:
133+
return BinaryOperationHint::kString;
132134
case BinaryOperationFeedback::kAny:
133135
default:
134136
return BinaryOperationHint::kAny;

‎test/cctest/interpreter/test-interpreter.cc‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -386,28 +386,36 @@ TEST(InterpreterStringAdd) {
386386
Handle<Object> lhs;
387387
Handle<Object> rhs;
388388
Handle<Object> expected_value;
389+
int32_t expected_feedback;
389390
} test_cases[] = {
390391
{factory->NewStringFromStaticChars("a"),
391392
factory->NewStringFromStaticChars("b"),
392-
factory->NewStringFromStaticChars("ab")},
393+
factory->NewStringFromStaticChars("ab"),
394+
BinaryOperationFeedback::kString},
393395
{factory->NewStringFromStaticChars("aaaaaa"),
394396
factory->NewStringFromStaticChars("b"),
395-
factory->NewStringFromStaticChars("aaaaaab")},
397+
factory->NewStringFromStaticChars("aaaaaab"),
398+
BinaryOperationFeedback::kString},
396399
{factory->NewStringFromStaticChars("aaa"),
397400
factory->NewStringFromStaticChars("bbbbb"),
398-
factory->NewStringFromStaticChars("aaabbbbb")},
401+
factory->NewStringFromStaticChars("aaabbbbb"),
402+
BinaryOperationFeedback::kString},
399403
{factory->NewStringFromStaticChars(""),
400404
factory->NewStringFromStaticChars("b"),
401-
factory->NewStringFromStaticChars("b")},
405+
factory->NewStringFromStaticChars("b"),
406+
BinaryOperationFeedback::kString},
402407
{factory->NewStringFromStaticChars("a"),
403408
factory->NewStringFromStaticChars(""),
404-
factory->NewStringFromStaticChars("a")},
409+
factory->NewStringFromStaticChars("a"),
410+
BinaryOperationFeedback::kString},
405411
{factory->NewStringFromStaticChars("1.11"), factory->NewHeapNumber(2.5),
406-
factory->NewStringFromStaticChars("1.112.5")},
412+
factory->NewStringFromStaticChars("1.112.5"),
413+
BinaryOperationFeedback::kAny},
407414
{factory->NewStringFromStaticChars("-1.11"), factory->NewHeapNumber(2.56),
408-
factory->NewStringFromStaticChars("-1.112.56")},
415+
factory->NewStringFromStaticChars("-1.112.56"),
416+
BinaryOperationFeedback::kAny},
409417
{factory->NewStringFromStaticChars(""), factory->NewHeapNumber(2.5),
410-
factory->NewStringFromStaticChars("2.5")},
418+
factory->NewStringFromStaticChars("2.5"), BinaryOperationFeedback::kAny},
411419
};
412420

413421
for (size_t i = 0; i < arraysize(test_cases); i++) {
@@ -429,6 +437,11 @@ TEST(InterpreterStringAdd) {
429437
auto callable = tester.GetCallable<>();
430438
Handle<Object> return_value = callable().ToHandleChecked();
431439
CHECK(return_value->SameValue(*test_cases[i].expected_value));
440+
441+
Object* feedback = vector->Get(slot);
442+
CHECK(feedback->IsSmi());
443+
CHECK_EQ(test_cases[i].expected_feedback,
444+
static_cast<Smi*>(feedback)->value());
432445
}
433446
}
434447

@@ -1703,7 +1716,7 @@ TEST(InterpreterSmiComparisons) {
17031716
CompareC(comparison, inputs[i], inputs[j]));
17041717
Object* feedback = vector->Get(slot);
17051718
CHECK(feedback->IsSmi());
1706-
CHECK_EQ(BinaryOperationFeedback::kSignedSmall,
1719+
CHECK_EQ(CompareOperationFeedback::kSignedSmall,
17071720
static_cast<Smi*>(feedback)->value());
17081721
}
17091722
}
@@ -1750,7 +1763,7 @@ TEST(InterpreterHeapNumberComparisons) {
17501763
CompareC(comparison, inputs[i], inputs[j]));
17511764
Object* feedback = vector->Get(slot);
17521765
CHECK(feedback->IsSmi());
1753-
CHECK_EQ(BinaryOperationFeedback::kNumber,
1766+
CHECK_EQ(CompareOperationFeedback::kNumber,
17541767
static_cast<Smi*>(feedback)->value());
17551768
}
17561769
}
@@ -1796,7 +1809,7 @@ TEST(InterpreterStringComparisons) {
17961809
CompareC(comparison, inputs[i], inputs[j]));
17971810
Object* feedback = vector->Get(slot);
17981811
CHECK(feedback->IsSmi());
1799-
CHECK_EQ(BinaryOperationFeedback::kAny,
1812+
CHECK_EQ(CompareOperationFeedback::kAny,
18001813
static_cast<Smi*>(feedback)->value());
18011814
}
18021815
}
@@ -1862,7 +1875,7 @@ TEST(InterpreterMixedComparisons) {
18621875
CompareC(comparison, lhs, rhs, true));
18631876
Object* feedback = vector->Get(slot);
18641877
CHECK(feedback->IsSmi());
1865-
CHECK_EQ(BinaryOperationFeedback::kAny,
1878+
CHECK_EQ(CompareOperationFeedback::kAny,
18661879
static_cast<Smi*>(feedback)->value());
18671880
}
18681881
}

0 commit comments

Comments
 (0)