From 10c5f2e85ef92b7ca002ef95e406d5dc4f0c410b Mon Sep 17 00:00:00 2001 From: bmeurer Date: Tue, 22 Sep 2015 01:01:26 -0700 Subject: [PATCH] [ic] Introduce BOOLEAN state for CompareIC. Slow path for relational comparison of boolean primitive values now goes through the runtime, which made the slow path even slower than it already was. So in order to repair the regression, we just track boolean feedback for comparisons and use that to generate decent code in Crankshaft (not the best possible code, but good enough for Crankshaft; TurboFan will be able to do better on that). R=jarin@chromium.org BUG=chromium:534200 LOG=n Review URL: https://codereview.chromium.org/1347063004 Cr-Commit-Position: refs/heads/master@{#30860} --- src/arm/code-stubs-arm.cc | 24 ++++++++++++++++++++++++ src/arm64/code-stubs-arm64.cc | 26 ++++++++++++++++++++++++++ src/code-stubs.cc | 4 ++++ src/code-stubs.h | 1 + src/hydrogen-instructions.h | 5 +++++ src/hydrogen.cc | 17 +++++++++++++++++ src/ia32/code-stubs-ia32.cc | 31 +++++++++++++++++++++++++++++++ src/ic/ic-state.cc | 11 +++++++++++ src/ic/ic-state.h | 2 ++ src/mips/code-stubs-mips.cc | 24 ++++++++++++++++++++++++ src/mips64/code-stubs-mips64.cc | 24 ++++++++++++++++++++++++ src/x64/code-stubs-x64.cc | 31 +++++++++++++++++++++++++++++++ 12 files changed, 200 insertions(+) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index ab5a89f..95548cf 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -3360,6 +3360,30 @@ void BinaryOpICWithAllocationSiteStub::Generate(MacroAssembler* masm) { } +void CompareICStub::GenerateBooleans(MacroAssembler* masm) { + DCHECK_EQ(CompareICState::BOOLEAN, state()); + Label miss; + + __ CheckMap(r1, r2, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + __ CheckMap(r0, r3, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + if (op() != Token::EQ_STRICT && is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (!Token::IsEqualityOp(op())) { + __ ldr(r1, FieldMemOperand(r1, Oddball::kToNumberOffset)); + __ AssertSmi(r1); + __ ldr(r0, FieldMemOperand(r0, Oddball::kToNumberOffset)); + __ AssertSmi(r0); + } + __ sub(r0, r1, r0); + __ Ret(); + } + + __ bind(&miss); + GenerateMiss(masm); +} + + void CompareICStub::GenerateSmis(MacroAssembler* masm) { DCHECK(state() == CompareICState::SMI); Label miss; diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index 5ba08f7..aaedcef 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -3246,6 +3246,32 @@ void StringCharFromCodeGenerator::GenerateSlow( } +void CompareICStub::GenerateBooleans(MacroAssembler* masm) { + // Inputs are in x0 (lhs) and x1 (rhs). + DCHECK_EQ(CompareICState::BOOLEAN, state()); + ASM_LOCATION("CompareICStub[Booleans]"); + Label miss; + + __ CheckMap(x1, x2, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + __ CheckMap(x0, x3, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + if (op() != Token::EQ_STRICT && is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (!Token::IsEqualityOp(op())) { + __ Ldr(x1, FieldMemOperand(x1, Oddball::kToNumberOffset)); + __ AssertSmi(x1); + __ Ldr(x0, FieldMemOperand(x0, Oddball::kToNumberOffset)); + __ AssertSmi(x0); + } + __ Sub(x0, x1, x0); + __ Ret(); + } + + __ Bind(&miss); + GenerateMiss(masm); +} + + void CompareICStub::GenerateSmis(MacroAssembler* masm) { // Inputs are in x0 (lhs) and x1 (rhs). DCHECK(state() == CompareICState::SMI); diff --git a/src/code-stubs.cc b/src/code-stubs.cc index 06e7a65..d9df19e 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -351,6 +351,7 @@ InlineCacheState CompareICStub::GetICState() const { switch (state) { case CompareICState::UNINITIALIZED: return ::v8::internal::UNINITIALIZED; + case CompareICState::BOOLEAN: case CompareICState::SMI: case CompareICState::NUMBER: case CompareICState::INTERNALIZED_STRING: @@ -416,6 +417,9 @@ void CompareICStub::Generate(MacroAssembler* masm) { case CompareICState::UNINITIALIZED: GenerateMiss(masm); break; + case CompareICState::BOOLEAN: + GenerateBooleans(masm); + break; case CompareICState::SMI: GenerateSmis(masm); break; diff --git a/src/code-stubs.h b/src/code-stubs.h index 4b259b6..32423c2 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1697,6 +1697,7 @@ class CompareICStub : public PlatformCodeStub { private: Code::Kind GetCodeKind() const override { return Code::COMPARE_IC; } + void GenerateBooleans(MacroAssembler* masm); void GenerateSmis(MacroAssembler* masm); void GenerateNumbers(MacroAssembler* masm); void GenerateInternalizedStrings(MacroAssembler* masm); diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index be03186..9f5bc20 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -5946,6 +5946,11 @@ class HObjectAccess final { Representation::Integer32()); } + static HObjectAccess ForOddballToNumber( + Representation representation = Representation::Tagged()) { + return HObjectAccess(kInobject, Oddball::kToNumberOffset, representation); + } + static HObjectAccess ForOddballTypeOf() { return HObjectAccess(kInobject, Oddball::kTypeOfOffset, Representation::HeapObject()); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index b24890a..eef51e5 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -11650,6 +11650,23 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction( HStringCompareAndBranch* result = New(left, right, op); return result; + } else if (combined_type->Is(Type::Boolean())) { + AddCheckMap(left, isolate()->factory()->boolean_map()); + AddCheckMap(right, isolate()->factory()->boolean_map()); + if (Token::IsEqualityOp(op)) { + HCompareObjectEqAndBranch* result = + New(left, right); + return result; + } + left = Add( + left, nullptr, + HObjectAccess::ForOddballToNumber(Representation::Smi())); + right = Add( + right, nullptr, + HObjectAccess::ForOddballToNumber(Representation::Smi())); + HCompareNumericAndBranch* result = + New(left, right, op); + return result; } else { if (combined_rep.IsTagged() || combined_rep.IsNone()) { HCompareGeneric* result = Add( diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index d1efe50..71fcc8d 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -3400,6 +3400,37 @@ void BinaryOpICWithAllocationSiteStub::Generate(MacroAssembler* masm) { } +void CompareICStub::GenerateBooleans(MacroAssembler* masm) { + DCHECK_EQ(CompareICState::BOOLEAN, state()); + Label miss; + Label::Distance const miss_distance = + masm->emit_debug_code() ? Label::kFar : Label::kNear; + + __ JumpIfSmi(edx, &miss, miss_distance); + __ mov(ecx, FieldOperand(edx, HeapObject::kMapOffset)); + __ JumpIfSmi(eax, &miss, miss_distance); + __ mov(ebx, FieldOperand(eax, HeapObject::kMapOffset)); + __ JumpIfNotRoot(ecx, Heap::kBooleanMapRootIndex, &miss, miss_distance); + __ JumpIfNotRoot(ebx, Heap::kBooleanMapRootIndex, &miss, miss_distance); + if (op() != Token::EQ_STRICT && is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (!Token::IsEqualityOp(op())) { + __ mov(eax, FieldOperand(eax, Oddball::kToNumberOffset)); + __ AssertSmi(eax); + __ mov(edx, FieldOperand(edx, Oddball::kToNumberOffset)); + __ AssertSmi(edx); + __ xchg(eax, edx); + } + __ sub(eax, edx); + __ Ret(); + } + + __ bind(&miss); + GenerateMiss(masm); +} + + void CompareICStub::GenerateSmis(MacroAssembler* masm) { DCHECK(state() == CompareICState::SMI); Label miss; diff --git a/src/ic/ic-state.cc b/src/ic/ic-state.cc index 79e2f14..bc03d7d 100644 --- a/src/ic/ic-state.cc +++ b/src/ic/ic-state.cc @@ -358,6 +358,8 @@ const char* CompareICState::GetStateName(State state) { switch (state) { case UNINITIALIZED: return "UNINITIALIZED"; + case BOOLEAN: + return "BOOLEAN"; case SMI: return "SMI"; case NUMBER: @@ -384,6 +386,8 @@ Type* CompareICState::StateToType(Zone* zone, State state, Handle map) { switch (state) { case UNINITIALIZED: return Type::None(zone); + case BOOLEAN: + return Type::Boolean(zone); case SMI: return Type::SignedSmall(zone); case NUMBER: @@ -410,6 +414,7 @@ CompareICState::State CompareICState::NewInputState(State old_state, Handle value) { switch (old_state) { case UNINITIALIZED: + if (value->IsBoolean()) return BOOLEAN; if (value->IsSmi()) return SMI; if (value->IsHeapNumber()) return NUMBER; if (value->IsInternalizedString()) return INTERNALIZED_STRING; @@ -417,6 +422,9 @@ CompareICState::State CompareICState::NewInputState(State old_state, if (value->IsSymbol()) return UNIQUE_NAME; if (value->IsJSObject()) return OBJECT; break; + case BOOLEAN: + if (value->IsBoolean()) return BOOLEAN; + break; case SMI: if (value->IsSmi()) return SMI; if (value->IsHeapNumber()) return NUMBER; @@ -454,6 +462,7 @@ CompareICState::State CompareICState::TargetState( bool has_inlined_smi_code, Handle x, Handle y) { switch (old_state) { case UNINITIALIZED: + if (x->IsBoolean() && y->IsBoolean()) return BOOLEAN; if (x->IsSmi() && y->IsSmi()) return SMI; if (x->IsNumber() && y->IsNumber()) return NUMBER; if (Token::IsOrderedRelationalCompareOp(op)) { @@ -500,6 +509,7 @@ CompareICState::State CompareICState::TargetState( return Token::IsEqualityOp(op) ? OBJECT : GENERIC; } return GENERIC; + case BOOLEAN: case STRING: case UNIQUE_NAME: case OBJECT: @@ -509,5 +519,6 @@ CompareICState::State CompareICState::TargetState( UNREACHABLE(); return GENERIC; // Make the compiler happy. } + } // namespace internal } // namespace v8 diff --git a/src/ic/ic-state.h b/src/ic/ic-state.h index 0b4b9cd..b529b8c 100644 --- a/src/ic/ic-state.h +++ b/src/ic/ic-state.h @@ -174,9 +174,11 @@ class CompareICState { // ... < GENERIC // SMI < NUMBER // INTERNALIZED_STRING < STRING + // INTERNALIZED_STRING < UNIQUE_NAME // KNOWN_OBJECT < OBJECT enum State { UNINITIALIZED, + BOOLEAN, SMI, NUMBER, STRING, diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index 40d77dc..b4a9d3b 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -3518,6 +3518,30 @@ void BinaryOpICWithAllocationSiteStub::Generate(MacroAssembler* masm) { } +void CompareICStub::GenerateBooleans(MacroAssembler* masm) { + DCHECK_EQ(CompareICState::BOOLEAN, state()); + Label miss; + + __ CheckMap(a1, a2, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + __ CheckMap(a0, a3, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + if (op() != Token::EQ_STRICT && is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (!Token::IsEqualityOp(op())) { + __ lw(a1, FieldMemOperand(a1, Oddball::kToNumberOffset)); + __ AssertSmi(a1); + __ lw(a0, FieldMemOperand(a0, Oddball::kToNumberOffset)); + __ AssertSmi(a0); + } + __ Ret(USE_DELAY_SLOT); + __ Subu(v0, a1, a0); + } + + __ bind(&miss); + GenerateMiss(masm); +} + + void CompareICStub::GenerateSmis(MacroAssembler* masm) { DCHECK(state() == CompareICState::SMI); Label miss; diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 87a077f..d840665 100644 --- a/src/mips64/code-stubs-mips64.cc +++ b/src/mips64/code-stubs-mips64.cc @@ -3551,6 +3551,30 @@ void BinaryOpICWithAllocationSiteStub::Generate(MacroAssembler* masm) { } +void CompareICStub::GenerateBooleans(MacroAssembler* masm) { + DCHECK_EQ(CompareICState::BOOLEAN, state()); + Label miss; + + __ CheckMap(a1, a2, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + __ CheckMap(a0, a3, Heap::kBooleanMapRootIndex, &miss, DO_SMI_CHECK); + if (op() != Token::EQ_STRICT && is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (!Token::IsEqualityOp(op())) { + __ ld(a1, FieldMemOperand(a1, Oddball::kToNumberOffset)); + __ AssertSmi(a1); + __ ld(a0, FieldMemOperand(a0, Oddball::kToNumberOffset)); + __ AssertSmi(a0); + } + __ Ret(USE_DELAY_SLOT); + __ Dsubu(v0, a1, a0); + } + + __ bind(&miss); + GenerateMiss(masm); +} + + void CompareICStub::GenerateSmis(MacroAssembler* masm) { DCHECK(state() == CompareICState::SMI); Label miss; diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 3383458..c24a6a0 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -3356,6 +3356,37 @@ void BinaryOpICWithAllocationSiteStub::Generate(MacroAssembler* masm) { } +void CompareICStub::GenerateBooleans(MacroAssembler* masm) { + DCHECK_EQ(CompareICState::BOOLEAN, state()); + Label miss; + Label::Distance const miss_distance = + masm->emit_debug_code() ? Label::kFar : Label::kNear; + + __ JumpIfSmi(rdx, &miss, miss_distance); + __ movp(rcx, FieldOperand(rdx, HeapObject::kMapOffset)); + __ JumpIfSmi(rax, &miss, miss_distance); + __ movp(rbx, FieldOperand(rax, HeapObject::kMapOffset)); + __ JumpIfNotRoot(rcx, Heap::kBooleanMapRootIndex, &miss, miss_distance); + __ JumpIfNotRoot(rbx, Heap::kBooleanMapRootIndex, &miss, miss_distance); + if (op() != Token::EQ_STRICT && is_strong(strength())) { + __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1); + } else { + if (!Token::IsEqualityOp(op())) { + __ movp(rax, FieldOperand(rax, Oddball::kToNumberOffset)); + __ AssertSmi(rax); + __ movp(rdx, FieldOperand(rdx, Oddball::kToNumberOffset)); + __ AssertSmi(rdx); + __ xchgp(rax, rdx); + } + __ subp(rax, rdx); + __ Ret(); + } + + __ bind(&miss); + GenerateMiss(masm); +} + + void CompareICStub::GenerateSmis(MacroAssembler* masm) { DCHECK(state() == CompareICState::SMI); Label miss; -- 2.7.4