Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(311)

Unified Diff: src/ia32/codegen-ia32.cc

Issue 1279001: Revert "Inline floating point compare" (Closed)
Patch Set: Created 10 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/ia32/codegen-ia32.h ('k') | src/jump-target.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/ia32/codegen-ia32.cc
diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index 7c115c5846219c06550ecccf2d1c18a33ac859a5..d71b63988d43b4380a2e952feb0ba4c9a69e006e 100644
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -911,7 +911,6 @@ class FloatingPointHelper : public AllStatic {
// operand in register number. Returns operand as floating point number
// on FPU stack.
static void LoadFloatOperand(MacroAssembler* masm, Register number);
-
// Code pattern for loading floating point values. Input values must
// be either smi or heap number objects (fp values). Requirements:
// operand_1 on TOS+1 or in edx, operand_2 on TOS+2 or in eax.
@@ -930,7 +929,6 @@ class FloatingPointHelper : public AllStatic {
static void CheckFloatOperands(MacroAssembler* masm,
Label* non_float,
Register scratch);
-
// Takes the operands in edx and eax and loads them as integers in eax
// and ecx.
static void LoadAsIntegers(MacroAssembler* masm,
@@ -949,7 +947,6 @@ class FloatingPointHelper : public AllStatic {
// into xmm0 and xmm1 if they are. Operands are in edx and eax.
// Leaves operands unchanged.
static void LoadSSE2Operands(MacroAssembler* masm);
-
// Test if operands are numbers (smi or HeapNumber objects), and load
// them into xmm0 and xmm1 if they are. Jump to label not_numbers if
// either operand is not a number. Operands are in edx and eax.
@@ -2364,22 +2361,6 @@ static bool CouldBeNaN(const Result& result) {
}
-// Convert from signed to unsigned comparison to match the way EFLAGS are set
-// by FPU and XMM compare instructions.
-static Condition DoubleCondition(Condition cc) {
- switch (cc) {
- case less: return below;
- case equal: return equal;
- case less_equal: return below_equal;
- case greater: return above;
- case greater_equal: return above_equal;
- default: UNREACHABLE();
- }
- UNREACHABLE();
- return equal;
-}
-
-
void CodeGenerator::Comparison(AstNode* node,
Condition cc,
bool strict,
@@ -2450,7 +2431,7 @@ void CodeGenerator::Comparison(AstNode* node,
left_side = right_side;
right_side = temp;
cc = ReverseCondition(cc);
- // This may re-introduce greater or less_equal as the value of cc.
+ // This may reintroduce greater or less_equal as the value of cc.
// CompareStub and the inline code both support all values of cc.
}
// Implement comparison against a constant Smi, inlining the case
@@ -2499,7 +2480,16 @@ void CodeGenerator::Comparison(AstNode* node,
// Jump to builtin for NaN.
not_number.Branch(parity_even, &left_side);
left_side.Unuse();
- dest->true_target()->Branch(DoubleCondition(cc));
+ Condition double_cc = cc;
+ switch (cc) {
+ case less: double_cc = below; break;
+ case equal: double_cc = equal; break;
+ case less_equal: double_cc = below_equal; break;
+ case greater: double_cc = above; break;
+ case greater_equal: double_cc = above_equal; break;
+ default: UNREACHABLE();
+ }
+ dest->true_target()->Branch(double_cc);
dest->false_target()->Jump();
not_number.Bind(&left_side);
}
@@ -2698,53 +2688,21 @@ void CodeGenerator::Comparison(AstNode* node,
dest->Split(cc);
}
} else {
- // Neither side is a constant Smi, constant 1-char string or constant null.
- // If either side is a non-smi constant, or known to be a heap number skip
- // the smi check.
+ // Neither side is a constant Smi or null.
+ // If either side is a non-smi constant, skip the smi check.
bool known_non_smi =
(left_side.is_constant() && !left_side.handle()->IsSmi()) ||
- (right_side.is_constant() && !right_side.handle()->IsSmi()) ||
- left_side.number_info().IsHeapNumber() ||
- right_side.number_info().IsHeapNumber();
+ (right_side.is_constant() && !right_side.handle()->IsSmi());
NaNInformation nan_info =
(CouldBeNaN(left_side) && CouldBeNaN(right_side)) ?
kBothCouldBeNaN :
kCantBothBeNaN;
-
- // Inline number comparison handling any combination of smi's and heap
- // numbers if:
- // code is in a loop
- // the compare operation is different from equal
- // compare is not a for-loop comparison
- // The reason for excluding equal is that it will most likely be done
- // with smi's (not heap numbers) and the code to comparing smi's is inlined
- // separately. The same reason applies for for-loop comparison which will
- // also most likely be smi comparisons.
- bool is_loop_condition = (node->AsExpression() != NULL)
- && node->AsExpression()->is_loop_condition();
- bool inline_number_compare =
- loop_nesting() > 0 && cc != equal && !is_loop_condition;
-
- // Left and right needed in registers for the following code.
left_side.ToRegister();
right_side.ToRegister();
if (known_non_smi) {
- // Inline the equality check if both operands can't be a NaN. If both
- // objects are the same they are equal.
- if (nan_info == kCantBothBeNaN && cc == equal) {
- __ cmp(left_side.reg(), Operand(right_side.reg()));
- dest->true_target()->Branch(equal);
- }
-
- // Inline number comparison.
- if (inline_number_compare) {
- GenerateInlineNumberComparison(&left_side, &right_side, cc, dest);
- }
-
- // End of in-line compare, call out to the compare stub. Don't include
- // number comparison in the stub if it was inlined.
- CompareStub stub(cc, strict, nan_info, !inline_number_compare);
+ // When non-smi, call out to the compare stub.
+ CompareStub stub(cc, strict, nan_info);
Result answer = frame_->CallStub(&stub, &left_side, &right_side);
if (cc == equal) {
__ test(answer.reg(), Operand(answer.reg()));
@@ -2763,7 +2721,6 @@ void CodeGenerator::Comparison(AstNode* node,
Register left_reg = left_side.reg();
Register right_reg = right_side.reg();
- // In-line check for comparing two smis.
Result temp = allocator_->Allocate();
ASSERT(temp.is_valid());
__ mov(temp.reg(), left_side.reg());
@@ -2771,22 +2728,8 @@ void CodeGenerator::Comparison(AstNode* node,
__ test(temp.reg(), Immediate(kSmiTagMask));
temp.Unuse();
is_smi.Branch(zero, taken);
-
- // Inline the equality check if both operands can't be a NaN. If both
- // objects are the same they are equal.
- if (nan_info == kCantBothBeNaN && cc == equal) {
- __ cmp(left_side.reg(), Operand(right_side.reg()));
- dest->true_target()->Branch(equal);
- }
-
- // Inline number comparison.
- if (inline_number_compare) {
- GenerateInlineNumberComparison(&left_side, &right_side, cc, dest);
- }
-
- // End of in-line compare, call out to the compare stub. Don't include
- // number comparison in the stub if it was inlined.
- CompareStub stub(cc, strict, nan_info, !inline_number_compare);
+ // When non-smi, call out to the compare stub.
+ CompareStub stub(cc, strict, nan_info);
Result answer = frame_->CallStub(&stub, &left_side, &right_side);
if (cc == equal) {
__ test(answer.reg(), Operand(answer.reg()));
@@ -2809,150 +2752,6 @@ void CodeGenerator::Comparison(AstNode* node,
}
-// Check that the comparison operand is a number. Jump to not_numbers jump
-// target passing the left and right result if the operand is not a number.
-static void CheckComparisonOperand(MacroAssembler* masm_,
- Result* operand,
- Result* left_side,
- Result* right_side,
- JumpTarget* not_numbers) {
- // Perform check if operand is not known to be a number.
- if (!operand->number_info().IsNumber()) {
- Label done;
- __ test(operand->reg(), Immediate(kSmiTagMask));
- __ j(zero, &done);
- __ cmp(FieldOperand(operand->reg(), HeapObject::kMapOffset),
- Immediate(Factory::heap_number_map()));
- not_numbers->Branch(not_equal, left_side, right_side, not_taken);
- __ bind(&done);
- }
-}
-
-
-// Load a comparison operand to the FPU stack. This assumes that the operand has
-// already been checked and is a number.
-static void LoadComparisonOperand(MacroAssembler* masm_,
- Result* operand,
- Result* left_side,
- Result* right_side) {
- Label done;
- if (operand->number_info().IsHeapNumber()) {
- // Operand is known to be a heap number, just load it.
- __ fld_d(FieldOperand(operand->reg(), HeapNumber::kValueOffset));
- } else if (operand->number_info().IsSmi()) {
- // Operand is known to be a smi. Convert it to double and keep the original
- // smi.
- __ SmiUntag(operand->reg());
- __ push(operand->reg());
- __ fild_s(Operand(esp, 0));
- __ pop(operand->reg());
- __ SmiTag(operand->reg());
- } else {
- // Operand type not known, check for smi otherwise assume heap number.
- Label smi;
- __ test(operand->reg(), Immediate(kSmiTagMask));
- __ j(zero, &smi);
- __ fld_d(FieldOperand(operand->reg(), HeapNumber::kValueOffset));
- __ jmp(&done);
- __ bind(&smi);
- __ SmiUntag(operand->reg());
- __ push(operand->reg());
- __ fild_s(Operand(esp, 0));
- __ pop(operand->reg());
- __ SmiTag(operand->reg());
- __ jmp(&done);
- }
- __ bind(&done);
-}
-
-
-// Load a comparison operand into into a XMM register. Jump to not_numbers jump
-// target passing the left and right result if the operand is not a number.
-static void LoadComparisonOperandSSE2(MacroAssembler* masm_,
- Result* operand,
- XMMRegister reg,
- Result* left_side,
- Result* right_side,
- JumpTarget* not_numbers) {
- Label done;
- if (operand->number_info().IsHeapNumber()) {
- // Operand is known to be a heap number, just load it.
- __ movdbl(reg, FieldOperand(operand->reg(), HeapNumber::kValueOffset));
- } else if (operand->number_info().IsSmi()) {
- // Operand is known to be a smi. Convert it to double and keep the original
- // smi.
- __ SmiUntag(operand->reg());
- __ cvtsi2sd(reg, Operand(operand->reg()));
- __ SmiTag(left_side->reg());
- } else {
- // Operand type not known, check for smi or heap number.
- Label smi;
- __ test(operand->reg(), Immediate(kSmiTagMask));
- __ j(zero, &smi);
- if (!operand->number_info().IsNumber()) {
- __ cmp(FieldOperand(operand->reg(), HeapObject::kMapOffset),
- Immediate(Factory::heap_number_map()));
- not_numbers->Branch(not_equal, left_side, right_side, taken);
- }
- __ movdbl(reg, FieldOperand(operand->reg(), HeapNumber::kValueOffset));
- __ jmp(&done);
-
- __ bind(&smi);
- // Comvert smi to float and keep the original smi.
- __ SmiUntag(operand->reg());
- __ cvtsi2sd(reg, Operand(operand->reg()));
- __ SmiTag(operand->reg());
- __ jmp(&done);
- }
- __ bind(&done);
-}
-
-
-void CodeGenerator::GenerateInlineNumberComparison(Result* left_side,
- Result* right_side,
- Condition cc,
- ControlDestination* dest) {
- ASSERT(left_side->is_register());
- ASSERT(right_side->is_register());
-
- JumpTarget not_numbers;
- if (CpuFeatures::IsSupported(SSE2)) {
- CpuFeatures::Scope use_sse2(SSE2);
-
- // Load left and right operand into registers xmm0 and xmm1 and compare.
- LoadComparisonOperandSSE2(masm_, left_side, xmm0, left_side, right_side,
- &not_numbers);
- LoadComparisonOperandSSE2(masm_, right_side, xmm1, left_side, right_side,
- &not_numbers);
- __ comisd(xmm0, xmm1);
- } else {
- Label check_right, compare;
-
- // Make sure that both comparison operands are numbers.
- CheckComparisonOperand(masm_, left_side, left_side, right_side,
- &not_numbers);
- CheckComparisonOperand(masm_, right_side, left_side, right_side,
- &not_numbers);
-
- // Load right and left operand to FPU stack and compare.
- LoadComparisonOperand(masm_, right_side, left_side, right_side);
- LoadComparisonOperand(masm_, left_side, left_side, right_side);
- __ FCmp();
- }
-
- // Bail out if a NaN is involved.
- not_numbers.Branch(parity_even, left_side, right_side, not_taken);
-
- // Split to destination targets based on comparison.
- left_side->Unuse();
- right_side->Unuse();
- dest->true_target()->Branch(DoubleCondition(cc));
- dest->false_target()->Jump();
-
- not_numbers.Bind(left_side, right_side);
-}
-
-
// Call the function just below TOS on the stack with the given
// arguments. The receiver is the TOS.
void CodeGenerator::CallWithArguments(ZoneList<Expression*>* args,
@@ -11078,70 +10877,63 @@ void CompareStub::Generate(MacroAssembler* masm) {
__ push(edx);
__ push(ecx);
- // Generate the number comparison code.
- if (include_number_compare_) {
- Label non_number_comparison;
- Label unordered;
- if (CpuFeatures::IsSupported(SSE2)) {
- CpuFeatures::Scope use_sse2(SSE2);
- CpuFeatures::Scope use_cmov(CMOV);
-
- FloatingPointHelper::LoadSSE2Operands(masm, &non_number_comparison);
- __ comisd(xmm0, xmm1);
-
- // Don't base result on EFLAGS when a NaN is involved.
- __ j(parity_even, &unordered, not_taken);
- // Return a result of -1, 0, or 1, based on EFLAGS.
- __ mov(eax, 0); // equal
- __ mov(ecx, Immediate(Smi::FromInt(1)));
- __ cmov(above, eax, Operand(ecx));
- __ mov(ecx, Immediate(Smi::FromInt(-1)));
- __ cmov(below, eax, Operand(ecx));
- __ ret(2 * kPointerSize);
- } else {
- FloatingPointHelper::CheckFloatOperands(
- masm, &non_number_comparison, ebx);
- FloatingPointHelper::LoadFloatOperands(masm, ecx);
- __ FCmp();
+ // Inlined floating point compare.
+ // Call builtin if operands are not floating point or smi.
+ Label check_for_symbols;
+ Label unordered;
+ if (CpuFeatures::IsSupported(SSE2)) {
+ CpuFeatures::Scope use_sse2(SSE2);
+ CpuFeatures::Scope use_cmov(CMOV);
- // Don't base result on EFLAGS when a NaN is involved.
- __ j(parity_even, &unordered, not_taken);
+ FloatingPointHelper::LoadSSE2Operands(masm, &check_for_symbols);
+ __ comisd(xmm0, xmm1);
- Label below_label, above_label;
- // Return a result of -1, 0, or 1, based on EFLAGS. In all cases remove
- // two arguments from the stack as they have been pushed in preparation
- // of a possible runtime call.
- __ j(below, &below_label, not_taken);
- __ j(above, &above_label, not_taken);
+ // Jump to builtin for NaN.
+ __ j(parity_even, &unordered, not_taken);
+ __ mov(eax, 0); // equal
+ __ mov(ecx, Immediate(Smi::FromInt(1)));
+ __ cmov(above, eax, Operand(ecx));
+ __ mov(ecx, Immediate(Smi::FromInt(-1)));
+ __ cmov(below, eax, Operand(ecx));
+ __ ret(2 * kPointerSize);
+ } else {
+ FloatingPointHelper::CheckFloatOperands(masm, &check_for_symbols, ebx);
+ FloatingPointHelper::LoadFloatOperands(masm, ecx);
+ __ FCmp();
- __ xor_(eax, Operand(eax));
- __ ret(2 * kPointerSize);
+ // Jump to builtin for NaN.
+ __ j(parity_even, &unordered, not_taken);
- __ bind(&below_label);
- __ mov(eax, Immediate(Smi::FromInt(-1)));
- __ ret(2 * kPointerSize);
+ Label below_lbl, above_lbl;
+ // Return a result of -1, 0, or 1, to indicate result of comparison.
+ __ j(below, &below_lbl, not_taken);
+ __ j(above, &above_lbl, not_taken);
- __ bind(&above_label);
- __ mov(eax, Immediate(Smi::FromInt(1)));
- __ ret(2 * kPointerSize);
- }
+ __ xor_(eax, Operand(eax)); // equal
+ // Both arguments were pushed in case a runtime call was needed.
+ __ ret(2 * kPointerSize);
- // If one of the numbers was NaN, then the result is always false.
- // The cc is never not-equal.
- __ bind(&unordered);
- ASSERT(cc_ != not_equal);
- if (cc_ == less || cc_ == less_equal) {
- __ mov(eax, Immediate(Smi::FromInt(1)));
- } else {
- __ mov(eax, Immediate(Smi::FromInt(-1)));
- }
- __ ret(2 * kPointerSize); // eax, edx were pushed
+ __ bind(&below_lbl);
+ __ mov(eax, Immediate(Smi::FromInt(-1)));
+ __ ret(2 * kPointerSize);
- // The number comparison code did not provide a valid result.
- __ bind(&non_number_comparison);
+ __ bind(&above_lbl);
+ __ mov(eax, Immediate(Smi::FromInt(1)));
+ __ ret(2 * kPointerSize); // eax, edx were pushed
+ }
+ // If one of the numbers was NaN, then the result is always false.
+ // The cc is never not-equal.
+ __ bind(&unordered);
+ ASSERT(cc_ != not_equal);
+ if (cc_ == less || cc_ == less_equal) {
+ __ mov(eax, Immediate(Smi::FromInt(1)));
+ } else {
+ __ mov(eax, Immediate(Smi::FromInt(-1)));
}
+ __ ret(2 * kPointerSize); // eax, edx were pushed
// Fast negative check for symbol-to-symbol equality.
+ __ bind(&check_for_symbols);
Label check_for_strings;
if (cc_ == equal) {
BranchIfNonSymbol(masm, &check_for_strings, eax, ecx);
@@ -11751,59 +11543,57 @@ void InstanceofStub::Generate(MacroAssembler* masm) {
}
-int CompareStub::MinorKey() {
- // Encode the three parameters in a unique 16 bit value. To avoid duplicate
- // stubs the never NaN NaN condition is only taken into account if the
- // condition is equals.
- ASSERT(static_cast<unsigned>(cc_) < (1 << 13));
- return ConditionField::encode(static_cast<unsigned>(cc_))
- | StrictField::encode(strict_)
- | NeverNanNanField::encode(cc_ == equal ? never_nan_nan_ : false)
- | IncludeNumberCompareField::encode(include_number_compare_);
-}
-
-
// Unfortunately you have to run without snapshots to see most of these
// names in the profile since most compare stubs end up in the snapshot.
const char* CompareStub::GetName() {
- if (name_ != NULL) return name_;
- const int kMaxNameLength = 100;
- name_ = Bootstrapper::AllocateAutoDeletedArray(kMaxNameLength);
- if (name_ == NULL) return "OOM";
-
- const char* cc_name;
switch (cc_) {
- case less: cc_name = "LT"; break;
- case greater: cc_name = "GT"; break;
- case less_equal: cc_name = "LE"; break;
- case greater_equal: cc_name = "GE"; break;
- case equal: cc_name = "EQ"; break;
- case not_equal: cc_name = "NE"; break;
- default: cc_name = "UnknownCondition"; break;
- }
-
- const char* strict_name = "";
- if (strict_ && (cc_ == equal || cc_ == not_equal)) {
- strict_name = "_STRICT";
- }
-
- const char* never_nan_nan_name = "";
- if (never_nan_nan_ && (cc_ == equal || cc_ == not_equal)) {
- never_nan_nan_name = "_NO_NAN";
+ case less: return "CompareStub_LT";
+ case greater: return "CompareStub_GT";
+ case less_equal: return "CompareStub_LE";
+ case greater_equal: return "CompareStub_GE";
+ case not_equal: {
+ if (strict_) {
+ if (never_nan_nan_) {
+ return "CompareStub_NE_STRICT_NO_NAN";
+ } else {
+ return "CompareStub_NE_STRICT";
+ }
+ } else {
+ if (never_nan_nan_) {
+ return "CompareStub_NE_NO_NAN";
+ } else {
+ return "CompareStub_NE";
+ }
+ }
+ }
+ case equal: {
+ if (strict_) {
+ if (never_nan_nan_) {
+ return "CompareStub_EQ_STRICT_NO_NAN";
+ } else {
+ return "CompareStub_EQ_STRICT";
+ }
+ } else {
+ if (never_nan_nan_) {
+ return "CompareStub_EQ_NO_NAN";
+ } else {
+ return "CompareStub_EQ";
+ }
+ }
+ }
+ default: return "CompareStub";
}
+}
- const char* include_number_compare_name = "";
- if (!include_number_compare_) {
- include_number_compare_name = "_NO_NUMBER";
- }
- OS::SNPrintF(Vector<char>(name_, kMaxNameLength),
- "CompareStub_%s%s%s%s",
- cc_name,
- strict_name,
- never_nan_nan_name,
- include_number_compare_name);
- return name_;
+int CompareStub::MinorKey() {
+ // Encode the three parameters in a unique 16 bit value. To avoid duplicate
+ // stubs the never NaN NaN condition is only taken into account if the
+ // condition is equals.
+ ASSERT(static_cast<unsigned>(cc_) < (1 << 14));
+ return ConditionField::encode(static_cast<unsigned>(cc_))
+ | StrictField::encode(strict_)
+ | NeverNanNanField::encode(cc_ == equal ? never_nan_nan_ : false);
}
@@ -12437,9 +12227,6 @@ void StringCompareStub::GenerateCompareFlatAsciiStrings(MacroAssembler* masm,
Label result_not_equal;
Label result_greater;
Label compare_lengths;
-
- __ IncrementCounter(&Counters::string_compare_native, 1);
-
// Find minimum length.
Label left_shorter;
__ mov(scratch1, FieldOperand(left, String::kLengthOffset));
@@ -12537,6 +12324,7 @@ void StringCompareStub::Generate(MacroAssembler* masm) {
__ JumpIfNotBothSequentialAsciiStrings(edx, eax, ecx, ebx, &runtime);
// Compare flat ascii strings.
+ __ IncrementCounter(&Counters::string_compare_native, 1);
GenerateCompareFlatAsciiStrings(masm, edx, eax, ecx, ebx, edi);
// Call the runtime; it returns -1 (less), 0 (equal), or 1 (greater)
« no previous file with comments | « src/ia32/codegen-ia32.h ('k') | src/jump-target.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698