From 0d8b7f76084c30b91acce1e4df118bff065c81c1 Mon Sep 17 00:00:00 2001 From: "svenpanne@chromium.org" Date: Wed, 17 Apr 2013 12:47:15 +0000 Subject: [PATCH] Improve handling of unary plus. Simple strategy: Transform unary plus into multiplication by one directly in the parser and remove it from the Hydrogen graph later. This gives correct type feedback without any special stub for it. BUG=v8:2527 Review URL: https://codereview.chromium.org/13902013 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14306 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 12 ------------ src/hydrogen-instructions.cc | 15 +++++++++++++++ src/hydrogen-instructions.h | 2 ++ src/hydrogen.cc | 16 ---------------- src/hydrogen.h | 1 - src/ia32/full-codegen-ia32.cc | 12 ------------ src/mips/full-codegen-mips.cc | 13 ------------- src/parser.cc | 10 ++++++++++ src/x64/full-codegen-x64.cc | 12 ------------ 9 files changed, 27 insertions(+), 66 deletions(-) diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index ba0f141289..6a33234031 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -3976,18 +3976,6 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { break; } - case Token::ADD: { - Comment cmt(masm_, "[ UnaryOperation (ADD)"); - VisitForAccumulatorValue(expr->expression()); - Label no_conversion; - __ JumpIfSmi(result_register(), &no_conversion); - ToNumberStub convert_stub; - __ CallStub(&convert_stub); - __ bind(&no_conversion); - context()->Plug(result_register()); - break; - } - case Token::SUB: EmitUnaryOperation(expr, "[ UnaryOperation (SUB)"); break; diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 60a6912654..0aabfbc432 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -1436,6 +1436,21 @@ HValue* HSub::Canonicalize() { } +// TODO(svenpanne) Use this in other Canonicalize() functions. +static bool IsIdentityOperation(HValue* arg1, HValue* arg2, int32_t identity) { + return arg1->representation().IsSpecialization() && + arg2->IsInteger32Constant() && + arg2->GetInteger32Constant() == identity; +} + + +HValue* HMul::Canonicalize() { + if (IsIdentityOperation(left(), right(), 1)) return left(); + if (IsIdentityOperation(right(), left(), 1)) return right(); + return this; +} + + HValue* HChange::Canonicalize() { return (from().Equals(to())) ? value() : this; } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 6853dfe100..e74de4a1b1 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -4401,6 +4401,8 @@ class HMul: public HArithmeticBinaryOperation { virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited); + virtual HValue* Canonicalize(); + // Only commutative if it is certain that not two objects are multiplicated. virtual bool IsCommutative() const { return !representation().IsTagged(); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 127d7a9aa1..8bfc52efb6 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -9160,7 +9160,6 @@ void HOptimizedGraphBuilder::VisitUnaryOperation(UnaryOperation* expr) { case Token::DELETE: return VisitDelete(expr); case Token::VOID: return VisitVoid(expr); case Token::TYPEOF: return VisitTypeof(expr); - case Token::ADD: return VisitAdd(expr); case Token::SUB: return VisitSub(expr); case Token::BIT_NOT: return VisitBitNot(expr); case Token::NOT: return VisitNot(expr); @@ -9218,21 +9217,6 @@ void HOptimizedGraphBuilder::VisitTypeof(UnaryOperation* expr) { } -void HOptimizedGraphBuilder::VisitAdd(UnaryOperation* expr) { - CHECK_ALIVE(VisitForValue(expr->expression())); - HValue* value = Pop(); - HValue* context = environment()->LookupContext(); - HInstruction* instr = - HMul::New(zone(), context, value, graph()->GetConstant1()); - if (instr->IsBinaryOperation()) { - // Since we don't have type feedback, we must be cautious/pessimistic. - HBinaryOperation::cast(instr)->set_observed_input_representation( - Representation::Tagged(), Representation::Tagged()); - } - return ast_context()->ReturnInstruction(instr, expr->id()); -} - - void HOptimizedGraphBuilder::VisitSub(UnaryOperation* expr) { CHECK_ALIVE(VisitForValue(expr->expression())); HValue* value = Pop(); diff --git a/src/hydrogen.h b/src/hydrogen.h index 3dbca3c3ea..a849e416f4 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -1267,7 +1267,6 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor { void VisitDelete(UnaryOperation* expr); void VisitVoid(UnaryOperation* expr); void VisitTypeof(UnaryOperation* expr); - void VisitAdd(UnaryOperation* expr); void VisitSub(UnaryOperation* expr); void VisitBitNot(UnaryOperation* expr); void VisitNot(UnaryOperation* expr); diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 19989b1c62..a35f12d8cc 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -3967,18 +3967,6 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { break; } - case Token::ADD: { - Comment cmt(masm_, "[ UnaryOperation (ADD)"); - VisitForAccumulatorValue(expr->expression()); - Label no_conversion; - __ JumpIfSmi(result_register(), &no_conversion); - ToNumberStub convert_stub; - __ CallStub(&convert_stub); - __ bind(&no_conversion); - context()->Plug(result_register()); - break; - } - case Token::SUB: EmitUnaryOperation(expr, "[ UnaryOperation (SUB)"); break; diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 8e2d5abbed..bc0d85543b 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -3995,19 +3995,6 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { break; } - case Token::ADD: { - Comment cmt(masm_, "[ UnaryOperation (ADD)"); - VisitForAccumulatorValue(expr->expression()); - Label no_conversion; - __ JumpIfSmi(result_register(), &no_conversion); - __ mov(a0, result_register()); - ToNumberStub convert_stub; - __ CallStub(&convert_stub); - __ bind(&no_conversion); - context()->Plug(result_register()); - break; - } - case Token::SUB: EmitUnaryOperation(expr, "[ UnaryOperation (SUB)"); break; diff --git a/src/parser.cc b/src/parser.cc index b4ab623829..06dcced8ff 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3286,6 +3286,16 @@ Expression* Parser::ParseUnaryExpression(bool* ok) { } } + // Desugar '+foo' into 'foo*1', this enables the collection of type feedback + // without any special stub and the multiplication is removed later in + // Crankshaft's canonicalization pass. + if (op == Token::ADD) { + return factory()->NewBinaryOperation(Token::MUL, + expression, + factory()->NewNumberLiteral(1), + position); + } + return factory()->NewUnaryOperation(op, expression, position); } else if (Token::IsCountOp(op)) { diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 564c3def86..8ca173b30f 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -3968,18 +3968,6 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { break; } - case Token::ADD: { - Comment cmt(masm_, "[ UnaryOperation (ADD)"); - VisitForAccumulatorValue(expr->expression()); - Label no_conversion; - __ JumpIfSmi(result_register(), &no_conversion); - ToNumberStub convert_stub; - __ CallStub(&convert_stub); - __ bind(&no_conversion); - context()->Plug(result_register()); - break; - } - case Token::SUB: EmitUnaryOperation(expr, "[ UnaryOperation (SUB)"); break;