From e56f265f6d41fadbcea2be65d9b573bad8b4709d Mon Sep 17 00:00:00 2001 From: bmeurer Date: Mon, 21 Sep 2015 09:05:27 -0700 Subject: [PATCH] [ic] Also collect known map for relational comparison. Previously we only collected the known map for equality comparisons. But if we also collect it for relational comparisons, we can inline a fast path of ToPrimitive on the objects, which is especially interesting since both sides have the same map. For now we only inline a very limited subset of ToPrimitive in Crankshaft, which is when the receiver map (and its prototype chain) doesn't have @@toPrimitive, and both valueOf and toString are the default versions on the %ObjectPrototype%. In this case the relational comparison would reduce to a string comparison of "[object CLASS]" with itself and so we can reduce that to a boolean constant plus map checks on both left and right hand side, plus code dependencies on the prototype chain. This repairs the regression on box2d. R=jkummerow@chromium.org BUG=chromium:534200 LOG=n Review URL: https://codereview.chromium.org/1355113002 Cr-Commit-Position: refs/heads/master@{#30852} --- src/arm/code-stubs-arm.cc | 16 +++++++-- src/arm64/code-stubs-arm64.cc | 13 +++++++ src/code-stubs.cc | 1 - src/contexts.h | 2 ++ src/hydrogen.cc | 40 ++++++++++++++++++++++ src/ia32/code-stubs-ia32.cc | 21 ++++++++---- src/ic/ic-state.cc | 9 +++-- src/mips/code-stubs-mips.cc | 16 +++++++-- src/mips64/code-stubs-mips64.cc | 16 +++++++-- src/v8natives.js | 2 ++ src/x64/code-stubs-x64.cc | 21 ++++++++---- test/mjsunit/compare-known-objects-slow.js | 32 +++++++++++++++++ test/mjsunit/compare-known-objects.js | 32 +++++++++++++++++ 13 files changed, 197 insertions(+), 24 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index cb7d15fce..154db4f24 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -3672,8 +3672,20 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) { __ cmp(r3, r4); __ b(ne, &miss); - __ sub(r0, r0, Operand(r1)); - __ Ret(); + if (Token::IsEqualityOp(op())) { + __ sub(r0, r0, Operand(r1)); + __ Ret(); + } else if (is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (op() == Token::LT || op() == Token::LTE) { + __ mov(r2, Operand(Smi::FromInt(GREATER))); + } else { + __ mov(r2, Operand(Smi::FromInt(LESS))); + } + __ Push(r1, r0, r2); + __ TailCallRuntime(Runtime::kCompare, 3, 1); + } __ bind(&miss); GenerateMiss(masm); diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index e93fe44e7..9def80315 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -3567,8 +3567,21 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) { __ Cmp(lhs_map, map); __ B(ne, &miss); + if (Token::IsEqualityOp(op())) { __ Sub(result, rhs, lhs); __ Ret(); + } else if (is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + Register ncr = x2; + if (op() == Token::LT || op() == Token::LTE) { + __ Mov(ncr, Smi::FromInt(GREATER)); + } else { + __ Mov(ncr, Smi::FromInt(LESS)); + } + __ Push(lhs, rhs, ncr); + __ TailCallRuntime(Runtime::kCompare, 3, 1); + } __ Bind(&miss); GenerateMiss(masm); diff --git a/src/code-stubs.cc b/src/code-stubs.cc index 3e1b54835..8bd7c5449 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -383,7 +383,6 @@ bool CompareICStub::FindCodeInSpecialCache(Code** code_out) { Code::Flags flags = Code::ComputeFlags( GetCodeKind(), UNINITIALIZED); - DCHECK(op() == Token::EQ || op() == Token::EQ_STRICT); Handle probe( known_map_->FindInCodeCache( strict() ? diff --git a/src/contexts.h b/src/contexts.h index fb83923b6..1692038f9 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -132,6 +132,8 @@ enum BindingFlags { V(NATIVE_OBJECT_OBSERVE_INDEX, JSFunction, native_object_observe) \ V(NO_SIDE_EFFECT_TO_STRING_FUN_INDEX, JSFunction, \ no_side_effect_to_string_fun) \ + V(OBJECT_VALUE_OF, JSFunction, object_value_of) \ + V(OBJECT_TO_STRING, JSFunction, object_to_string) \ V(OBJECT_DEFINE_OWN_PROPERTY_INDEX, JSFunction, object_define_own_property) \ V(OBJECT_GET_OWN_PROPERTY_DESCROPTOR_INDEX, JSFunction, \ object_get_own_property_descriptor) \ diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 5ea5776f4..2796754fc 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -11578,6 +11578,46 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction( return result; } } else { + if (combined_type->IsClass()) { + // TODO(bmeurer): This is an optimized version of an x < y, x > y, + // x <= y or x >= y, where both x and y are spec objects with the + // same map. The CompareIC collects this map for us. So if we know + // that there's no @@toPrimitive on the map (including the prototype + // chain), and both valueOf and toString are the default initial + // implementations (on the %ObjectPrototype%), then we can reduce + // the comparison to map checks on x and y, because the comparison + // will turn into a comparison of "[object CLASS]" to itself (the + // default outcome of toString, since valueOf returns a spec object). + // This is pretty much adhoc, so in TurboFan we could do a lot better + // and inline the interesting parts of ToPrimitive (actually we could + // even do that in Crankshaft but we don't want to waste too much + // time on this now). + DCHECK(Token::IsOrderedRelationalCompareOp(op)); + Handle map = combined_type->AsClass()->Map(); + PropertyAccessInfo value_of(this, LOAD, map, + isolate()->factory()->valueOf_string()); + PropertyAccessInfo to_primitive( + this, LOAD, map, isolate()->factory()->to_primitive_symbol()); + PropertyAccessInfo to_string(this, LOAD, map, + isolate()->factory()->toString_string()); + if (to_primitive.CanAccessMonomorphic() && !to_primitive.IsFound() && + value_of.CanAccessMonomorphic() && value_of.IsDataConstant() && + value_of.constant().is_identical_to(isolate()->object_value_of()) && + to_string.CanAccessMonomorphic() && to_string.IsDataConstant() && + to_string.constant().is_identical_to( + isolate()->object_to_string())) { + // We depend on the prototype chain to stay the same, because we + // also need to deoptimize when someone installs @@toPrimitive + // somewhere in the prototype chain. + BuildCheckPrototypeMaps(handle(JSObject::cast(map->prototype())), + Handle::null()); + AddCheckMap(left, map); + AddCheckMap(right, map); + // The caller expects a branch instruction, so make it happy. + return New( + graph()->GetConstantBool(op == Token::LTE || op == Token::GTE)); + } + } Bailout(kUnsupportedNonPrimitiveCompare); return NULL; } diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 3cb99eb07..7da077962 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -3744,15 +3744,24 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) { __ JumpIfSmi(ecx, &miss, Label::kNear); __ GetWeakValue(edi, cell); - __ mov(ecx, FieldOperand(eax, HeapObject::kMapOffset)); - __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset)); - __ cmp(ecx, edi); + __ cmp(edi, FieldOperand(eax, HeapObject::kMapOffset)); __ j(not_equal, &miss, Label::kNear); - __ cmp(ebx, edi); + __ cmp(edi, FieldOperand(edx, HeapObject::kMapOffset)); __ j(not_equal, &miss, Label::kNear); - __ sub(eax, edx); - __ ret(0); + if (Token::IsEqualityOp(op())) { + __ sub(eax, edx); + __ ret(0); + } else if (is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + __ PopReturnAddressTo(ecx); + __ Push(edx); + __ Push(eax); + __ Push(Immediate(Smi::FromInt(NegativeComparisonResult(GetCondition())))); + __ PushReturnAddressFrom(ecx); + __ TailCallRuntime(Runtime::kCompare, 3, 1); + } __ bind(&miss); GenerateMiss(masm); diff --git a/src/ic/ic-state.cc b/src/ic/ic-state.cc index fc33c8048..79e2f1401 100644 --- a/src/ic/ic-state.cc +++ b/src/ic/ic-state.cc @@ -470,16 +470,16 @@ CompareICState::State CompareICState::TargetState( return Token::IsEqualityOp(op) ? INTERNALIZED_STRING : STRING; } if (x->IsString() && y->IsString()) return STRING; - if (!Token::IsEqualityOp(op)) return GENERIC; - if (x->IsUniqueName() && y->IsUniqueName()) return UNIQUE_NAME; if (x->IsJSObject() && y->IsJSObject()) { if (Handle::cast(x)->map() == Handle::cast(y)->map()) { return KNOWN_OBJECT; } else { - return OBJECT; + return Token::IsEqualityOp(op) ? OBJECT : GENERIC; } } + if (!Token::IsEqualityOp(op)) return GENERIC; + if (x->IsUniqueName() && y->IsUniqueName()) return UNIQUE_NAME; return GENERIC; case SMI: return x->IsNumber() && y->IsNumber() ? NUMBER : GENERIC; @@ -496,9 +496,8 @@ CompareICState::State CompareICState::TargetState( if (old_right == SMI && y->IsHeapNumber()) return NUMBER; return GENERIC; case KNOWN_OBJECT: - DCHECK(Token::IsEqualityOp(op)); if (x->IsJSObject() && y->IsJSObject()) { - return OBJECT; + return Token::IsEqualityOp(op) ? OBJECT : GENERIC; } return GENERIC; case STRING: diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index 78c24cf9b..bdb382369 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -3859,8 +3859,20 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) { __ Branch(&miss, ne, a2, Operand(t0)); __ Branch(&miss, ne, a3, Operand(t0)); - __ Ret(USE_DELAY_SLOT); - __ subu(v0, a0, a1); + if (Token::IsEqualityOp(op())) { + __ Ret(USE_DELAY_SLOT); + __ subu(v0, a0, a1); + } else if (is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (op() == Token::LT || op() == Token::LTE) { + __ li(a2, Operand(Smi::FromInt(GREATER))); + } else { + __ li(a2, Operand(Smi::FromInt(LESS))); + } + __ Push(a1, a0, a2); + __ TailCallRuntime(Runtime::kCompare, 3, 1); + } __ bind(&miss); GenerateMiss(masm); diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 9df14e387..f139636ca 100644 --- a/src/mips64/code-stubs-mips64.cc +++ b/src/mips64/code-stubs-mips64.cc @@ -3891,8 +3891,20 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) { __ Branch(&miss, ne, a2, Operand(a4)); __ Branch(&miss, ne, a3, Operand(a4)); - __ Ret(USE_DELAY_SLOT); - __ dsubu(v0, a0, a1); + if (Token::IsEqualityOp(op())) { + __ Ret(USE_DELAY_SLOT); + __ dsubu(v0, a0, a1); + } else if (is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (op() == Token::LT || op() == Token::LTE) { + __ li(a2, Operand(Smi::FromInt(GREATER))); + } else { + __ li(a2, Operand(Smi::FromInt(LESS))); + } + __ Push(a1, a0, a2); + __ TailCallRuntime(Runtime::kCompare, 3, 1); + } __ bind(&miss); GenerateMiss(masm); diff --git a/src/v8natives.js b/src/v8natives.js index 63cf0a0a3..2a235a26c 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -1844,6 +1844,8 @@ utils.Export(function(to) { %InstallToContext([ "global_eval_fun", GlobalEval, + "object_value_of", ObjectValueOf, + "object_to_string", ObjectToString, "object_define_own_property", DefineOwnPropertyFromAPI, "object_get_own_property_descriptor", ObjectGetOwnPropertyDescriptor, "to_complete_property_descriptor", ToCompletePropertyDescriptor, diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 86cca8979..cda5d4f9f 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -3683,15 +3683,24 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) { __ j(either_smi, &miss, Label::kNear); __ GetWeakValue(rdi, cell); - __ movp(rcx, FieldOperand(rax, HeapObject::kMapOffset)); - __ movp(rbx, FieldOperand(rdx, HeapObject::kMapOffset)); - __ cmpp(rcx, rdi); + __ cmpp(FieldOperand(rdx, HeapObject::kMapOffset), rdi); __ j(not_equal, &miss, Label::kNear); - __ cmpp(rbx, rdi); + __ cmpp(FieldOperand(rax, HeapObject::kMapOffset), rdi); __ j(not_equal, &miss, Label::kNear); - __ subp(rax, rdx); - __ ret(0); + if (Token::IsEqualityOp(op())) { + __ subp(rax, rdx); + __ ret(0); + } else if (is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + __ PopReturnAddressTo(rcx); + __ Push(rdx); + __ Push(rax); + __ Push(Smi::FromInt(NegativeComparisonResult(GetCondition()))); + __ PushReturnAddressFrom(rcx); + __ TailCallRuntime(Runtime::kCompare, 3, 1); + } __ bind(&miss); GenerateMiss(masm); diff --git a/test/mjsunit/compare-known-objects-slow.js b/test/mjsunit/compare-known-objects-slow.js index afa198fcb..41e9fb4ad 100644 --- a/test/mjsunit/compare-known-objects-slow.js +++ b/test/mjsunit/compare-known-objects-slow.js @@ -39,6 +39,22 @@ function eq_strict(a, b) { return a === b; } +function le(a, b) { + return a <= b; +} + +function lt(a, b) { + return a < b; +} + +function ge(a, b) { + return a >= b; +} + +function gt(a, b) { + return a > b; +} + function test(a, b) { // Check CompareIC for equality of known objects. assertTrue(eq(a, a)); @@ -48,6 +64,22 @@ function test(a, b) { assertTrue(eq_strict(a, a)); assertTrue(eq_strict(b, b)); assertFalse(eq_strict(a, b)); + // Check CompareIC for less than or equal of known objects. + assertTrue(le(a, a)); + assertTrue(le(a, b)); + assertTrue(le(b, a)); + // Check CompareIC for less than of known objects. + assertFalse(lt(a, a)); + assertFalse(lt(a, b)); + assertFalse(lt(b, a)); + // Check CompareIC for greater than or equal of known objects. + assertTrue(ge(a, a)); + assertTrue(ge(a, b)); + assertTrue(ge(b, a)); + // Check CompareIC for greater than of known objects. + assertFalse(gt(a, a)); + assertFalse(gt(a, b)); + assertFalse(gt(b, a)); } // Prepare two objects in slow mode that have the same map. diff --git a/test/mjsunit/compare-known-objects.js b/test/mjsunit/compare-known-objects.js index afffc0701..051b12d70 100644 --- a/test/mjsunit/compare-known-objects.js +++ b/test/mjsunit/compare-known-objects.js @@ -39,6 +39,22 @@ function eq_strict(a, b) { return a === b; } +function le(a, b) { + return a <= b; +} + +function lt(a, b) { + return a < b; +} + +function ge(a, b) { + return a >= b; +} + +function gt(a, b) { + return a > b; +} + function test(a, b) { // Check CompareIC for equality of known objects. assertTrue(eq(a, a)); @@ -48,6 +64,22 @@ function test(a, b) { assertTrue(eq_strict(a, a)); assertTrue(eq_strict(b, b)); assertFalse(eq_strict(a, b)); + // Check CompareIC for less than or equal of known objects. + assertTrue(le(a, a)); + assertTrue(le(a, b)); + assertTrue(le(b, a)); + // Check CompareIC for less than of known objects. + assertFalse(lt(a, a)); + assertFalse(lt(a, b)); + assertFalse(lt(b, a)); + // Check CompareIC for greater than or equal of known objects. + assertTrue(ge(a, a)); + assertTrue(ge(a, b)); + assertTrue(ge(b, a)); + // Check CompareIC for greater than of known objects. + assertFalse(gt(a, a)); + assertFalse(gt(a, b)); + assertFalse(gt(b, a)); } function O(){}; -- 2.34.1