Skip to content

Commit 55b4df7

Browse files
bmeurerCommit bot
bmeurer
authored and
Commit bot
committed
[runtime] Unify comparison operator runtime entries.
Only use one set of %StrictEquals/%StrictNotEquals and %Equals/%NotEquals runtime entries for both the interpreter and the old-style CompareICStub. The long-term plan is to update the CompareICStub to also return boolean values, and even allow some more code sharing with the interpreter there. [email protected] Review URL: https://codereview.chromium.org/1738883002 Cr-Commit-Position: refs/heads/master@{#34303}
1 parent 01b8fc8 commit 55b4df7

12 files changed

Lines changed: 122 additions & 127 deletions

File tree

‎src/arm/code-stubs-arm.cc‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -671,11 +671,19 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) {
671671

672672
__ bind(&slow);
673673

674-
__ Push(lhs, rhs);
675-
// Figure out which native to call and setup the arguments.
676674
if (cc == eq) {
677-
__ TailCallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
675+
{
676+
FrameAndConstantPoolScope scope(masm, StackFrame::INTERNAL);
677+
__ Push(lhs, rhs);
678+
__ CallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
679+
}
680+
// Turn true into 0 and false into some non-zero value.
681+
STATIC_ASSERT(EQUAL == 0);
682+
__ LoadRoot(r1, Heap::kTrueValueRootIndex);
683+
__ sub(r0, r0, r1);
684+
__ Ret();
678685
} else {
686+
__ Push(lhs, rhs);
679687
int ncr; // NaN compare result
680688
if (cc == lt || cc == le) {
681689
ncr = GREATER;

‎src/arm64/code-stubs-arm64.cc‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -628,11 +628,19 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) {
628628

629629
__ Bind(&slow);
630630

631-
__ Push(lhs, rhs);
632-
// Figure out which native to call and setup the arguments.
633631
if (cond == eq) {
634-
__ TailCallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
632+
{
633+
FrameScope scope(masm, StackFrame::INTERNAL);
634+
__ Push(lhs, rhs);
635+
__ CallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
636+
}
637+
// Turn true into 0 and false into some non-zero value.
638+
STATIC_ASSERT(EQUAL == 0);
639+
__ LoadRoot(x1, Heap::kTrueValueRootIndex);
640+
__ Sub(x0, x0, x1);
641+
__ Ret();
635642
} else {
643+
__ Push(lhs, rhs);
636644
int ncr; // NaN compare result
637645
if ((cond == lt) || (cond == le)) {
638646
ncr = GREATER;

‎src/ia32/code-stubs-ia32.cc‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,21 +1442,24 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) {
14421442
}
14431443
__ bind(&runtime_call);
14441444

1445-
// Push arguments below the return address.
1446-
__ pop(ecx);
1447-
__ push(edx);
1448-
__ push(eax);
1449-
1450-
// Figure out which native to call and setup the arguments.
14511445
if (cc == equal) {
1452-
__ push(ecx);
1453-
__ TailCallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
1446+
{
1447+
FrameScope scope(masm, StackFrame::INTERNAL);
1448+
__ Push(edx);
1449+
__ Push(eax);
1450+
__ CallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
1451+
}
1452+
// Turn true into 0 and false into some non-zero value.
1453+
STATIC_ASSERT(EQUAL == 0);
1454+
__ sub(eax, Immediate(isolate()->factory()->true_value()));
1455+
__ Ret();
14541456
} else {
1457+
// Push arguments below the return address.
1458+
__ pop(ecx);
1459+
__ push(edx);
1460+
__ push(eax);
14551461
__ push(Immediate(Smi::FromInt(NegativeComparisonResult(cc))));
1456-
1457-
// Restore return address on the stack.
14581462
__ push(ecx);
1459-
14601463
// Call the native; it returns -1 (less), 0 (equal), or 1 (greater)
14611464
// tagged as a small integer.
14621465
__ TailCallRuntime(Runtime::kCompare);

‎src/interpreter/interpreter.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,23 +1144,23 @@ void Interpreter::DoNewWide(InterpreterAssembler* assembler) {
11441144
//
11451145
// Test if the value in the <src> register equals the accumulator.
11461146
void Interpreter::DoTestEqual(InterpreterAssembler* assembler) {
1147-
DoBinaryOp(Runtime::kInterpreterEquals, assembler);
1147+
DoBinaryOp(Runtime::kEquals, assembler);
11481148
}
11491149

11501150

11511151
// TestNotEqual <src>
11521152
//
11531153
// Test if the value in the <src> register is not equal to the accumulator.
11541154
void Interpreter::DoTestNotEqual(InterpreterAssembler* assembler) {
1155-
DoBinaryOp(Runtime::kInterpreterNotEquals, assembler);
1155+
DoBinaryOp(Runtime::kNotEquals, assembler);
11561156
}
11571157

11581158

11591159
// TestEqualStrict <src>
11601160
//
11611161
// Test if the value in the <src> register is strictly equal to the accumulator.
11621162
void Interpreter::DoTestEqualStrict(InterpreterAssembler* assembler) {
1163-
DoBinaryOp(Runtime::kInterpreterStrictEquals, assembler);
1163+
DoBinaryOp(Runtime::kStrictEquals, assembler);
11641164
}
11651165

11661166

@@ -1169,7 +1169,7 @@ void Interpreter::DoTestEqualStrict(InterpreterAssembler* assembler) {
11691169
// Test if the value in the <src> register is not strictly equal to the
11701170
// accumulator.
11711171
void Interpreter::DoTestNotEqualStrict(InterpreterAssembler* assembler) {
1172-
DoBinaryOp(Runtime::kInterpreterStrictNotEquals, assembler);
1172+
DoBinaryOp(Runtime::kStrictNotEquals, assembler);
11731173
}
11741174

11751175

‎src/mips/code-stubs-mips.cc‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -718,13 +718,21 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) {
718718
// Never falls through to here.
719719

720720
__ bind(&slow);
721-
// Prepare for call to builtin. Push object pointers, a0 (lhs) first,
722-
// a1 (rhs) second.
723-
__ Push(lhs, rhs);
724-
// Figure out which native to call and setup the arguments.
725721
if (cc == eq) {
726-
__ TailCallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
722+
{
723+
FrameScope scope(masm, StackFrame::INTERNAL);
724+
__ Push(lhs, rhs);
725+
__ CallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
726+
}
727+
// Turn true into 0 and false into some non-zero value.
728+
STATIC_ASSERT(EQUAL == 0);
729+
__ LoadRoot(a0, Heap::kTrueValueRootIndex);
730+
__ Ret(USE_DELAY_SLOT);
731+
__ subu(v0, v0, a0); // In delay slot.
727732
} else {
733+
// Prepare for call to builtin. Push object pointers, a0 (lhs) first,
734+
// a1 (rhs) second.
735+
__ Push(lhs, rhs);
728736
int ncr; // NaN compare result.
729737
if (cc == lt || cc == le) {
730738
ncr = GREATER;

‎src/mips64/code-stubs-mips64.cc‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -715,13 +715,21 @@ void CompareICStub::GenerateGeneric(MacroAssembler* masm) {
715715
// Never falls through to here.
716716

717717
__ bind(&slow);
718-
// Prepare for call to builtin. Push object pointers, a0 (lhs) first,
719-
// a1 (rhs) second.
720-
__ Push(lhs, rhs);
721-
// Figure out which native to call and setup the arguments.
722718
if (cc == eq) {
723-
__ TailCallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
719+
{
720+
FrameScope scope(masm, StackFrame::INTERNAL);
721+
__ Push(lhs, rhs);
722+
__ CallRuntime(strict() ? Runtime::kStrictEquals : Runtime::kEquals);
723+
}
724+
// Turn true into 0 and false into some non-zero value.
725+
STATIC_ASSERT(EQUAL == 0);
726+
__ LoadRoot(a0, Heap::kTrueValueRootIndex);
727+
__ Ret(USE_DELAY_SLOT);
728+
__ subu(v0, v0, a0); // In delay slot.
724729
} else {
730+
// Prepare for call to builtin. Push object pointers, a0 (lhs) first,
731+
// a1 (rhs) second.
732+
__ Push(lhs, rhs);
725733
int ncr; // NaN compare result.
726734
if (cc == lt || cc == le) {
727735
ncr = GREATER;

‎src/runtime/runtime-interpreter.cc‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,6 @@
1616
namespace v8 {
1717
namespace internal {
1818

19-
20-
RUNTIME_FUNCTION(Runtime_InterpreterEquals) {
21-
HandleScope scope(isolate);
22-
DCHECK_EQ(2, args.length());
23-
CONVERT_ARG_HANDLE_CHECKED(Object, x, 0);
24-
CONVERT_ARG_HANDLE_CHECKED(Object, y, 1);
25-
Maybe<bool> result = Object::Equals(x, y);
26-
if (result.IsJust()) {
27-
return isolate->heap()->ToBoolean(result.FromJust());
28-
} else {
29-
return isolate->heap()->exception();
30-
}
31-
}
32-
33-
34-
RUNTIME_FUNCTION(Runtime_InterpreterNotEquals) {
35-
HandleScope scope(isolate);
36-
DCHECK_EQ(2, args.length());
37-
CONVERT_ARG_HANDLE_CHECKED(Object, x, 0);
38-
CONVERT_ARG_HANDLE_CHECKED(Object, y, 1);
39-
Maybe<bool> result = Object::Equals(x, y);
40-
if (result.IsJust()) {
41-
return isolate->heap()->ToBoolean(!result.FromJust());
42-
} else {
43-
return isolate->heap()->exception();
44-
}
45-
}
46-
47-
4819
RUNTIME_FUNCTION(Runtime_InterpreterLessThan) {
4920
HandleScope scope(isolate);
5021
DCHECK_EQ(2, args.length());
@@ -101,24 +72,6 @@ RUNTIME_FUNCTION(Runtime_InterpreterGreaterThanOrEqual) {
10172
}
10273

10374

104-
RUNTIME_FUNCTION(Runtime_InterpreterStrictEquals) {
105-
SealHandleScope shs(isolate);
106-
DCHECK_EQ(2, args.length());
107-
CONVERT_ARG_CHECKED(Object, x, 0);
108-
CONVERT_ARG_CHECKED(Object, y, 1);
109-
return isolate->heap()->ToBoolean(x->StrictEquals(y));
110-
}
111-
112-
113-
RUNTIME_FUNCTION(Runtime_InterpreterStrictNotEquals) {
114-
SealHandleScope shs(isolate);
115-
DCHECK_EQ(2, args.length());
116-
CONVERT_ARG_CHECKED(Object, x, 0);
117-
CONVERT_ARG_CHECKED(Object, y, 1);
118-
return isolate->heap()->ToBoolean(!x->StrictEquals(y));
119-
}
120-
121-
12275
RUNTIME_FUNCTION(Runtime_InterpreterToBoolean) {
12376
SealHandleScope shs(isolate);
12477
DCHECK_EQ(1, args.length());

‎src/runtime/runtime-object.cc‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,28 +1073,6 @@ RUNTIME_FUNCTION(Runtime_ToName) {
10731073
}
10741074

10751075

1076-
RUNTIME_FUNCTION(Runtime_Equals) {
1077-
HandleScope scope(isolate);
1078-
DCHECK_EQ(2, args.length());
1079-
CONVERT_ARG_HANDLE_CHECKED(Object, x, 0);
1080-
CONVERT_ARG_HANDLE_CHECKED(Object, y, 1);
1081-
Maybe<bool> result = Object::Equals(x, y);
1082-
if (!result.IsJust()) return isolate->heap()->exception();
1083-
// TODO(bmeurer): Change this at some point to return true/false instead.
1084-
return Smi::FromInt(result.FromJust() ? EQUAL : NOT_EQUAL);
1085-
}
1086-
1087-
1088-
RUNTIME_FUNCTION(Runtime_StrictEquals) {
1089-
SealHandleScope scope(isolate);
1090-
DCHECK_EQ(2, args.length());
1091-
CONVERT_ARG_CHECKED(Object, x, 0);
1092-
CONVERT_ARG_CHECKED(Object, y, 1);
1093-
// TODO(bmeurer): Change this at some point to return true/false instead.
1094-
return Smi::FromInt(x->StrictEquals(y) ? EQUAL : NOT_EQUAL);
1095-
}
1096-
1097-
10981076
RUNTIME_FUNCTION(Runtime_SameValue) {
10991077
SealHandleScope scope(isolate);
11001078
DCHECK_EQ(2, args.length());

‎src/runtime/runtime-operators.cc‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,5 +140,41 @@ RUNTIME_FUNCTION(Runtime_BitwiseXor) {
140140
return *result;
141141
}
142142

143+
RUNTIME_FUNCTION(Runtime_Equals) {
144+
HandleScope scope(isolate);
145+
DCHECK_EQ(2, args.length());
146+
CONVERT_ARG_HANDLE_CHECKED(Object, x, 0);
147+
CONVERT_ARG_HANDLE_CHECKED(Object, y, 1);
148+
Maybe<bool> result = Object::Equals(x, y);
149+
if (!result.IsJust()) return isolate->heap()->exception();
150+
return isolate->heap()->ToBoolean(result.FromJust());
151+
}
152+
153+
RUNTIME_FUNCTION(Runtime_NotEquals) {
154+
HandleScope scope(isolate);
155+
DCHECK_EQ(2, args.length());
156+
CONVERT_ARG_HANDLE_CHECKED(Object, x, 0);
157+
CONVERT_ARG_HANDLE_CHECKED(Object, y, 1);
158+
Maybe<bool> result = Object::Equals(x, y);
159+
if (!result.IsJust()) return isolate->heap()->exception();
160+
return isolate->heap()->ToBoolean(!result.FromJust());
161+
}
162+
163+
RUNTIME_FUNCTION(Runtime_StrictEquals) {
164+
SealHandleScope scope(isolate);
165+
DCHECK_EQ(2, args.length());
166+
CONVERT_ARG_CHECKED(Object, x, 0);
167+
CONVERT_ARG_CHECKED(Object, y, 1);
168+
return isolate->heap()->ToBoolean(x->StrictEquals(y));
169+
}
170+
171+
RUNTIME_FUNCTION(Runtime_StrictNotEquals) {
172+
SealHandleScope scope(isolate);
173+
DCHECK_EQ(2, args.length());
174+
CONVERT_ARG_CHECKED(Object, x, 0);
175+
CONVERT_ARG_CHECKED(Object, y, 1);
176+
return isolate->heap()->ToBoolean(!x->StrictEquals(y));
177+
}
178+
143179
} // namespace internal
144180
} // namespace v8

‎src/runtime/runtime.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,6 @@ namespace internal {
206206
F(ForInStep, 1, 1)
207207

208208
#define FOR_EACH_INTRINSIC_INTERPRETER(F) \
209-
F(InterpreterEquals, 2, 1) \
210-
F(InterpreterNotEquals, 2, 1) \
211-
F(InterpreterStrictEquals, 2, 1) \
212-
F(InterpreterStrictNotEquals, 2, 1) \
213209
F(InterpreterLessThan, 2, 1) \
214210
F(InterpreterGreaterThan, 2, 1) \
215211
F(InterpreterLessThanOrEqual, 2, 1) \
@@ -455,8 +451,6 @@ namespace internal {
455451
F(ToLength, 1, 1) \
456452
F(ToString, 1, 1) \
457453
F(ToName, 1, 1) \
458-
F(Equals, 2, 1) \
459-
F(StrictEquals, 2, 1) \
460454
F(SameValue, 2, 1) \
461455
F(SameValueZero, 2, 1) \
462456
F(Compare, 3, 1) \
@@ -491,7 +485,11 @@ namespace internal {
491485
F(ShiftRightLogical, 2, 1) \
492486
F(BitwiseAnd, 2, 1) \
493487
F(BitwiseOr, 2, 1) \
494-
F(BitwiseXor, 2, 1)
488+
F(BitwiseXor, 2, 1) \
489+
F(Equals, 2, 1) \
490+
F(NotEquals, 2, 1) \
491+
F(StrictEquals, 2, 1) \
492+
F(StrictNotEquals, 2, 1)
495493

496494
#define FOR_EACH_INTRINSIC_PROXY(F) \
497495
F(IsJSProxy, 1, 1) \

0 commit comments

Comments
 (0)