Commit d9e43297 authored by bmeurer's avatar bmeurer Committed by Commit bot

[turbofan] Add Symbol feedback to Equal/StrictEqual.

Introduce a new Symbol comparison feedback bit in the lattice and
collect that feedback on Equal/StrictEqual in Ignition. Utilize this
feedback in TurboFan by adding a dedicated CheckSymbol operator to
check for symbol inputs. This way we can optimize Symbol comparison
where TurboFan doesn't know anything statically about either side, or
abstract equality comparisons where TurboFan doesn't statically know
anything about one side.

BUG=v8:6278,v8:6344,v8:6423
R=jarin@chromium.org

Review-Url: https://codereview.chromium.org/2893263002
Cr-Commit-Position: refs/heads/master@{#45455}
parent b5e610c1
......@@ -3284,6 +3284,10 @@ Node* CodeStubAssembler::IsString(Node* object) {
Int32Constant(FIRST_NONSTRING_TYPE));
}
Node* CodeStubAssembler::IsSymbolInstanceType(Node* instance_type) {
return Word32Equal(instance_type, Int32Constant(SYMBOL_TYPE));
}
Node* CodeStubAssembler::IsSymbol(Node* object) {
return IsSymbolMap(LoadMap(object));
}
......@@ -7462,9 +7466,12 @@ void CodeStubAssembler::GenerateEqual_Same(Node* value, Label* if_equal,
// Collect type feedback.
Node* instance_type = LoadMapInstanceType(value_map);
Label if_valueisstring(this), if_valueisnotstring(this);
Branch(IsStringInstanceType(instance_type), &if_valueisstring,
&if_valueisnotstring);
Label if_valueisstring(this), if_valueisreceiver(this),
if_valueissymbol(this), if_valueisother(this, Label::kDeferred);
GotoIf(IsStringInstanceType(instance_type), &if_valueisstring);
GotoIf(IsJSReceiverInstanceType(instance_type), &if_valueisreceiver);
Branch(IsSymbolInstanceType(instance_type), &if_valueissymbol,
&if_valueisother);
BIND(&if_valueisstring);
{
......@@ -7473,15 +7480,26 @@ void CodeStubAssembler::GenerateEqual_Same(Node* value, Label* if_equal,
Goto(if_equal);
}
BIND(&if_valueisnotstring);
BIND(&if_valueissymbol);
{
var_type_feedback->Bind(SmiConstant(CompareOperationFeedback::kAny));
GotoIfNot(IsJSReceiverInstanceType(instance_type), if_equal);
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kSymbol));
Goto(if_equal);
}
BIND(&if_valueisreceiver);
{
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kReceiver));
Goto(if_equal);
}
BIND(&if_valueisother);
{
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kAny));
Goto(if_equal);
}
} else {
Goto(if_equal);
}
......@@ -7879,14 +7897,8 @@ Node* CodeStubAssembler::Equal(Node* lhs, Node* rhs, Node* context,
BIND(&if_lhsissymbol);
{
if (var_type_feedback != nullptr) {
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
}
// Check if the {rhs} is a JSReceiver.
Label if_rhsisreceiver(this), if_rhsisnotreceiver(this);
STATIC_ASSERT(LAST_TYPE == LAST_JS_RECEIVER_TYPE);
Branch(IsJSReceiverInstanceType(rhs_instance_type),
&if_rhsisreceiver, &if_rhsisnotreceiver);
......@@ -7896,6 +7908,10 @@ Node* CodeStubAssembler::Equal(Node* lhs, Node* rhs, Node* context,
// Swapping {lhs} and {rhs} is not observable and doesn't
// matter for the result, so we can just swap them and use
// the JSReceiver handling below (for {lhs} being a JSReceiver).
if (var_type_feedback != nullptr) {
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
}
var_lhs.Bind(rhs);
var_rhs.Bind(lhs);
Goto(&loop);
......@@ -7905,8 +7921,28 @@ Node* CodeStubAssembler::Equal(Node* lhs, Node* rhs, Node* context,
{
// The {rhs} is not a JSReceiver and also not the same Symbol
// as the {lhs}, so this is equality check is considered false.
if (var_type_feedback != nullptr) {
Label if_rhsissymbol(this), if_rhsisnotsymbol(this);
Branch(IsSymbolInstanceType(rhs_instance_type), &if_rhsissymbol,
&if_rhsisnotsymbol);
BIND(&if_rhsissymbol);
{
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kSymbol));
Goto(&if_notequal);
}
BIND(&if_rhsisnotsymbol);
{
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
Goto(&if_notequal);
}
} else {
Goto(&if_notequal);
}
}
}
BIND(&if_lhsisreceiver);
......@@ -8195,17 +8231,34 @@ Node* CodeStubAssembler::StrictEqual(Node* lhs, Node* rhs,
BIND(&if_lhsisnotstring);
if (var_type_feedback != nullptr) {
GotoIfNot(IsJSReceiverInstanceType(lhs_instance_type),
Label if_lhsissymbol(this), if_lhsisreceiver(this);
GotoIf(IsJSReceiverInstanceType(lhs_instance_type),
&if_lhsisreceiver);
Branch(IsSymbolInstanceType(lhs_instance_type), &if_lhsissymbol,
&if_notequal);
BIND(&if_lhsisreceiver);
{
GotoIfNot(IsJSReceiverInstanceType(rhs_instance_type),
&if_notequal);
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kReceiver));
Goto(&if_notequal);
}
BIND(&if_lhsissymbol);
{
GotoIfNot(IsSymbolInstanceType(rhs_instance_type), &if_notequal);
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kSymbol));
Goto(&if_notequal);
}
} else {
Goto(&if_notequal);
}
}
}
}
BIND(&if_lhsissmi);
{
......
......@@ -777,6 +777,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
Node* IsAccessorPair(Node* object);
Node* IsHeapNumber(Node* object);
Node* IsName(Node* object);
Node* IsSymbolInstanceType(Node* instance_type);
Node* IsSymbol(Node* object);
Node* IsPrivateSymbol(Node* object);
Node* IsJSValueInstanceType(Node* instance_type);
......
......@@ -641,6 +641,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
case IrOpcode::kCheckReceiver:
result = LowerCheckReceiver(node, frame_state);
break;
case IrOpcode::kCheckSymbol:
result = LowerCheckSymbol(node, frame_state);
break;
case IrOpcode::kCheckString:
result = LowerCheckString(node, frame_state);
break;
......@@ -1300,6 +1303,17 @@ Node* EffectControlLinearizer::LowerCheckReceiver(Node* node,
return value;
}
Node* EffectControlLinearizer::LowerCheckSymbol(Node* node, Node* frame_state) {
Node* value = node->InputAt(0);
Node* value_map = __ LoadField(AccessBuilder::ForMap(), value);
Node* check =
__ WordEqual(value_map, __ HeapConstant(factory()->symbol_map()));
__ DeoptimizeUnless(DeoptimizeReason::kNotASymbol, check, frame_state);
return value;
}
Node* EffectControlLinearizer::LowerCheckString(Node* node, Node* frame_state) {
Node* value = node->InputAt(0);
......
......@@ -58,6 +58,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
Node* LowerCheckNumber(Node* node, Node* frame_state);
Node* LowerCheckReceiver(Node* node, Node* frame_state);
Node* LowerCheckString(Node* node, Node* frame_state);
Node* LowerCheckSymbol(Node* node, Node* frame_state);
Node* LowerCheckIf(Node* node, Node* frame_state);
Node* LowerCheckedInt32Add(Node* node, Node* frame_state);
Node* LowerCheckedInt32Sub(Node* node, Node* frame_state);
......
......@@ -667,6 +667,7 @@ struct JSOperatorGlobalCache final {
Name##Operator<CompareOperationHint::kInternalizedString> \
k##Name##InternalizedStringOperator; \
Name##Operator<CompareOperationHint::kString> k##Name##StringOperator; \
Name##Operator<CompareOperationHint::kSymbol> k##Name##SymbolOperator; \
Name##Operator<CompareOperationHint::kReceiver> k##Name##ReceiverOperator; \
Name##Operator<CompareOperationHint::kAny> k##Name##AnyOperator;
COMPARE_OP_LIST(COMPARE_OP)
......@@ -723,6 +724,8 @@ BINARY_OP_LIST(BINARY_OP)
return &cache_.k##Name##InternalizedStringOperator; \
case CompareOperationHint::kString: \
return &cache_.k##Name##StringOperator; \
case CompareOperationHint::kSymbol: \
return &cache_.k##Name##SymbolOperator; \
case CompareOperationHint::kReceiver: \
return &cache_.k##Name##ReceiverOperator; \
case CompareOperationHint::kAny: \
......
......@@ -82,6 +82,7 @@ class JSSpeculativeBinopBuilder final {
case CompareOperationHint::kAny:
case CompareOperationHint::kNone:
case CompareOperationHint::kString:
case CompareOperationHint::kSymbol:
case CompareOperationHint::kReceiver:
case CompareOperationHint::kInternalizedString:
break;
......
......@@ -47,6 +47,7 @@ class JSBinopReduction final {
case CompareOperationHint::kAny:
case CompareOperationHint::kNone:
case CompareOperationHint::kString:
case CompareOperationHint::kSymbol:
case CompareOperationHint::kReceiver:
case CompareOperationHint::kInternalizedString:
break;
......@@ -85,6 +86,16 @@ class JSBinopReduction final {
return false;
}
bool IsSymbolCompareOperation() {
if (lowering_->flags() & JSTypedLowering::kDeoptimizationEnabled) {
DCHECK_EQ(1, node_->op()->EffectOutputCount());
return (CompareOperationHintOf(node_->op()) ==
CompareOperationHint::kSymbol) &&
BothInputsMaybe(Type::Symbol());
}
return false;
}
// Check if a string addition will definitely result in creating a ConsString,
// i.e. if the combined length of the resulting string exceeds the ConsString
// minimum length.
......@@ -137,6 +148,24 @@ class JSBinopReduction final {
}
}
// Checks that both inputs are Symbol, and if we don't know
// statically that one side is already a Symbol, insert a
// CheckSymbol node.
void CheckInputsToSymbol() {
if (!left_type()->Is(Type::Symbol())) {
Node* left_input = graph()->NewNode(simplified()->CheckSymbol(), left(),
effect(), control());
node_->ReplaceInput(0, left_input);
update_effect(left_input);
}
if (!right_type()->Is(Type::Symbol())) {
Node* right_input = graph()->NewNode(simplified()->CheckSymbol(), right(),
effect(), control());
node_->ReplaceInput(1, right_input);
update_effect(right_input);
}
}
// Checks that both inputs are String, and if we don't know
// statically that one side is already a String, insert a
// CheckString node.
......@@ -896,6 +925,9 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node) {
} else if (r.IsStringCompareOperation()) {
r.CheckInputsToString();
return r.ChangeToPureOperator(simplified()->StringEqual());
} else if (r.IsSymbolCompareOperation()) {
r.CheckInputsToSymbol();
return r.ChangeToPureOperator(simplified()->ReferenceEqual());
}
return NoChange();
}
......@@ -953,6 +985,9 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node) {
} else if (r.IsStringCompareOperation()) {
r.CheckInputsToString();
return r.ChangeToPureOperator(simplified()->StringEqual());
} else if (r.IsSymbolCompareOperation()) {
r.CheckInputsToSymbol();
return r.ChangeToPureOperator(simplified()->ReferenceEqual());
}
return NoChange();
}
......
......@@ -321,6 +321,7 @@
V(CheckInternalizedString) \
V(CheckReceiver) \
V(CheckString) \
V(CheckSymbol) \
V(CheckSmi) \
V(CheckHeapObject) \
V(CheckFloat64Hole) \
......
......@@ -992,6 +992,18 @@ class RepresentationSelector {
}
}
void VisitCheck(Node* node, Type* type, SimplifiedLowering* lowering) {
if (InputIs(node, type)) {
VisitUnop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTaggedPointer);
if (lower()) DeferReplacement(node, node->InputAt(0));
} else {
VisitUnop(node, UseInfo::CheckedHeapObjectAsTaggedPointer(),
MachineRepresentation::kTaggedPointer);
}
return;
}
void VisitCall(Node* node, SimplifiedLowering* lowering) {
const CallDescriptor* desc = CallDescriptorOf(node->op());
int params = static_cast<int>(desc->ParameterCount());
......@@ -2390,14 +2402,7 @@ class RepresentationSelector {
return;
}
case IrOpcode::kCheckInternalizedString: {
if (InputIs(node, Type::InternalizedString())) {
VisitUnop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTaggedPointer);
if (lower()) DeferReplacement(node, node->InputAt(0));
} else {
VisitUnop(node, UseInfo::CheckedHeapObjectAsTaggedPointer(),
MachineRepresentation::kTaggedPointer);
}
VisitCheck(node, Type::InternalizedString(), lowering);
return;
}
case IrOpcode::kCheckNumber: {
......@@ -2410,14 +2415,7 @@ class RepresentationSelector {
return;
}
case IrOpcode::kCheckReceiver: {
if (InputIs(node, Type::Receiver())) {
VisitUnop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTaggedPointer);
if (lower()) DeferReplacement(node, node->InputAt(0));
} else {
VisitUnop(node, UseInfo::CheckedHeapObjectAsTaggedPointer(),
MachineRepresentation::kTaggedPointer);
}
VisitCheck(node, Type::Receiver(), lowering);
return;
}
case IrOpcode::kCheckSmi: {
......@@ -2433,14 +2431,11 @@ class RepresentationSelector {
return;
}
case IrOpcode::kCheckString: {
if (InputIs(node, Type::String())) {
VisitUnop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTaggedPointer);
if (lower()) DeferReplacement(node, node->InputAt(0));
} else {
VisitUnop(node, UseInfo::CheckedHeapObjectAsTaggedPointer(),
MachineRepresentation::kTaggedPointer);
VisitCheck(node, Type::String(), lowering);
return;
}
case IrOpcode::kCheckSymbol: {
VisitCheck(node, Type::Symbol(), lowering);
return;
}
......
......@@ -530,6 +530,7 @@ UnicodeEncoding UnicodeEncodingOf(const Operator* op) {
V(CheckReceiver, 1, 1) \
V(CheckSmi, 1, 1) \
V(CheckString, 1, 1) \
V(CheckSymbol, 1, 1) \
V(CheckTaggedHole, 1, 1) \
V(CheckedInt32Add, 2, 1) \
V(CheckedInt32Sub, 2, 1) \
......
......@@ -414,6 +414,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
const Operator* CheckNumber();
const Operator* CheckSmi();
const Operator* CheckString();
const Operator* CheckSymbol();
const Operator* CheckReceiver();
const Operator* CheckedInt32Add();
......
......@@ -1860,6 +1860,11 @@ Type* Typer::Visitor::TypeCheckString(Node* node) {
return Type::Intersect(arg, Type::String(), zone());
}
Type* Typer::Visitor::TypeCheckSymbol(Node* node) {
Type* arg = Operand(node, 0);
return Type::Intersect(arg, Type::Symbol(), zone());
}
Type* Typer::Visitor::TypeCheckFloat64Hole(Node* node) {
return typer_->operation_typer_.CheckFloat64Hole(Operand(node, 0));
}
......
......@@ -1178,6 +1178,10 @@ void Verifier::Visitor::Check(Node* node) {
CheckValueInputIs(node, 0, Type::Any());
CheckTypeIs(node, Type::String());
break;
case IrOpcode::kCheckSymbol:
CheckValueInputIs(node, 0, Type::Any());
CheckTypeIs(node, Type::Symbol());
break;
case IrOpcode::kCheckedInt32Add:
case IrOpcode::kCheckedInt32Sub:
......
......@@ -49,6 +49,7 @@ namespace internal {
V(NotAJavaScriptObject, "not a JavaScript object") \
V(NotANumberOrOddball, "not a Number or Oddball") \
V(NotASmi, "not a Smi") \
V(NotASymbol, "not a Symbol") \
V(OutOfBounds, "out of bounds") \
V(OutsideOfRange, "Outside of range") \
V(Overflow, "overflow") \
......
......@@ -173,6 +173,8 @@ CompareOperationHint CompareOperationHintFromFeedback(int type_feedback) {
return CompareOperationHint::kInternalizedString;
case CompareOperationFeedback::kString:
return CompareOperationHint::kString;
case CompareOperationFeedback::kSymbol:
return CompareOperationHint::kSymbol;
case CompareOperationFeedback::kReceiver:
return CompareOperationHint::kReceiver;
default:
......
......@@ -1278,6 +1278,7 @@ class BinaryOperationFeedback {
// to a more generic type when we combine feedback.
// kSignedSmall -> kNumber -> kAny
// kInternalizedString -> kString -> kAny
// kSymbol -> kAny
// kReceiver -> kAny
// TODO(epertoso): consider unifying this with BinaryOperationFeedback.
class CompareOperationFeedback {
......@@ -1289,8 +1290,9 @@ class CompareOperationFeedback {
kNumberOrOddball = 0x7,
kInternalizedString = 0x8,
kString = 0x18,
kReceiver = 0x20,
kAny = 0x7F
kSymbol = 0x20,
kReceiver = 0x40,
kAny = 0xff
};
};
......
......@@ -40,6 +40,8 @@ std::ostream& operator<<(std::ostream& os, CompareOperationHint hint) {
return os << "InternalizedString";
case CompareOperationHint::kString:
return os << "String";
case CompareOperationHint::kSymbol:
return os << "Symbol";
case CompareOperationHint::kReceiver:
return os << "Receiver";
case CompareOperationHint::kAny:
......
......@@ -35,6 +35,7 @@ enum class CompareOperationHint : uint8_t {
kNumberOrOddball,
kInternalizedString,
kString,
kSymbol,
kReceiver,
kAny
};
......
......@@ -191,6 +191,8 @@ AstType* CompareOpHintToType(CompareOperationHint hint) {
return AstType::InternalizedString();
case CompareOperationHint::kString:
return AstType::String();
case CompareOperationHint::kSymbol:
UNREACHABLE();
case CompareOperationHint::kReceiver:
return AstType::Receiver();
case CompareOperationHint::kAny:
......
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