From 07742f5672187e0b023bf3aa0eefa1846e527591 Mon Sep 17 00:00:00 2001 From: "bak@chromium.org" Date: Fri, 18 Dec 2009 06:38:12 +0000 Subject: [PATCH] -Inlined double variant of compare iff one of the sides is a constant smi and it is not a for loop condition. Review URL: http://codereview.chromium.org/507040 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3487 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ast.h | 11 ++++++++- src/compiler.cc | 6 ++--- src/ia32/assembler-ia32.cc | 11 +++++++++ src/ia32/assembler-ia32.h | 1 + src/ia32/codegen-ia32.cc | 59 ++++++++++++++++++++++++++++++++++++++++------ src/ia32/codegen-ia32.h | 7 +++--- src/ia32/disasm-ia32.cc | 8 +++++++ src/parser.cc | 3 +++ 8 files changed, 92 insertions(+), 14 deletions(-) diff --git a/src/ast.h b/src/ast.h index 52b1b06..195fc14 100644 --- a/src/ast.h +++ b/src/ast.h @@ -139,6 +139,7 @@ class AstNode: public ZoneObject { virtual MaterializedLiteral* AsMaterializedLiteral() { return NULL; } virtual ObjectLiteral* AsObjectLiteral() { return NULL; } virtual ArrayLiteral* AsArrayLiteral() { return NULL; } + virtual CompareOperation* AsCompareOperation() { return NULL; } }; @@ -1185,7 +1186,7 @@ class CountOperation: public Expression { class CompareOperation: public Expression { public: CompareOperation(Token::Value op, Expression* left, Expression* right) - : op_(op), left_(left), right_(right) { + : op_(op), left_(left), right_(right), is_for_loop_condition_(false) { ASSERT(Token::IsCompareOp(op)); } @@ -1195,10 +1196,18 @@ class CompareOperation: public Expression { Expression* left() const { return left_; } Expression* right() const { return right_; } + // Accessors for flag whether this compare operation is hanging of a for loop. + bool is_for_loop_condition() const { return is_for_loop_condition_; } + void set_is_for_loop_condition() { is_for_loop_condition_ = true; } + + // Type testing & conversion + virtual CompareOperation* AsCompareOperation() { return this; } + private: Token::Value op_; Expression* left_; Expression* right_; + bool is_for_loop_condition_; }; diff --git a/src/compiler.cc b/src/compiler.cc index 2c055a3..03771d9 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -1101,9 +1101,9 @@ void CodeGenSelector::VisitBinaryOperation(BinaryOperation* expr) { void CodeGenSelector::VisitCompareOperation(CompareOperation* expr) { - ProcessExpression(expr->left(), Expression::kValue); - CHECK_BAILOUT; - ProcessExpression(expr->right(), Expression::kValue); + ProcessExpression(expr->left(), Expression::kValue); + CHECK_BAILOUT; + ProcessExpression(expr->right(), Expression::kValue); } diff --git a/src/ia32/assembler-ia32.cc b/src/ia32/assembler-ia32.cc index d6f5550..a0236d0 100644 --- a/src/ia32/assembler-ia32.cc +++ b/src/ia32/assembler-ia32.cc @@ -2004,6 +2004,17 @@ void Assembler::divsd(XMMRegister dst, XMMRegister src) { } +void Assembler::xorpd(XMMRegister dst, XMMRegister src) { + ASSERT(CpuFeatures::IsEnabled(SSE2)); + EnsureSpace ensure_space(this); + last_pc_ = pc_; + EMIT(0x66); + EMIT(0x0F); + EMIT(0x57); + emit_sse_operand(dst, src); +} + + void Assembler::comisd(XMMRegister dst, XMMRegister src) { ASSERT(CpuFeatures::IsEnabled(SSE2)); EnsureSpace ensure_space(this); diff --git a/src/ia32/assembler-ia32.h b/src/ia32/assembler-ia32.h index 662ebc9..21fa0d5 100644 --- a/src/ia32/assembler-ia32.h +++ b/src/ia32/assembler-ia32.h @@ -745,6 +745,7 @@ class Assembler : public Malloced { void subsd(XMMRegister dst, XMMRegister src); void mulsd(XMMRegister dst, XMMRegister src); void divsd(XMMRegister dst, XMMRegister src); + void xorpd(XMMRegister dst, XMMRegister src); void comisd(XMMRegister dst, XMMRegister src); diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 9738944..72979c6 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -1865,7 +1865,8 @@ void CodeGenerator::ConstantSmiBinaryOperation(Token::Value op, } -void CodeGenerator::Comparison(Condition cc, +void CodeGenerator::Comparison(AstNode* node, + Condition cc, bool strict, ControlDestination* dest) { // Strict only makes sense for equality comparisons. @@ -1912,7 +1913,8 @@ void CodeGenerator::Comparison(Condition cc, default: UNREACHABLE(); } - } else { // Only one side is a constant Smi. + } else { + // Only one side is a constant Smi. // If left side is a constant Smi, reverse the operands. // Since one side is a constant Smi, conversion order does not matter. if (left_side_constant_smi) { @@ -1926,6 +1928,8 @@ void CodeGenerator::Comparison(Condition cc, // Implement comparison against a constant Smi, inlining the case // where both sides are Smis. left_side.ToRegister(); + Register left_reg = left_side.reg(); + Handle right_val = right_side.handle(); // Here we split control flow to the stub call and inlined cases // before finally splitting it to the control destination. We use @@ -1933,11 +1937,50 @@ void CodeGenerator::Comparison(Condition cc, // the first split. We manually handle the off-frame references // by reconstituting them on the non-fall-through path. JumpTarget is_smi; - Register left_reg = left_side.reg(); - Handle right_val = right_side.handle(); __ test(left_side.reg(), Immediate(kSmiTagMask)); is_smi.Branch(zero, taken); + bool is_for_loop_compare = (node->AsCompareOperation() != NULL) + && node->AsCompareOperation()->is_for_loop_condition(); + if (!is_for_loop_compare + && CpuFeatures::IsSupported(SSE2) + && right_val->IsSmi()) { + // Right side is a constant smi and left side has been checked + // not to be a smi. + CpuFeatures::Scope use_sse2(SSE2); + JumpTarget not_number; + __ cmp(FieldOperand(left_reg, HeapObject::kMapOffset), + Immediate(Factory::heap_number_map())); + not_number.Branch(not_equal, &left_side); + __ movdbl(xmm1, + FieldOperand(left_reg, HeapNumber::kValueOffset)); + int value = Smi::cast(*right_val)->value(); + if (value == 0) { + __ xorpd(xmm0, xmm0); + } else { + Result temp = allocator()->Allocate(); + __ mov(temp.reg(), Immediate(value)); + __ cvtsi2sd(xmm0, Operand(temp.reg())); + temp.Unuse(); + } + __ comisd(xmm1, xmm0); + // 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->false_target()->Jump(); + not_number.Bind(&left_side); + } + // Setup and call the compare stub. CompareStub stub(cc, strict); Result result = frame_->CallStub(&stub, &left_side, &right_side); @@ -1961,6 +2004,7 @@ void CodeGenerator::Comparison(Condition cc, right_side.Unuse(); dest->Split(cc); } + } else if (cc == equal && (left_side_constant_null || right_side_constant_null)) { // To make null checks efficient, we check if either the left side or @@ -1997,7 +2041,8 @@ void CodeGenerator::Comparison(Condition cc, operand.Unuse(); dest->Split(not_zero); } - } else { // Neither side is a constant Smi or null. + } else { + // 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()) || @@ -2664,7 +2709,7 @@ void CodeGenerator::VisitSwitchStatement(SwitchStatement* node) { // Compare and branch to the body if true or the next test if // false. Prefer the next test as a fall through. ControlDestination dest(clause->body_target(), &next_test, false); - Comparison(equal, true, &dest); + Comparison(node, equal, true, &dest); // If the comparison fell through to the true target, jump to the // actual body. @@ -6077,7 +6122,7 @@ void CodeGenerator::VisitCompareOperation(CompareOperation* node) { } Load(left); Load(right); - Comparison(cc, strict, destination()); + Comparison(node, cc, strict, destination()); } diff --git a/src/ia32/codegen-ia32.h b/src/ia32/codegen-ia32.h index 684f1e7..3d17c96 100644 --- a/src/ia32/codegen-ia32.h +++ b/src/ia32/codegen-ia32.h @@ -459,7 +459,8 @@ class CodeGenerator: public AstVisitor { Result* right, OverwriteMode overwrite_mode); - void Comparison(Condition cc, + void Comparison(AstNode* node, + Condition cc, bool strict, ControlDestination* destination); @@ -727,8 +728,8 @@ class GenericBinaryOpStub: public CodeStub { bool ArgsInRegistersSupported() { return ((op_ == Token::ADD) || (op_ == Token::SUB) - || (op_ == Token::MUL) || (op_ == Token::DIV)) - && flags_ != NO_SMI_CODE_IN_STUB; + || (op_ == Token::MUL) || (op_ == Token::DIV)) + && flags_ != NO_SMI_CODE_IN_STUB; } bool IsOperationCommutative() { return (op_ == Token::ADD) || (op_ == Token::MUL); diff --git a/src/ia32/disasm-ia32.cc b/src/ia32/disasm-ia32.cc index df5a28a..661c313 100644 --- a/src/ia32/disasm-ia32.cc +++ b/src/ia32/disasm-ia32.cc @@ -1049,6 +1049,14 @@ int DisassemblerIA32::InstructionDecode(v8::internal::Vector out_buffer, NameOfXMMRegister(regop), NameOfXMMRegister(rm)); data++; + } if (*data == 0x57) { + data++; + int mod, regop, rm; + get_modrm(*data, &mod, ®op, &rm); + AppendToBuffer("xorpd %s,%s", + NameOfXMMRegister(regop), + NameOfXMMRegister(rm)); + data++; } else { UnimplementedInstruction(); } diff --git a/src/parser.cc b/src/parser.cc index c37078c..a3bc6da 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2657,6 +2657,9 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) { Expression* cond = NULL; if (peek() != Token::SEMICOLON) { cond = ParseExpression(true, CHECK_OK); + if (cond && cond->AsCompareOperation()) { + cond->AsCompareOperation()->set_is_for_loop_condition(); + } } Expect(Token::SEMICOLON, CHECK_OK); -- 2.7.4