From 6a639106505f10f9d3471aff6ca84f67c003f8f0 Mon Sep 17 00:00:00 2001 From: "sgjesse@chromium.org" Date: Thu, 25 Mar 2010 12:04:34 +0000 Subject: [PATCH] Re-apply "Inline floating point compare" This re-applies r4220 and r4233, which was reverted in r4254 due to a bug. This bug has now been fixed, with the only change being line 2884 changed from __ SmiTag(left_side->reg()); to __ SmiTag(operand->reg()); Added a regression test. BUG=http://crbug.com/39160 TEST=test/mjsunit/regress/regress-crbug-39160.js Review URL: http://codereview.chromium.org/1251009 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4261 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/codegen-arm.cc | 78 ++-- src/codegen.h | 24 +- src/ia32/codegen-ia32.cc | 430 +++++++++++++++----- src/ia32/codegen-ia32.h | 4 + src/jump-target.cc | 30 ++ src/jump-target.h | 5 + src/x64/codegen-x64.cc | 78 ++-- test/mjsunit/regress/regress-crbug-39160.js | 41 ++ 8 files changed, 501 insertions(+), 189 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-39160.js diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index 1d67918c5..5e0067716 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -7018,44 +7018,47 @@ void CallFunctionStub::Generate(MacroAssembler* masm) { } +// 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 lt: return "CompareStub_LT"; - case gt: return "CompareStub_GT"; - case le: return "CompareStub_LE"; - case ge: return "CompareStub_GE"; - case ne: { - 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 eq: { - 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"; + case lt: cc_name = "LT"; break; + case gt: cc_name = "GT"; break; + case le: cc_name = "LE"; break; + case ge: cc_name = "GE"; break; + case eq: cc_name = "EQ"; break; + case ne: cc_name = "NE"; break; + default: cc_name = "UnknownCondition"; break; + } + + const char* strict_name = ""; + if (strict_ && (cc_ == eq || cc_ == ne)) { + strict_name = "_STRICT"; } + + const char* never_nan_nan_name = ""; + if (never_nan_nan_ && (cc_ == eq || cc_ == ne)) { + never_nan_nan_name = "_NO_NAN"; + } + + const char* include_number_compare_name = ""; + if (!include_number_compare_) { + include_number_compare_name = "_NO_NUMBER"; + } + + OS::SNPrintF(Vector(name_, kMaxNameLength), + "CompareStub_%s%s%s%s", + cc_name, + strict_name, + never_nan_nan_name, + include_number_compare_name); + return name_; } @@ -7063,10 +7066,11 @@ 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(cc_) >> 28) < (1 << 14)); + ASSERT((static_cast(cc_) >> 28) < (1 << 13)); return ConditionField::encode(static_cast(cc_) >> 28) | StrictField::encode(strict_) - | NeverNanNanField::encode(cc_ == eq ? never_nan_nan_ : false); + | NeverNanNanField::encode(cc_ == eq ? never_nan_nan_ : false) + | IncludeNumberCompareField::encode(include_number_compare_); } diff --git a/src/codegen.h b/src/codegen.h index d8a339d4e..c459280c2 100644 --- a/src/codegen.h +++ b/src/codegen.h @@ -346,8 +346,13 @@ class CompareStub: public CodeStub { public: CompareStub(Condition cc, bool strict, - NaNInformation nan_info = kBothCouldBeNaN) : - cc_(cc), strict_(strict), never_nan_nan_(nan_info == kCantBothBeNaN) { } + NaNInformation nan_info = kBothCouldBeNaN, + bool include_number_compare = true) : + cc_(cc), + strict_(strict), + never_nan_nan_(nan_info == kCantBothBeNaN), + include_number_compare_(include_number_compare), + name_(NULL) { } void Generate(MacroAssembler* masm); @@ -360,11 +365,16 @@ class CompareStub: public CodeStub { // generating the minor key for other comparisons to avoid creating more // stubs. bool never_nan_nan_; + // Do generate the number comparison code in the stub. Stubs without number + // comparison code is used when the number comparison has been inlined, and + // the stub will be called if one of the operands is not a number. + bool include_number_compare_; // Encoding of the minor key CCCCCCCCCCCCCCNS. class StrictField: public BitField {}; class NeverNanNanField: public BitField {}; - class ConditionField: public BitField {}; + class IncludeNumberCompareField: public BitField {}; + class ConditionField: public BitField {}; Major MajorKey() { return Compare; } @@ -378,12 +388,16 @@ class CompareStub: public CodeStub { // 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. + char* name_; const char* GetName(); #ifdef DEBUG void Print() { - PrintF("CompareStub (cc %d), (strict %s)\n", + PrintF("CompareStub (cc %d), (strict %s), " + "(never_nan_nan %s), (number_compare %s)\n", static_cast(cc_), - strict_ ? "true" : "false"); + strict_ ? "true" : "false", + never_nan_nan_ ? "true" : "false", + include_number_compare_ ? "included" : "not included"); } #endif }; diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 683b18a4e..f8ae7e41a 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -911,6 +911,7 @@ 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. @@ -929,6 +930,7 @@ 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, @@ -947,6 +949,7 @@ 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. @@ -2361,6 +2364,22 @@ 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, @@ -2431,7 +2450,7 @@ void CodeGenerator::Comparison(AstNode* node, left_side = right_side; right_side = temp; cc = ReverseCondition(cc); - // This may reintroduce greater or less_equal as the value of cc. + // This may re-introduce 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 @@ -2480,16 +2499,7 @@ void CodeGenerator::Comparison(AstNode* node, // Jump to builtin for NaN. not_number.Branch(parity_even, &left_side); left_side.Unuse(); - 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->true_target()->Branch(DoubleCondition(cc)); dest->false_target()->Jump(); not_number.Bind(&left_side); } @@ -2688,21 +2698,53 @@ void CodeGenerator::Comparison(AstNode* node, dest->Split(cc); } } else { - // Neither side is a constant Smi or null. - // If either side is a non-smi constant, skip the smi check. + // 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. bool known_non_smi = (left_side.is_constant() && !left_side.handle()->IsSmi()) || - (right_side.is_constant() && !right_side.handle()->IsSmi()); + (right_side.is_constant() && !right_side.handle()->IsSmi()) || + left_side.number_info().IsDouble() || + right_side.number_info().IsDouble(); 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) { - // When non-smi, call out to the compare stub. - CompareStub stub(cc, strict, nan_info); + // 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); Result answer = frame_->CallStub(&stub, &left_side, &right_side); if (cc == equal) { __ test(answer.reg(), Operand(answer.reg())); @@ -2721,6 +2763,7 @@ 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()); @@ -2728,8 +2771,22 @@ void CodeGenerator::Comparison(AstNode* node, __ test(temp.reg(), Immediate(kSmiTagMask)); temp.Unuse(); is_smi.Branch(zero, taken); - // When non-smi, call out to the compare stub. - CompareStub stub(cc, strict, nan_info); + + // 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); Result answer = frame_->CallStub(&stub, &left_side, &right_side); if (cc == equal) { __ test(answer.reg(), Operand(answer.reg())); @@ -2752,6 +2809,148 @@ 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) { + Label done; + if (operand->number_info().IsDouble()) { + // 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().IsDouble()) { + // 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(operand->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, + ¬_numbers); + LoadComparisonOperandSSE2(masm_, right_side, xmm1, left_side, right_side, + ¬_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, + ¬_numbers); + CheckComparisonOperand(masm_, right_side, left_side, right_side, + ¬_numbers); + + // Load right and left operand to FPU stack and compare. + LoadComparisonOperand(masm_, right_side); + LoadComparisonOperand(masm_, left_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* args, @@ -10877,63 +11076,70 @@ void CompareStub::Generate(MacroAssembler* masm) { __ push(edx); __ push(ecx); - // 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); - - FloatingPointHelper::LoadSSE2Operands(masm, &check_for_symbols); - __ comisd(xmm0, xmm1); + // 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(); - // 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(); + // Don't base result on EFLAGS when a NaN is involved. + __ j(parity_even, &unordered, not_taken); - // Jump to builtin for NaN. - __ j(parity_even, &unordered, not_taken); + 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); - 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); + __ xor_(eax, Operand(eax)); + __ ret(2 * kPointerSize); - __ xor_(eax, Operand(eax)); // equal - // Both arguments were pushed in case a runtime call was needed. - __ ret(2 * kPointerSize); + __ bind(&below_label); + __ mov(eax, Immediate(Smi::FromInt(-1))); + __ ret(2 * kPointerSize); - __ bind(&below_lbl); - __ mov(eax, Immediate(Smi::FromInt(-1))); - __ ret(2 * kPointerSize); + __ bind(&above_label); + __ mov(eax, Immediate(Smi::FromInt(1))); + __ ret(2 * kPointerSize); + } - __ bind(&above_lbl); - __ mov(eax, Immediate(Smi::FromInt(1))); + // 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 + + // The number comparison code did not provide a valid result. + __ bind(&non_number_comparison); } - // 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); @@ -11543,57 +11749,59 @@ 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(cc_) < (1 << 13)); + return ConditionField::encode(static_cast(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: 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"; + 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"; + } -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(cc_) < (1 << 14)); - return ConditionField::encode(static_cast(cc_)) - | StrictField::encode(strict_) - | NeverNanNanField::encode(cc_ == equal ? never_nan_nan_ : false); + const char* never_nan_nan_name = ""; + if (never_nan_nan_ && (cc_ == equal || cc_ == not_equal)) { + never_nan_nan_name = "_NO_NAN"; + } + + const char* include_number_compare_name = ""; + if (!include_number_compare_) { + include_number_compare_name = "_NO_NUMBER"; + } + + OS::SNPrintF(Vector(name_, kMaxNameLength), + "CompareStub_%s%s%s%s", + cc_name, + strict_name, + never_nan_nan_name, + include_number_compare_name); + return name_; } @@ -12227,6 +12435,9 @@ 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)); @@ -12324,7 +12535,6 @@ 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) diff --git a/src/ia32/codegen-ia32.h b/src/ia32/codegen-ia32.h index 509bbd6f6..e661a41c0 100644 --- a/src/ia32/codegen-ia32.h +++ b/src/ia32/codegen-ia32.h @@ -528,6 +528,10 @@ class CodeGenerator: public AstVisitor { Condition cc, bool strict, ControlDestination* destination); + void GenerateInlineNumberComparison(Result* left_side, + Result* right_side, + Condition cc, + ControlDestination* dest); // To prevent long attacker-controlled byte sequences, integer constants // from the JavaScript source are loaded in two parts if they are larger diff --git a/src/jump-target.cc b/src/jump-target.cc index 7b1ced7eb..8e949fba5 100644 --- a/src/jump-target.cc +++ b/src/jump-target.cc @@ -290,6 +290,25 @@ void JumpTarget::Branch(Condition cc, Result* arg, Hint hint) { } +void JumpTarget::Branch(Condition cc, Result* arg0, Result* arg1, Hint hint) { + ASSERT(cgen()->has_valid_frame()); + + // We want to check that non-frame registers at the call site stay in + // the same registers on the fall-through branch. + DECLARE_ARGCHECK_VARS(arg0); + DECLARE_ARGCHECK_VARS(arg1); + + cgen()->frame()->Push(arg0); + cgen()->frame()->Push(arg1); + DoBranch(cc, hint); + *arg1 = cgen()->frame()->Pop(); + *arg0 = cgen()->frame()->Pop(); + + ASSERT_ARGCHECK(arg0); + ASSERT_ARGCHECK(arg1); +} + + void BreakTarget::Branch(Condition cc, Result* arg, Hint hint) { ASSERT(cgen()->has_valid_frame()); @@ -331,6 +350,17 @@ void JumpTarget::Bind(Result* arg) { } +void JumpTarget::Bind(Result* arg0, Result* arg1) { + if (cgen()->has_valid_frame()) { + cgen()->frame()->Push(arg0); + cgen()->frame()->Push(arg1); + } + DoBind(); + *arg1 = cgen()->frame()->Pop(); + *arg0 = cgen()->frame()->Pop(); +} + + void JumpTarget::AddReachingFrame(VirtualFrame* frame) { ASSERT(reaching_frames_.length() == merge_labels_.length()); ASSERT(entry_frame_ == NULL); diff --git a/src/jump-target.h b/src/jump-target.h index db7c11553..db523b55b 100644 --- a/src/jump-target.h +++ b/src/jump-target.h @@ -117,12 +117,17 @@ class JumpTarget : public ZoneObject { // Shadows are dynamically allocated. // the target and the fall-through. virtual void Branch(Condition cc, Hint hint = no_hint); virtual void Branch(Condition cc, Result* arg, Hint hint = no_hint); + virtual void Branch(Condition cc, + Result* arg0, + Result* arg1, + Hint hint = no_hint); // Bind a jump target. If there is no current frame at the binding // site, there must be at least one frame reaching via a forward // jump. virtual void Bind(); virtual void Bind(Result* arg); + virtual void Bind(Result* arg0, Result* arg1); // Emit a call to a jump target. There must be a current frame at // the call. The frame at the target is the same as the current diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 2d875e131..2f873c54b 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -9105,51 +9105,55 @@ 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(cc_) < (1 << 14)); + ASSERT(static_cast(cc_) < (1 << 13)); return ConditionField::encode(static_cast(cc_)) | StrictField::encode(strict_) - | NeverNanNanField::encode(cc_ == equal ? never_nan_nan_ : false); + | 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: 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"; + 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"; + } + + const char* include_number_compare_name = ""; + if (!include_number_compare_) { + include_number_compare_name = "_NO_NUMBER"; + } + + OS::SNPrintF(Vector(name_, kMaxNameLength), + "CompareStub_%s%s%s%s", + cc_name, + strict_name, + never_nan_nan_name, + include_number_compare_name); + return name_; } diff --git a/test/mjsunit/regress/regress-crbug-39160.js b/test/mjsunit/regress/regress-crbug-39160.js new file mode 100644 index 000000000..a8a856790 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-39160.js @@ -0,0 +1,41 @@ +// Copyright 2010 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// See http://crbug.com/39160 + +// To reproduce the bug we need an inlined comparison (i.e. in a loop) where +// the left hand side is known to be a smi (max smi value is 1073741823). This +// test crashes with the bug. +function f(a) { + for (var i = 1073741820; i < 1073741822; i++) { + if (a < i) { + a += i; + } + } +} + +f(5) -- 2.34.1