Revision: 4130
Author: [email protected]
Date: Mon Mar 15 07:03:36 2010
Log: Add static analysis to AST expressions that records whether a negative
zero will be treated identically to a positive zero in the expression's
context. Use this flag to avoid some tests in inlined smi code.
Review URL: http://codereview.chromium.org/965001
http://code.google.com/p/v8/source/detail?r=4130
Modified:
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/ia32/codegen-ia32.cc
/branches/bleeding_edge/src/ia32/codegen-ia32.h
/branches/bleeding_edge/src/rewriter.cc
=======================================
--- /branches/bleeding_edge/src/ast.h Fri Mar 12 05:10:42 2010
+++ /branches/bleeding_edge/src/ast.h Mon Mar 15 07:03:36 2010
@@ -250,6 +250,14 @@
bitfields_ &= ~SideEffectFreeField::mask();
bitfields_ |= SideEffectFreeField::encode(is_side_effect_free);
}
+
+ // Will the use of this expression treat -0 the same as 0 in all cases?
+ // If so, we can return 0 instead of -0 if we want to, to optimize code.
+ bool no_negative_zero() { return
NoNegativeZeroField::decode(bitfields_); }
+ void set_no_negative_zero(bool no_negative_zero) {
+ bitfields_ &= ~NoNegativeZeroField::mask();
+ bitfields_ |= NoNegativeZeroField::encode(no_negative_zero);
+ }
// Will ToInt32 (ECMA 262-3 9.5) or ToUint32 (ECMA 262-3 9.6)
// be applied to the value of this expression?
@@ -270,7 +278,8 @@
// Using template BitField<type, start, size>.
class SideEffectFreeField : public BitField<bool, 0, 1> {};
- class ToInt32Field : public BitField<bool, 1, 1> {};
+ class NoNegativeZeroField : public BitField<bool, 1, 1> {};
+ class ToInt32Field : public BitField<bool, 2, 1> {};
};
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Thu Mar 11 08:24:31
2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Mon Mar 15 07:03:36
2010
@@ -1102,7 +1102,8 @@
void CodeGenerator::GenericBinaryOperation(Token::Value op,
StaticType* type,
- OverwriteMode overwrite_mode) {
+ OverwriteMode overwrite_mode,
+ bool no_negative_zero) {
Comment cmnt(masm_, "[ BinaryOperation");
Comment cmnt_token(masm_, Token::String(op));
@@ -1170,10 +1171,12 @@
answer = stub.GenerateCall(masm_, frame_, &left, &right);
} else if (right_is_smi_constant) {
answer = ConstantSmiBinaryOperation(op, &left, right.handle(),
- type, false, overwrite_mode);
+ type, false, overwrite_mode,
+ no_negative_zero);
} else if (left_is_smi_constant) {
answer = ConstantSmiBinaryOperation(op, &right, left.handle(),
- type, true, overwrite_mode);
+ type, true, overwrite_mode,
+ no_negative_zero);
} else {
// Set the flags based on the operation, type and loop nesting level.
// Bit operations always assume they likely operate on Smis. Still only
@@ -1184,7 +1187,8 @@
(Token::IsBitOp(op) ||
operands_type.IsInteger32() ||
type->IsLikelySmi())) {
- answer = LikelySmiBinaryOperation(op, &left, &right, overwrite_mode);
+ answer = LikelySmiBinaryOperation(op, &left, &right,
+ overwrite_mode, no_negative_zero);
} else {
GenericBinaryOpStub stub(op,
overwrite_mode,
@@ -1291,7 +1295,8 @@
Result CodeGenerator::LikelySmiBinaryOperation(Token::Value op,
Result* left,
Result* right,
- OverwriteMode
overwrite_mode) {
+ OverwriteMode
overwrite_mode,
+ bool no_negative_zero) {
Result answer;
// Special handling of div and mod because they use fixed registers.
if (op == Token::DIV || op == Token::MOD) {
@@ -1395,13 +1400,16 @@
// Check for negative zero result. If result is zero, and divisor
// is negative, return a floating point negative zero. The
// virtual frame is unchanged in this block, so local control flow
- // can use a Label rather than a JumpTarget.
- Label non_zero_result;
- __ test(left->reg(), Operand(left->reg()));
- __ j(not_zero, &non_zero_result);
- __ test(right->reg(), Operand(right->reg()));
- deferred->Branch(negative);
- __ bind(&non_zero_result);
+ // can use a Label rather than a JumpTarget. If the context of this
+ // expression will treat -0 like 0, do not do this test.
+ if (!no_negative_zero) {
+ Label non_zero_result;
+ __ test(left->reg(), Operand(left->reg()));
+ __ j(not_zero, &non_zero_result);
+ __ test(right->reg(), Operand(right->reg()));
+ deferred->Branch(negative);
+ __ bind(&non_zero_result);
+ }
// Check for the corner case of dividing the most negative smi by
// -1. We cannot use the overflow flag, since it is not set by
// idiv instruction.
@@ -1423,12 +1431,14 @@
// the dividend is negative, return a floating point negative
// zero. The frame is unchanged in this block, so local control
// flow can use a Label rather than a JumpTarget.
- Label non_zero_result;
- __ test(edx, Operand(edx));
- __ j(not_zero, &non_zero_result, taken);
- __ test(left->reg(), Operand(left->reg()));
- deferred->Branch(negative);
- __ bind(&non_zero_result);
+ if (!no_negative_zero) {
+ Label non_zero_result;
+ __ test(edx, Operand(edx));
+ __ j(not_zero, &non_zero_result, taken);
+ __ test(left->reg(), Operand(left->reg()));
+ deferred->Branch(negative);
+ __ bind(&non_zero_result);
+ }
deferred->BindExit();
left->Unuse();
right->Unuse();
@@ -1571,14 +1581,16 @@
// argument is negative, go to slow case. The frame is unchanged
// in this block, so local control flow can use a Label rather
// than a JumpTarget.
- Label non_zero_result;
- __ test(answer.reg(), Operand(answer.reg()));
- __ j(not_zero, &non_zero_result, taken);
- __ mov(answer.reg(), left->reg());
- __ or_(answer.reg(), Operand(right->reg()));
- deferred->Branch(negative);
- __ xor_(answer.reg(), Operand(answer.reg())); // Positive 0 is
correct.
- __ bind(&non_zero_result);
+ if (!no_negative_zero) {
+ Label non_zero_result;
+ __ test(answer.reg(), Operand(answer.reg()));
+ __ j(not_zero, &non_zero_result, taken);
+ __ mov(answer.reg(), left->reg());
+ __ or_(answer.reg(), Operand(right->reg()));
+ deferred->Branch(negative);
+ __ xor_(answer.reg(), Operand(answer.reg())); // Positive 0 is
correct.
+ __ bind(&non_zero_result);
+ }
break;
}
@@ -1817,7 +1829,8 @@
Handle<Object> value,
StaticType* type,
bool reversed,
- OverwriteMode
overwrite_mode) {
+ OverwriteMode
overwrite_mode,
+ bool no_negative_zero) {
// NOTE: This is an attempt to inline (a bit) more of the code for
// some possible smi operations (like + and -) when (at least) one
// of the operands is a constant smi.
@@ -1828,10 +1841,10 @@
Result unsafe_operand(value);
if (reversed) {
return LikelySmiBinaryOperation(op, &unsafe_operand, operand,
- overwrite_mode);
+ overwrite_mode, no_negative_zero);
} else {
return LikelySmiBinaryOperation(op, operand, &unsafe_operand,
- overwrite_mode);
+ overwrite_mode, no_negative_zero);
}
}
@@ -1911,7 +1924,7 @@
if (reversed) {
Result constant_operand(value);
answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
- overwrite_mode);
+ overwrite_mode,
no_negative_zero);
} else {
// Only the least significant 5 bits of the shift value are used.
// In the slow case, this masking is done inside the runtime call.
@@ -1947,7 +1960,7 @@
if (reversed) {
Result constant_operand(value);
answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
- overwrite_mode);
+ overwrite_mode,
no_negative_zero);
} else {
// Only the least significant 5 bits of the shift value are used.
// In the slow case, this masking is done inside the runtime call.
@@ -2140,10 +2153,10 @@
Result constant_operand(value);
if (reversed) {
answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
- overwrite_mode);
+ overwrite_mode,
no_negative_zero);
} else {
answer = LikelySmiBinaryOperation(op, operand, &constant_operand,
- overwrite_mode);
+ overwrite_mode,
no_negative_zero);
}
}
break;
@@ -2180,10 +2193,10 @@
Result constant_operand(value);
if (reversed) {
answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
- overwrite_mode);
+ overwrite_mode,
no_negative_zero);
} else {
answer = LikelySmiBinaryOperation(op, operand, &constant_operand,
- overwrite_mode);
+ overwrite_mode,
no_negative_zero);
}
break;
}
@@ -4950,7 +4963,8 @@
node->value()->AsBinaryOperation()->ResultOverwriteAllowed());
GenericBinaryOperation(node->binary_op(),
node->type(),
- overwrite_value ? OVERWRITE_RIGHT :
NO_OVERWRITE);
+ overwrite_value ? OVERWRITE_RIGHT :
NO_OVERWRITE,
+ node->no_negative_zero());
} else {
Load(node->value());
}
@@ -5027,7 +5041,8 @@
node->value()->AsBinaryOperation()->ResultOverwriteAllowed());
GenericBinaryOperation(node->binary_op(),
node->type(),
- overwrite_value ? OVERWRITE_RIGHT :
NO_OVERWRITE);
+ overwrite_value ? OVERWRITE_RIGHT :
NO_OVERWRITE,
+ node->no_negative_zero());
} else {
Load(node->value());
}
@@ -5106,7 +5121,8 @@
node->value()->AsBinaryOperation()->ResultOverwriteAllowed());
GenericBinaryOperation(node->binary_op(),
node->type(),
- overwrite_value ? OVERWRITE_RIGHT :
NO_OVERWRITE);
+ overwrite_value ? OVERWRITE_RIGHT :
NO_OVERWRITE,
+ node->no_negative_zero());
} else {
Load(node->value());
}
@@ -6868,7 +6884,8 @@
Load(node->left());
Load(node->right());
}
- GenericBinaryOperation(node->op(), node->type(), overwrite_mode);
+ GenericBinaryOperation(node->op(), node->type(),
+ overwrite_mode, node->no_negative_zero());
}
}
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.h Thu Mar 11 08:24:31 2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.h Mon Mar 15 07:03:36 2010
@@ -469,7 +469,8 @@
void GenericBinaryOperation(
Token::Value op,
StaticType* type,
- OverwriteMode overwrite_mode);
+ OverwriteMode overwrite_mode,
+ bool no_negative_zero);
// If possible, combine two constant smi values using op to produce
// a smi result, and push it on the virtual frame, all at compile time.
@@ -483,7 +484,8 @@
Handle<Object> constant_operand,
StaticType* type,
bool reversed,
- OverwriteMode overwrite_mode);
+ OverwriteMode overwrite_mode,
+ bool no_negative_zero);
// Emit code to perform a binary operation on two likely smis.
// The code to handle smi arguments is produced inline.
@@ -491,7 +493,8 @@
Result LikelySmiBinaryOperation(Token::Value op,
Result* left,
Result* right,
- OverwriteMode overwrite_mode);
+ OverwriteMode overwrite_mode,
+ bool no_negative_zero);
void Comparison(AstNode* node,
Condition cc,
=======================================
--- /branches/bleeding_edge/src/rewriter.cc Tue Mar 9 02:39:18 2010
+++ /branches/bleeding_edge/src/rewriter.cc Mon Mar 15 07:03:36 2010
@@ -220,6 +220,7 @@
void AstOptimizer::VisitConditional(Conditional* node) {
+ node->condition()->set_no_negative_zero(true);
Visit(node->condition());
Visit(node->then_expression());
Visit(node->else_expression());
@@ -319,6 +320,7 @@
node->type()->SetAsLikelySmiIfUnknown();
node->target()->type()->SetAsLikelySmiIfUnknown();
node->value()->type()->SetAsLikelySmiIfUnknown();
+ node->value()->set_no_negative_zero(true);
break;
case Token::ASSIGN_ADD:
case Token::ASSIGN_SUB:
@@ -393,6 +395,7 @@
void AstOptimizer::VisitProperty(Property* node) {
+ node->key()->set_no_negative_zero(true);
Visit(node->obj());
Visit(node->key());
}
@@ -422,6 +425,11 @@
void AstOptimizer::VisitUnaryOperation(UnaryOperation* node) {
+ if (node->op() == Token::ADD || node->op() == Token::SUB) {
+ node->expression()->set_no_negative_zero(node->no_negative_zero());
+ } else {
+ node->expression()->set_no_negative_zero(true);
+ }
Visit(node->expression());
if (FLAG_safe_int32_compiler) {
switch (node->op()) {
@@ -449,6 +457,9 @@
void AstOptimizer::VisitCountOperation(CountOperation* node) {
// Count operations assume that they work on Smis.
+ node->expression()->set_no_negative_zero(node->is_prefix() ?
+ true :
+ node->no_negative_zero());
node->type()->SetAsLikelySmiIfUnknown();
node->expression()->type()->SetAsLikelySmiIfUnknown();
Visit(node->expression());
@@ -461,7 +472,12 @@
switch (node->op()) {
case Token::COMMA:
case Token::OR:
+ node->left()->set_no_negative_zero(true);
+ node->right()->set_no_negative_zero(node->no_negative_zero());
+ break;
case Token::AND:
+ node->left()->set_no_negative_zero(node->no_negative_zero());
+ node->right()->set_no_negative_zero(node->no_negative_zero());
break;
case Token::BIT_OR:
case Token::BIT_XOR:
@@ -474,6 +490,8 @@
node->right()->type()->SetAsLikelySmiIfUnknown();
node->left()->set_to_int32(true);
node->right()->set_to_int32(true);
+ node->left()->set_no_negative_zero(true);
+ node->right()->set_no_negative_zero(true);
break;
case Token::ADD:
case Token::SUB:
@@ -484,6 +502,13 @@
node->left()->type()->SetAsLikelySmiIfUnknown();
node->right()->type()->SetAsLikelySmiIfUnknown();
}
+ node->left()->set_no_negative_zero(node->no_negative_zero());
+ node->right()->set_no_negative_zero(node->no_negative_zero());
+ if (node->op() == Token::DIV) {
+ node->right()->set_no_negative_zero(false);
+ } else if (node->op() == Token::MOD) {
+ node->right()->set_no_negative_zero(true);
+ }
break;
default:
UNREACHABLE();
@@ -550,6 +575,10 @@
node->left()->type()->SetAsLikelySmiIfUnknown();
node->right()->type()->SetAsLikelySmiIfUnknown();
}
+
+ node->left()->set_no_negative_zero(true);
+ // Only [[HasInstance]] has the right argument passed unchanged to it.
+ node->right()->set_no_negative_zero(true);
Visit(node->left());
Visit(node->right());
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
