[Interpreter] Inline feedback collection in relational compare bytecode handlers.

This is the last in the series of simplifying the logic to collect feedback
in compare bytecode handlers. This cl inlines the type feedback collection
for the relational compare (lessthan, lessthan or equal, greater than,
gerater than or equal) bytecode handlers.

Bug: v8:4280
Change-Id: I4a896c9cbe5628c76785882c0632bfa07b18b099
Reviewed-on: https://chromium-review.googlesource.com/500309
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45296}
This commit is contained in:
Mythri 2017-05-09 14:36:10 +01:00 committed by Commit Bot
parent 5ce6090d1b
commit 4acca5ba88
3 changed files with 122 additions and 265 deletions

View File

@ -6983,7 +6983,8 @@ void CodeStubAssembler::GotoUnlessNumberLessThan(Node* lhs, Node* rhs,
Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
Node* lhs, Node* rhs,
Node* context) {
Node* context,
Variable* var_type_feedback) {
Label return_true(this), return_false(this), end(this);
VARIABLE(result, MachineRepresentation::kTagged);
@ -6996,8 +6997,14 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
// conversions.
VARIABLE(var_lhs, MachineRepresentation::kTagged, lhs);
VARIABLE(var_rhs, MachineRepresentation::kTagged, rhs);
Variable* loop_vars[2] = {&var_lhs, &var_rhs};
Label loop(this, 2, loop_vars);
VariableList loop_variable_list({&var_lhs, &var_rhs}, zone());
if (var_type_feedback != nullptr) {
// Initialize the type feedback to None. The current feedback is combined
// with the previous feedback.
var_type_feedback->Bind(SmiConstant(CompareOperationFeedback::kNone));
loop_variable_list.Add(var_type_feedback, zone());
}
Label loop(this, loop_variable_list);
Goto(&loop);
BIND(&loop);
{
@ -7018,6 +7025,10 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_rhsissmi);
{
// Both {lhs} and {rhs} are Smi, so just perform a fast Smi comparison.
if (var_type_feedback != nullptr) {
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kSignedSmall));
}
switch (mode) {
case kLessThan:
BranchIfSmiLessThan(lhs, rhs, &return_true, &return_false);
@ -7047,6 +7058,10 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
{
// Convert the {lhs} and {rhs} to floating point values, and
// perform a floating point comparison.
if (var_type_feedback != nullptr) {
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kNumber));
}
var_fcmp_lhs.Bind(SmiToFloat64(lhs));
var_fcmp_rhs.Bind(LoadHeapNumberValue(rhs));
Goto(&do_fcmp);
@ -7054,6 +7069,11 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_rhsisnotnumber);
{
// The {rhs} is not a HeapNumber and {lhs} is an Smi.
if (var_type_feedback != nullptr) {
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
}
// Convert the {rhs} to a Number; we don't need to perform the
// dedicated ToPrimitive(rhs, hint Number) operation, as the
// ToNumber(rhs) will by itself already invoke ToPrimitive with
@ -7084,6 +7104,10 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
{
// Convert the {lhs} and {rhs} to floating point values, and
// perform a floating point comparison.
if (var_type_feedback != nullptr) {
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kNumber));
}
var_fcmp_lhs.Bind(LoadHeapNumberValue(lhs));
var_fcmp_rhs.Bind(SmiToFloat64(rhs));
Goto(&do_fcmp);
@ -7091,6 +7115,11 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_lhsisnotnumber);
{
// The {lhs} is not a HeapNumber and {rhs} is an Smi.
if (var_type_feedback != nullptr) {
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
}
// Convert the {lhs} to a Number; we don't need to perform the
// dedicated ToPrimitive(lhs, hint Number) operation, as the
// ToNumber(lhs) will by itself already invoke ToPrimitive with
@ -7121,6 +7150,10 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
{
// Convert the {lhs} and {rhs} to floating point values, and
// perform a floating point comparison.
if (var_type_feedback != nullptr) {
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kNumber));
}
var_fcmp_lhs.Bind(LoadHeapNumberValue(lhs));
var_fcmp_rhs.Bind(LoadHeapNumberValue(rhs));
Goto(&do_fcmp);
@ -7128,6 +7161,11 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_rhsisnotnumber);
{
// The {rhs} is not a HeapNumber and {lhs} is a HeapNumber.
if (var_type_feedback != nullptr) {
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
}
// Convert the {rhs} to a Number; we don't need to perform
// dedicated ToPrimitive(rhs, hint Number) operation, as the
// ToNumber(rhs) will by itself already invoke ToPrimitive with
@ -7162,6 +7200,10 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_rhsisstring);
{
// Both {lhs} and {rhs} are strings.
if (var_type_feedback != nullptr) {
CombineFeedback(var_type_feedback,
SmiConstant(CompareOperationFeedback::kString));
}
switch (mode) {
case kLessThan:
result.Bind(CallStub(CodeFactory::StringLessThan(isolate()),
@ -7191,6 +7233,11 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_rhsisnotstring);
{
// The {lhs} is a String and {rhs} is not a String.
if (var_type_feedback != nullptr) {
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
}
// The {lhs} is a String, while {rhs} is neither a Number nor a
// String, so we need to call ToPrimitive(rhs, hint Number) if
// {rhs} is a receiver or ToNumber(lhs) and ToNumber(rhs) in the
@ -7223,6 +7270,41 @@ Node* CodeStubAssembler::RelationalComparison(RelationalComparisonMode mode,
BIND(&if_lhsisnotstring);
{
if (var_type_feedback != nullptr) {
// The {lhs} is not an Smi, HeapNumber or String and {rhs} is not
// an Smi: collect NumberOrOddball feedback if {lhs} is an Oddball
// and {rhs} is either a HeapNumber or Oddball.
Label collect_any_feedback(this), collect_oddball_feedback(this),
collect_feedback_done(this);
GotoIfNot(
Word32Equal(lhs_instance_type, Int32Constant(ODDBALL_TYPE)),
&collect_any_feedback);
Node* rhs_instance_type = LoadMapInstanceType(rhs_map);
GotoIf(Word32Equal(rhs_instance_type,
Int32Constant(HEAP_NUMBER_TYPE)),
&collect_oddball_feedback);
Branch(
Word32Equal(rhs_instance_type, Int32Constant(ODDBALL_TYPE)),
&collect_oddball_feedback, &collect_any_feedback);
BIND(&collect_oddball_feedback);
{
CombineFeedback(
var_type_feedback,
SmiConstant(CompareOperationFeedback::kNumberOrOddball));
Goto(&collect_feedback_done);
}
BIND(&collect_any_feedback);
{
var_type_feedback->Bind(
SmiConstant(CompareOperationFeedback::kAny));
Goto(&collect_feedback_done);
}
BIND(&collect_feedback_done);
}
// The {lhs} is neither a Number nor a String, so we need to call
// ToPrimitive(lhs, hint Number) if {lhs} is a receiver or
// ToNumber(lhs) and ToNumber(rhs) in the other cases.

View File

@ -1351,7 +1351,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
};
Node* RelationalComparison(RelationalComparisonMode mode, Node* lhs,
Node* rhs, Node* context);
Node* rhs, Node* context,
Variable* var_type_feedback = nullptr);
void BranchIfNumericRelationalComparison(RelationalComparisonMode mode,
Node* lhs, Node* rhs, Label* if_true,

View File

@ -2124,244 +2124,41 @@ class InterpreterCompareOpAssembler : public InterpreterAssembler {
Node* lhs = LoadRegister(reg_index);
Node* rhs = GetAccumulator();
Node* context = GetContext();
Variable var_type_feedback(this, MachineRepresentation::kTagged);
Node* result;
switch (compare_op) {
case Token::EQ:
result = Equal(lhs, rhs, context, &var_type_feedback);
break;
case Token::EQ_STRICT:
result = StrictEqual(lhs, rhs, &var_type_feedback);
break;
case Token::LT:
result = RelationalComparison(CodeStubAssembler::kLessThan, lhs, rhs,
context, &var_type_feedback);
break;
case Token::GT:
result = RelationalComparison(CodeStubAssembler::kGreaterThan, lhs, rhs,
context, &var_type_feedback);
break;
case Token::LTE:
result = RelationalComparison(CodeStubAssembler::kLessThanOrEqual, lhs,
rhs, context, &var_type_feedback);
break;
case Token::GTE:
result = RelationalComparison(CodeStubAssembler::kGreaterThanOrEqual,
lhs, rhs, context, &var_type_feedback);
break;
default:
UNREACHABLE();
}
Node* slot_index = BytecodeOperandIdx(1);
Node* feedback_vector = LoadFeedbackVector();
Variable var_result(this, MachineRepresentation::kTagged),
var_fcmp_lhs(this, MachineRepresentation::kFloat64),
var_fcmp_rhs(this, MachineRepresentation::kFloat64),
non_number_value(this, MachineRepresentation::kTagged),
maybe_smi_value(this, MachineRepresentation::kTagged);
Label lhs_is_not_smi(this), do_fcmp(this), slow_path(this),
fast_path_dispatch(this);
GotoIf(TaggedIsNotSmi(lhs), &lhs_is_not_smi);
{
Label rhs_is_not_smi(this);
GotoIf(TaggedIsNotSmi(rhs), &rhs_is_not_smi);
{
Comment("Do integer comparison");
UpdateFeedback(SmiConstant(CompareOperationFeedback::kSignedSmall),
feedback_vector, slot_index);
Node* result;
switch (compare_op) {
case Token::LT:
result = SelectBooleanConstant(SmiLessThan(lhs, rhs));
break;
case Token::LTE:
result = SelectBooleanConstant(SmiLessThanOrEqual(lhs, rhs));
break;
case Token::GT:
result = SelectBooleanConstant(SmiLessThan(rhs, lhs));
break;
case Token::GTE:
result = SelectBooleanConstant(SmiLessThanOrEqual(rhs, lhs));
break;
default:
UNREACHABLE();
}
var_result.Bind(result);
Goto(&fast_path_dispatch);
}
Bind(&rhs_is_not_smi);
{
Node* rhs_map = LoadMap(rhs);
Label rhs_is_not_number(this);
GotoIfNot(IsHeapNumberMap(rhs_map), &rhs_is_not_number);
Comment("Convert lhs to float and load HeapNumber value from rhs");
var_fcmp_lhs.Bind(SmiToFloat64(lhs));
var_fcmp_rhs.Bind(LoadHeapNumberValue(rhs));
Goto(&do_fcmp);
Bind(&rhs_is_not_number);
{
non_number_value.Bind(rhs);
maybe_smi_value.Bind(lhs);
Goto(&slow_path);
}
}
}
Bind(&lhs_is_not_smi);
{
Label rhs_is_not_smi(this), lhs_is_not_number(this),
rhs_is_not_number(this);
Node* lhs_map = LoadMap(lhs);
GotoIfNot(IsHeapNumberMap(lhs_map), &lhs_is_not_number);
GotoIfNot(TaggedIsSmi(rhs), &rhs_is_not_smi);
Comment("Convert rhs to double and load HeapNumber value from lhs");
var_fcmp_lhs.Bind(LoadHeapNumberValue(lhs));
var_fcmp_rhs.Bind(SmiToFloat64(rhs));
Goto(&do_fcmp);
Bind(&rhs_is_not_smi);
{
Node* rhs_map = LoadMap(rhs);
GotoIfNot(IsHeapNumberMap(rhs_map), &rhs_is_not_number);
Comment("Load HeapNumber values from lhs and rhs");
var_fcmp_lhs.Bind(LoadHeapNumberValue(lhs));
var_fcmp_rhs.Bind(LoadHeapNumberValue(rhs));
Goto(&do_fcmp);
}
Bind(&lhs_is_not_number);
{
non_number_value.Bind(lhs);
maybe_smi_value.Bind(rhs);
Goto(&slow_path);
}
Bind(&rhs_is_not_number);
{
non_number_value.Bind(rhs);
maybe_smi_value.Bind(lhs);
Goto(&slow_path);
}
}
Bind(&do_fcmp);
{
Comment("Do floating point comparison");
Node* lhs_float = var_fcmp_lhs.value();
Node* rhs_float = var_fcmp_rhs.value();
UpdateFeedback(SmiConstant(CompareOperationFeedback::kNumber),
feedback_vector, slot_index);
// Perform a fast floating point comparison.
Node* result;
switch (compare_op) {
case Token::LT:
result = SelectBooleanConstant(Float64LessThan(lhs_float, rhs_float));
break;
case Token::LTE:
result = SelectBooleanConstant(
Float64LessThanOrEqual(lhs_float, rhs_float));
break;
case Token::GT:
result =
SelectBooleanConstant(Float64GreaterThan(lhs_float, rhs_float));
break;
case Token::GTE:
result = SelectBooleanConstant(
Float64GreaterThanOrEqual(lhs_float, rhs_float));
break;
default:
UNREACHABLE();
}
var_result.Bind(result);
Goto(&fast_path_dispatch);
}
Bind(&fast_path_dispatch);
{
SetAccumulator(var_result.value());
Dispatch();
}
// Marking a block with more than one predecessor causes register allocator
// to fail (v8:5998). Add a dummy block as a workaround.
Label slow_path_deferred(this, Label::kDeferred);
Bind(&slow_path);
Goto(&slow_path_deferred);
Bind(&slow_path_deferred);
{
// When we reach here, one of the operands is not a Smi / HeapNumber and
// the other operand could be of any type. The cases where both of them
// are HeapNumbers / Smis are handled earlier.
Comment("Collect feedback for non HeapNumber cases.");
Label update_feedback_and_do_compare(this);
Variable var_type_feedback(this, MachineRepresentation::kTaggedSigned);
var_type_feedback.Bind(SmiConstant(CompareOperationFeedback::kAny));
DCHECK(!Token::IsEqualityOp(compare_op));
Label check_for_oddball(this);
// Check for NumberOrOddball feedback.
Node* non_number_instance_type =
LoadInstanceType(non_number_value.value());
GotoIf(Word32Equal(non_number_instance_type, Int32Constant(ODDBALL_TYPE)),
&check_for_oddball);
// Check for string feedback.
GotoIfNot(IsStringInstanceType(non_number_instance_type),
&update_feedback_and_do_compare);
GotoIf(TaggedIsSmi(maybe_smi_value.value()),
&update_feedback_and_do_compare);
Node* maybe_smi_instance_type = LoadInstanceType(maybe_smi_value.value());
GotoIfNot(IsStringInstanceType(maybe_smi_instance_type),
&update_feedback_and_do_compare);
var_type_feedback.Bind(SmiConstant(CompareOperationFeedback::kString));
Goto(&update_feedback_and_do_compare);
Bind(&check_for_oddball);
{
Label compare_with_oddball_feedback(this);
GotoIf(TaggedIsSmi(maybe_smi_value.value()),
&compare_with_oddball_feedback);
Node* maybe_smi_instance_type =
LoadInstanceType(maybe_smi_value.value());
GotoIf(Word32Equal(maybe_smi_instance_type,
Int32Constant(HEAP_NUMBER_TYPE)),
&compare_with_oddball_feedback);
Branch(
Word32Equal(maybe_smi_instance_type, Int32Constant(ODDBALL_TYPE)),
&compare_with_oddball_feedback, &update_feedback_and_do_compare);
Bind(&compare_with_oddball_feedback);
{
var_type_feedback.Bind(
SmiConstant(CompareOperationFeedback::kNumberOrOddball));
Goto(&update_feedback_and_do_compare);
}
}
Bind(&update_feedback_and_do_compare);
{
Comment("Do the full compare operation");
UpdateFeedback(var_type_feedback.value(), feedback_vector, slot_index);
Node* result;
switch (compare_op) {
case Token::EQ:
result = Equal(lhs, rhs, context);
break;
case Token::EQ_STRICT:
result = StrictEqual(lhs, rhs);
break;
case Token::LT:
result = RelationalComparison(CodeStubAssembler::kLessThan, lhs,
rhs, context);
break;
case Token::GT:
result = RelationalComparison(CodeStubAssembler::kGreaterThan, lhs,
rhs, context);
break;
case Token::LTE:
result = RelationalComparison(CodeStubAssembler::kLessThanOrEqual,
lhs, rhs, context);
break;
case Token::GTE:
result = RelationalComparison(
CodeStubAssembler::kGreaterThanOrEqual, lhs, rhs, context);
break;
default:
UNREACHABLE();
}
var_result.Bind(result);
SetAccumulator(var_result.value());
Dispatch();
}
}
UpdateFeedback(var_type_feedback.value(), feedback_vector, slot_index);
SetAccumulator(result);
Dispatch();
}
};
@ -2369,37 +2166,14 @@ class InterpreterCompareOpAssembler : public InterpreterAssembler {
//
// Test if the value in the <src> register equals the accumulator.
IGNITION_HANDLER(TestEqual, InterpreterCompareOpAssembler) {
Node* reg_index = BytecodeOperandReg(0);
Node* lhs = LoadRegister(reg_index);
Node* rhs = GetAccumulator();
Node* context = GetContext();
Variable var_type_feedback(this, MachineRepresentation::kTaggedSigned);
Node* result = Equal(lhs, rhs, context, &var_type_feedback);
Node* slot_index = BytecodeOperandIdx(1);
Node* feedback_vector = LoadFeedbackVector();
UpdateFeedback(var_type_feedback.value(), feedback_vector, slot_index);
SetAccumulator(result);
Dispatch();
CompareOpWithFeedback(Token::Value::EQ);
}
// TestEqualStrict <src>
//
// Test if the value in the <src> register is strictly equal to the accumulator.
IGNITION_HANDLER(TestEqualStrict, InterpreterCompareOpAssembler) {
Node* reg_index = BytecodeOperandReg(0);
Node* lhs = LoadRegister(reg_index);
Node* rhs = GetAccumulator();
Variable var_type_feedback(this, MachineRepresentation::kTaggedSigned);
Node* result = StrictEqual(lhs, rhs, &var_type_feedback);
Node* slot_index = BytecodeOperandIdx(1);
Node* feedback_vector = LoadFeedbackVector();
UpdateFeedback(var_type_feedback.value(), feedback_vector, slot_index);
SetAccumulator(result);
Dispatch();
CompareOpWithFeedback(Token::Value::EQ_STRICT);
}
// TestLessThan <src>