Commit 65716011 authored by mythria's avatar mythria Committed by Commit bot

[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}
parent ff694196
......@@ -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
......
......@@ -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);
......
......@@ -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
};
};
......
......@@ -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;
......
......@@ -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<int>();
*allocation_site = Handle<AllocationSite>::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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment