From 65716011492fce55475f6e57fc3e8d588afede88 Mon Sep 17 00:00:00 2001 From: mythria Date: Tue, 11 Oct 2016 05:41:50 -0700 Subject: [PATCH] [Interpreter] Collect feedback about Oddballs in Subtract Stub. Also include feedback about Oddballs when collecting the type feedback. For now, Number and NumberOrOddball are collected separately because crankshaft does not handle NumberOrOddballs consistently. This should change once we fix crankshaft. BUG=v8:4280, v8:5400 LOG=N Committed: https://crrev.com/d5d283b27d406486e2f8d7b44c6d2b3db4f98458 Review-Url: https://codereview.chromium.org/2406843002 Cr-Original-Commit-Position: refs/heads/master@{#40124} Cr-Commit-Position: refs/heads/master@{#40170} --- gypfiles/get_landmines.py | 1 + src/code-stubs.cc | 66 ++++++++++++++++++++++++++++++---- src/globals.h | 14 +++++--- src/type-feedback-vector-inl.h | 1 + src/type-info.cc | 23 ++++++------ 5 files changed, 85 insertions(+), 20 deletions(-) diff --git a/gypfiles/get_landmines.py b/gypfiles/get_landmines.py index 432dfd7ae5..e6b6da6c48 100755 --- a/gypfiles/get_landmines.py +++ b/gypfiles/get_landmines.py @@ -30,6 +30,7 @@ def main(): print 'Clobber after Android NDK update.' print 'Clober to fix windows build problems.' print 'Clober again to fix windows build problems.' + print 'Clobber to possibly resolve failure on win-32 bot.' return 0 diff --git a/src/code-stubs.cc b/src/code-stubs.cc index eb2bf32b6b..06b7afcaeb 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -828,8 +828,9 @@ compiler::Node* SubtractWithFeedbackStub::Generate( typedef CodeStubAssembler::Variable Variable; // Shared entry for floating point subtraction. - Label do_fsub(assembler), end(assembler), - call_subtract_stub(assembler, Label::kDeferred); + Label do_fsub(assembler), end(assembler), call_subtract_stub(assembler), + if_lhsisnotnumber(assembler), check_rhsisoddball(assembler), + call_with_any_feedback(assembler); Variable var_fsub_lhs(assembler, MachineRepresentation::kFloat64), var_fsub_rhs(assembler, MachineRepresentation::kFloat64), var_type_feedback(assembler, MachineRepresentation::kWord32), @@ -879,7 +880,7 @@ compiler::Node* SubtractWithFeedbackStub::Generate( // Check if {rhs} is a HeapNumber. assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), - &call_subtract_stub); + &check_rhsisoddball); // Perform a floating point subtraction. var_fsub_lhs.Bind(assembler->SmiToFloat64(lhs)); @@ -895,7 +896,7 @@ compiler::Node* SubtractWithFeedbackStub::Generate( // Check if the {lhs} is a HeapNumber. assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), - &call_subtract_stub); + &if_lhsisnotnumber); // Check if the {rhs} is a Smi. Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler); @@ -916,7 +917,7 @@ compiler::Node* SubtractWithFeedbackStub::Generate( // Check if the {rhs} is a HeapNumber. assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), - &call_subtract_stub); + &check_rhsisoddball); // Perform a floating point subtraction. var_fsub_lhs.Bind(assembler->LoadHeapNumberValue(lhs)); @@ -936,10 +937,63 @@ compiler::Node* SubtractWithFeedbackStub::Generate( assembler->Goto(&end); } - assembler->Bind(&call_subtract_stub); + assembler->Bind(&if_lhsisnotnumber); + { + // No checks on rhs are done yet. We just know lhs is not a number or Smi. + // Check if lhs is an oddball. + Node* lhs_instance_type = assembler->LoadInstanceType(lhs); + Node* lhs_is_oddball = assembler->Word32Equal( + lhs_instance_type, assembler->Int32Constant(ODDBALL_TYPE)); + assembler->GotoUnless(lhs_is_oddball, &call_with_any_feedback); + + Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler); + assembler->Branch(assembler->WordIsSmi(rhs), &if_rhsissmi, &if_rhsisnotsmi); + + assembler->Bind(&if_rhsissmi); + { + var_type_feedback.Bind( + assembler->Int32Constant(BinaryOperationFeedback::kNumberOrOddball)); + assembler->Goto(&call_subtract_stub); + } + + assembler->Bind(&if_rhsisnotsmi); + { + // Load the map of the {rhs}. + Node* rhs_map = assembler->LoadMap(rhs); + + // Check if {rhs} is a HeapNumber. + assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), + &check_rhsisoddball); + + var_type_feedback.Bind( + assembler->Int32Constant(BinaryOperationFeedback::kNumberOrOddball)); + assembler->Goto(&call_subtract_stub); + } + } + + assembler->Bind(&check_rhsisoddball); + { + // Check if rhs is an oddball. At this point we know lhs is either a + // Smi or number or oddball and rhs is not a number or Smi. + Node* rhs_instance_type = assembler->LoadInstanceType(rhs); + Node* rhs_is_oddball = assembler->Word32Equal( + rhs_instance_type, assembler->Int32Constant(ODDBALL_TYPE)); + assembler->GotoUnless(rhs_is_oddball, &call_with_any_feedback); + + var_type_feedback.Bind( + assembler->Int32Constant(BinaryOperationFeedback::kNumberOrOddball)); + assembler->Goto(&call_subtract_stub); + } + + assembler->Bind(&call_with_any_feedback); { var_type_feedback.Bind( assembler->Int32Constant(BinaryOperationFeedback::kAny)); + assembler->Goto(&call_subtract_stub); + } + + assembler->Bind(&call_subtract_stub); + { Callable callable = CodeFactory::Subtract(assembler->isolate()); var_result.Bind(assembler->CallStub(callable, context, lhs, rhs)); assembler->Goto(&end); diff --git a/src/globals.h b/src/globals.h index 03c5b1dc1a..0b63c7fcd8 100644 --- a/src/globals.h +++ b/src/globals.h @@ -1208,16 +1208,22 @@ inline uint32_t ObjectHash(Address address) { // Type feedback is encoded in such a way that, we can combine the feedback // 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 +// kSignedSmall -> kNumber -> kNumberOrOddball -> kAny +// kString -> kAny +// TODO(mythria): Remove kNumber type when crankshaft can handle Oddballs +// similar to Numbers. We don't need kNumber feedback for Turbofan. Extra +// information about Number might reduce few instructions but causes more +// deopts. We collect Number only because crankshaft does not handle all +// cases of oddballs. class BinaryOperationFeedback { public: enum { kNone = 0x0, kSignedSmall = 0x1, kNumber = 0x3, - kString = 0x4, - kAny = 0xF + kNumberOrOddball = 0x7, + kString = 0x8, + kAny = 0x1F }; }; diff --git a/src/type-feedback-vector-inl.h b/src/type-feedback-vector-inl.h index f70f01888f..da0ecaf057 100644 --- a/src/type-feedback-vector-inl.h +++ b/src/type-feedback-vector-inl.h @@ -128,6 +128,7 @@ BinaryOperationHint BinaryOperationHintFromFeedback(int type_feedback) { case BinaryOperationFeedback::kSignedSmall: return BinaryOperationHint::kSignedSmall; case BinaryOperationFeedback::kNumber: + case BinaryOperationFeedback::kNumberOrOddball: return BinaryOperationHint::kNumberOrOddball; case BinaryOperationFeedback::kString: return BinaryOperationHint::kString; diff --git a/src/type-info.cc b/src/type-info.cc index ce0ab6ca6a..efbc9b1f07 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -210,19 +210,21 @@ AstType* CompareOpHintToType(CompareOperationHint hint) { return AstType::None(); } -AstType* BinaryOpHintToType(BinaryOperationHint hint) { +AstType* BinaryOpFeedbackToType(int hint) { switch (hint) { - case BinaryOperationHint::kNone: + case BinaryOperationFeedback::kNone: return AstType::None(); - case BinaryOperationHint::kSignedSmall: + case BinaryOperationFeedback::kSignedSmall: return AstType::SignedSmall(); - case BinaryOperationHint::kSigned32: - return AstType::Signed32(); - case BinaryOperationHint::kNumberOrOddball: + case BinaryOperationFeedback::kNumber: return AstType::Number(); - case BinaryOperationHint::kString: + case BinaryOperationFeedback::kString: return AstType::String(); - case BinaryOperationHint::kAny: + // TODO(mythria): Merge Number and NumberOrOddball feedback, after + // fixing crankshaft to handle Oddballs along with Numbers. + case BinaryOperationFeedback::kNumberOrOddball: + case BinaryOperationFeedback::kAny: + default: return AstType::Any(); } UNREACHABLE(); @@ -299,7 +301,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, FeedbackVectorSlot slot, DCHECK(!slot.IsInvalid()); BinaryOpICNexus nexus(feedback_vector_, slot); *left = *right = *result = - BinaryOpHintToType(nexus.GetBinaryOperationFeedback()); + BinaryOpFeedbackToType(Smi::cast(nexus.GetFeedback())->value()); *fixed_right_arg = Nothing(); *allocation_site = Handle::null(); @@ -334,7 +336,8 @@ AstType* TypeFeedbackOracle::CountType(TypeFeedbackId id, DCHECK(!slot.IsInvalid()); BinaryOpICNexus nexus(feedback_vector_, slot); - AstType* type = BinaryOpHintToType(nexus.GetBinaryOperationFeedback()); + AstType* type = + BinaryOpFeedbackToType(Smi::cast(nexus.GetFeedback())->value()); if (!object->IsCode()) return type;