Revision: 16029
Author: [email protected]
Date: Fri Aug 2 04:56:35 2013
Log: Replaced unary negation by multiplication with -1.
This fixes a deopt loop in the Epic Citadel demo and removes some code.
Apart from that, this change is performance-neutral.
When we do something similar for BIT_NOT, the whole UnaryOp stuff can go
away.
[email protected]
Review URL: https://codereview.chromium.org/21782002
http://code.google.com/p/v8/source/detail?r=16029
Added:
/branches/bleeding_edge/test/mjsunit/unary-minus-deopt.js
Modified:
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/code-stubs-hydrogen.cc
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/mips/full-codegen-mips.cc
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/typing.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/unary-minus-deopt.js Fri Aug 2
04:56:35 2013
@@ -0,0 +1,55 @@
+// Copyright 2013 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.
+
+// Flags: --allow-natives-syntax
+
+// This is a boiled-down example happening in the Epic Citadel demo:
+// After deopting, the multiplication for unary minus stayed in Smi
+// mode instead of going to double mode, leading to deopt loops.
+
+function unaryMinusTest(x) {
+ var g = (1 << x) | 0;
+ // Optimized code will contain a LMulI with -1 as right operand.
+ return (g & -g) - 1 | 0;
+}
+
+unaryMinusTest(3);
+unaryMinusTest(3);
+%OptimizeFunctionOnNextCall(unaryMinusTest);
+unaryMinusTest(3);
+assertOptimized(unaryMinusTest);
+
+// Deopt on kMinInt
+unaryMinusTest(31);
+// The following is normally true, but not with --stress-opt. :-/
+// assertUnoptimized(unaryMinusTest);
+
+// We should have learned something from the deopt.
+unaryMinusTest(31);
+%OptimizeFunctionOnNextCall(unaryMinusTest);
+unaryMinusTest(31);
+assertOptimized(unaryMinusTest);
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Fri Aug 2 02:53:11
2013
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Fri Aug 2 04:56:35
2013
@@ -4348,10 +4348,6 @@
context()->Plug(r0);
break;
}
-
- case Token::SUB:
- EmitUnaryOperation(expr, "[ UnaryOperation (SUB)");
- break;
case Token::BIT_NOT:
EmitUnaryOperation(expr, "[ UnaryOperation (BIT_NOT)");
@@ -4365,6 +4361,7 @@
void FullCodeGenerator::EmitUnaryOperation(UnaryOperation* expr,
const char* comment) {
+ ASSERT_EQ(Token::BIT_NOT, expr->op());
// TODO(svenpanne): Allowing format strings in Comment would be nice
here...
Comment cmt(masm_, comment);
UnaryOpStub stub(expr->op());
=======================================
--- /branches/bleeding_edge/src/code-stubs-hydrogen.cc Fri Aug 2 02:53:11
2013
+++ /branches/bleeding_edge/src/code-stubs-hydrogen.cc Fri Aug 2 04:56:35
2013
@@ -804,6 +804,7 @@
template <>
HValue* CodeStubGraphBuilder<UnaryOpStub>::BuildCodeInitializedStub() {
UnaryOpStub* stub = casted_stub();
+ ASSERT_EQ(Token::BIT_NOT, stub->operation());
Handle<Type> type = stub->GetType(graph()->isolate());
HValue* input = GetParameter(0);
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Fri Aug 2 04:24:55 2013
+++ /branches/bleeding_edge/src/hydrogen.cc Fri Aug 2 04:56:35 2013
@@ -1718,33 +1718,14 @@
HInstruction* HGraphBuilder::BuildUnaryMathOp(
HValue* input, Handle<Type> type, Token::Value operation) {
+ ASSERT_EQ(Token::BIT_NOT, operation);
// We only handle the numeric cases here
type = handle(
Type::Intersect(type, handle(Type::Number(), isolate())), isolate());
-
- switch (operation) {
- default:
- UNREACHABLE();
- case Token::SUB: {
- HInstruction* instr =
- NewUncasted<HMul>(input, graph()->GetConstantMinus1());
- Representation rep = Representation::FromType(type);
- if (type->Is(Type::None())) {
- Add<HDeoptimize>(Deoptimizer::SOFT);
- }
- if (instr->IsBinaryOperation()) {
- HBinaryOperation* binop = HBinaryOperation::cast(instr);
- binop->set_observed_input_representation(1, rep);
- binop->set_observed_input_representation(2, rep);
- }
- return instr;
- }
- case Token::BIT_NOT:
- if (type->Is(Type::None())) {
- Add<HDeoptimize>(Deoptimizer::SOFT);
- }
- return New<HBitNot>(input);
+ if (type->Is(Type::None())) {
+ Add<HDeoptimize>(Deoptimizer::SOFT);
}
+ return New<HBitNot>(input);
}
@@ -7220,7 +7201,6 @@
case Token::DELETE: return VisitDelete(expr);
case Token::VOID: return VisitVoid(expr);
case Token::TYPEOF: return VisitTypeof(expr);
- case Token::SUB: return VisitSub(expr);
case Token::BIT_NOT: return VisitBitNot(expr);
case Token::NOT: return VisitNot(expr);
default: UNREACHABLE();
@@ -7281,15 +7261,6 @@
HInstruction* instr = new(zone()) HTypeof(context, value);
return ast_context()->ReturnInstruction(instr, expr->id());
}
-
-
-void HOptimizedGraphBuilder::VisitSub(UnaryOperation* expr) {
- CHECK_ALIVE(VisitForValue(expr->expression()));
- Handle<Type> operand_type = expr->expression()->bounds().lower;
- HValue* value = TruncateToNumber(Pop(), &operand_type);
- HInstruction* instr = BuildUnaryMathOp(value, operand_type, Token::SUB);
- return ast_context()->ReturnInstruction(instr, expr->id());
-}
void HOptimizedGraphBuilder::VisitBitNot(UnaryOperation* expr) {
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Aug 2
02:53:11 2013
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Aug 2
04:56:35 2013
@@ -4346,10 +4346,6 @@
context()->Plug(eax);
break;
}
-
- case Token::SUB:
- EmitUnaryOperation(expr, "[ UnaryOperation (SUB)");
- break;
case Token::BIT_NOT:
EmitUnaryOperation(expr, "[ UnaryOperation (BIT_NOT)");
@@ -4363,6 +4359,7 @@
void FullCodeGenerator::EmitUnaryOperation(UnaryOperation* expr,
const char* comment) {
+ ASSERT_EQ(Token::BIT_NOT, expr->op());
Comment cmt(masm_, comment);
UnaryOpStub stub(expr->op());
// UnaryOpStub expects the argument to be in the
=======================================
--- /branches/bleeding_edge/src/mips/full-codegen-mips.cc Fri Aug 2
02:53:11 2013
+++ /branches/bleeding_edge/src/mips/full-codegen-mips.cc Fri Aug 2
04:56:35 2013
@@ -4381,10 +4381,6 @@
context()->Plug(v0);
break;
}
-
- case Token::SUB:
- EmitUnaryOperation(expr, "[ UnaryOperation (SUB)");
- break;
case Token::BIT_NOT:
EmitUnaryOperation(expr, "[ UnaryOperation (BIT_NOT)");
@@ -4398,6 +4394,7 @@
void FullCodeGenerator::EmitUnaryOperation(UnaryOperation* expr,
const char* comment) {
+ ASSERT_EQ(Token::BIT_NOT, expr->op());
// TODO(svenpanne): Allowing format strings in Comment would be nice
here...
Comment cmt(masm_, comment);
UnaryOpStub stub(expr->op());
=======================================
--- /branches/bleeding_edge/src/parser.cc Fri Jul 19 02:57:35 2013
+++ /branches/bleeding_edge/src/parser.cc Fri Aug 2 04:56:35 2013
@@ -3197,6 +3197,13 @@
factory()->NewNumberLiteral(1),
position);
}
+ // The same idea for '-foo' => 'foo*(-1)'.
+ if (op == Token::SUB) {
+ return factory()->NewBinaryOperation(Token::MUL,
+ expression,
+ factory()->NewNumberLiteral(-1),
+ position);
+ }
return factory()->NewUnaryOperation(op, expression, position);
=======================================
--- /branches/bleeding_edge/src/typing.cc Fri Jul 19 12:55:09 2013
+++ /branches/bleeding_edge/src/typing.cc Fri Aug 2 04:56:35 2013
@@ -419,13 +419,6 @@
case Token::VOID:
NarrowType(expr, Bounds(Type::Undefined(), isolate_));
break;
- case Token::ADD:
- case Token::SUB: {
- Type* upper = *expr->expression()->bounds().upper;
- if (!upper->Is(Type::Number())) upper = Type::Number();
- NarrowType(expr, Bounds(Type::Smi(), upper, isolate_));
- break;
- }
case Token::BIT_NOT:
NarrowType(expr, Bounds(Type::Smi(), Type::Signed32(), isolate_));
break;
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Fri Aug 2 02:53:11
2013
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Fri Aug 2 04:56:35
2013
@@ -4334,10 +4334,6 @@
context()->Plug(rax);
break;
}
-
- case Token::SUB:
- EmitUnaryOperation(expr, "[ UnaryOperation (SUB)");
- break;
case Token::BIT_NOT:
EmitUnaryOperation(expr, "[ UnaryOperation (BIT_NOT)");
@@ -4351,6 +4347,7 @@
void FullCodeGenerator::EmitUnaryOperation(UnaryOperation* expr,
const char* comment) {
+ ASSERT_EQ(Token::BIT_NOT, expr->op());
// TODO(svenpanne): Allowing format strings in Comment would be nice
here...
Comment cmt(masm_, comment);
UnaryOpStub stub(expr->op());
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
