Skip to content

V
V8
Commit 5a9c4aad authored by fschneider@chromium.org's avatar fschneider@chromium.org
Browse files
Options

Make the materialization of smi constants consistent between the two compilers. 

This change makes the full code generator always materialize smi constants to match the behavior
 of the environment tracked in the optimizing compiler. The mismatch between environment
and what the non-optimized code causes problems and potentially crashes when doing deoptimization
and OSR.

BUG=67732

Review URL: http://codereview.chromium.org/6592064

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6982 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
parent 6b1530ea
Hide whitespace changes
Inline Side-by-side
Showing with 17 additions and 547 deletions
src/arm/full-codegen-arm.cc
View file @ 5a9c4aad
......@@ -339,23 +339,6 @@ void FullCodeGenerator::EmitReturnSequence() {
}
FullCodeGenerator::ConstantOperand FullCodeGenerator::GetConstantOperand(
Token::Value op, Expression* left, Expression* right) {
ASSERT(ShouldInlineSmiCase(op));
if (op == Token::DIV || op == Token::MOD || op == Token::MUL) {
// We never generate inlined constant smi operations for these.
return kNoConstants;
} else if (right->IsSmiLiteral()) {
return kRightConstant;
} else if (left->IsSmiLiteral() && !Token::IsShiftOp(op)) {
// Don't inline shifts with constant left hand side.
return kLeftConstant;
} else {
return kNoConstants;
}
}
void FullCodeGenerator::EffectContext::Plug(Slot* slot) const {
}
......@@ -1634,14 +1617,8 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
}
Token::Value op = expr->binary_op();
ConstantOperand constant = ShouldInlineSmiCase(op)
? GetConstantOperand(op, expr->target(), expr->value())
: kNoConstants;
ASSERT(constant == kRightConstant || constant == kNoConstants);
if (constant == kNoConstants) {
__ push(r0); // Left operand goes on the stack.
VisitForAccumulatorValue(expr->value());
}
__ push(r0); // Left operand goes on the stack.
VisitForAccumulatorValue(expr->value());
OverwriteMode mode = expr->value()->ResultOverwriteAllowed()
? OVERWRITE_RIGHT
......@@ -1653,8 +1630,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
op,
mode,
expr->target(),
expr->value(),
constant);
expr->value());
} else {
EmitBinaryOp(op, mode);
}
......@@ -1704,217 +1680,11 @@ void FullCodeGenerator::EmitKeyedPropertyLoad(Property* prop) {
}
void FullCodeGenerator::EmitConstantSmiAdd(Expression* expr,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value) {
Label call_stub, done;
// Optimistically add smi value with unknown object. If result overflows or is
// not a smi then we had either a smi overflow or added a smi with a tagged
// pointer.
__ mov(r1, Operand(value));
__ add(r2, r0, r1, SetCC);
__ b(vs, &call_stub);
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfNotSmi(r2, &call_stub);
__ mov(r0, r2);
__ b(&done);
// Call the shared stub.
__ bind(&call_stub);
if (!left_is_constant_smi) {
__ Swap(r0, r1, r2);
}
TypeRecordingBinaryOpStub stub(Token::ADD, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ bind(&done);
context()->Plug(r0);
}
void FullCodeGenerator::EmitConstantSmiSub(Expression* expr,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value) {
Label call_stub, done;
// Optimistically subtract smi value and unknown object. If result overflows
// or is not a smi then we had either a smi overflow or subtraction between a
// smi and a tagged pointer.
__ mov(r1, Operand(value));
if (left_is_constant_smi) {
__ sub(r2, r1, r0, SetCC);
} else {
__ sub(r2, r0, r1, SetCC);
}
__ b(vs, &call_stub);
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfNotSmi(r2, &call_stub);
__ mov(r0, r2);
__ b(&done);
// Call the shared stub.
__ bind(&call_stub);
if (!left_is_constant_smi) {
__ Swap(r0, r1, r2);
}
TypeRecordingBinaryOpStub stub(Token::SUB, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ bind(&done);
context()->Plug(r0);
}
void FullCodeGenerator::EmitConstantSmiShiftOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Smi* value) {
Label call_stub, smi_case, done;
int shift_value = value->value() & 0x1f;
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfSmi(r0, &smi_case);
// Call stub.
__ bind(&call_stub);
__ mov(r1, r0);
__ mov(r0, Operand(value));
TypeRecordingBinaryOpStub stub(op, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ b(&done);
// Smi case.
__ bind(&smi_case);
switch (op) {
case Token::SHL:
if (shift_value != 0) {
__ mov(r1, r0);
if (shift_value > 1) {
__ mov(r1, Operand(r1, LSL, shift_value - 1));
}
// Convert int result to smi, checking that it is in int range.
__ SmiTag(r1, SetCC);
__ b(vs, &call_stub);
__ mov(r0, r1); // Put result back into r0.
}
break;
case Token::SAR:
if (shift_value != 0) {
__ mov(r0, Operand(r0, ASR, shift_value));
__ bic(r0, r0, Operand(kSmiTagMask));
}
break;
case Token::SHR:
// SHR must return a positive value. When shifting by 0 or 1 we need to
// check that smi tagging the result will not create a negative value.
if (shift_value < 2) {
__ mov(r2, Operand(shift_value));
__ SmiUntag(r1, r0);
if (shift_value != 0) {
__ mov(r1, Operand(r1, LSR, shift_value));
}
__ tst(r1, Operand(0xc0000000));
__ b(ne, &call_stub);
__ SmiTag(r0, r1); // result in r0.
} else {
__ SmiUntag(r0);
__ mov(r0, Operand(r0, LSR, shift_value));
__ SmiTag(r0);
}
break;
default:
UNREACHABLE();
}
__ bind(&done);
context()->Plug(r0);
}
void FullCodeGenerator::EmitConstantSmiBitOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Smi* value) {
Label smi_case, done;
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfSmi(r0, &smi_case);
// The order of the arguments does not matter for bit-ops with a
// constant operand.
__ mov(r1, Operand(value));
TypeRecordingBinaryOpStub stub(op, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ jmp(&done);
// Smi case.
__ bind(&smi_case);
__ mov(r1, Operand(value));
switch (op) {
case Token::BIT_OR:
__ orr(r0, r0, Operand(r1));
break;
case Token::BIT_XOR:
__ eor(r0, r0, Operand(r1));
break;
case Token::BIT_AND:
__ and_(r0, r0, Operand(r1));
break;
default:
UNREACHABLE();
}
__ bind(&done);
context()->Plug(r0);
}
void FullCodeGenerator::EmitConstantSmiBinaryOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value) {
switch (op) {
case Token::BIT_OR:
case Token::BIT_XOR:
case Token::BIT_AND:
EmitConstantSmiBitOp(expr, op, mode, value);
break;
case Token::SHL:
case Token::SAR:
case Token::SHR:
ASSERT(!left_is_constant_smi);
EmitConstantSmiShiftOp(expr, op, mode, value);
break;
case Token::ADD:
EmitConstantSmiAdd(expr, mode, left_is_constant_smi, value);
break;
case Token::SUB:
EmitConstantSmiSub(expr, mode, left_is_constant_smi, value);
break;
default:
UNREACHABLE();
}
}
void FullCodeGenerator::EmitInlineSmiBinaryOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Expression* left_expr,
Expression* right_expr,
ConstantOperand constant) {
if (constant == kRightConstant) {
Smi* value = Smi::cast(*right_expr->AsLiteral()->handle());
EmitConstantSmiBinaryOp(expr, op, mode, false, value);
return;
} else if (constant == kLeftConstant) {
Smi* value = Smi::cast(*left_expr->AsLiteral()->handle());
EmitConstantSmiBinaryOp(expr, op, mode, true, value);
return;
}
Expression* right_expr) {
Label done, smi_case, stub_call;
Register scratch1 = r2;
......
src/full-codegen.cc
View file @ 5a9c4aad
......@@ -739,25 +739,13 @@ void FullCodeGenerator::VisitBinaryOperation(BinaryOperation* expr) {
case Token::SHL:
case Token::SHR:
case Token::SAR: {
// Figure out if either of the operands is a constant.
ConstantOperand constant = ShouldInlineSmiCase(op)
? GetConstantOperand(op, left, right)
: kNoConstants;
// Load only the operands that we need to materialize.
if (constant == kNoConstants) {
VisitForStackValue(left);
VisitForAccumulatorValue(right);
} else if (constant == kRightConstant) {
VisitForAccumulatorValue(left);
} else {
ASSERT(constant == kLeftConstant);
VisitForAccumulatorValue(right);
}
// Load both operands.
VisitForStackValue(left);
VisitForAccumulatorValue(right);
SetSourcePosition(expr->position());
if (ShouldInlineSmiCase(op)) {
EmitInlineSmiBinaryOp(expr, op, mode, left, right, constant);
EmitInlineSmiBinaryOp(expr, op, mode, left, right);
} else {
EmitBinaryOp(op, mode);
}
......
src/full-codegen.h
View file @ 5a9c4aad
......@@ -274,12 +274,6 @@ class FullCodeGenerator: public AstVisitor {
ForwardBailoutStack* const parent_;
};
enum ConstantOperand {
kNoConstants,
kLeftConstant,
kRightConstant
};
// Type of a member function that generates inline code for a native function.
typedef void (FullCodeGenerator::*InlineFunctionGenerator)
(ZoneList<Expression*>*);
......@@ -298,11 +292,6 @@ class FullCodeGenerator: public AstVisitor {
// operation.
bool ShouldInlineSmiCase(Token::Value op);
// Compute which (if any) of the operands is a compile-time constant.
ConstantOperand GetConstantOperand(Token::Value op,
Expression* left,
Expression* right);
// Helper function to convert a pure value into a test context. The value
// is expected on the stack or the accumulator, depending on the platform.
// See the platform-specific implementation for details.
......@@ -457,34 +446,7 @@ class FullCodeGenerator: public AstVisitor {
Token::Value op,
OverwriteMode mode,
Expression* left,
Expression* right,
ConstantOperand constant);
void EmitConstantSmiBinaryOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value);
void EmitConstantSmiBitOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Smi* value);
void EmitConstantSmiShiftOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Smi* value);
void EmitConstantSmiAdd(Expression* expr,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value);
void EmitConstantSmiSub(Expression* expr,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value);
Expression* right);
// Assign to the given expression as if via '='. The right-hand-side value
// is expected in the accumulator.
......
src/ia32/full-codegen-ia32.cc
View file @ 5a9c4aad
......@@ -322,23 +322,6 @@ void FullCodeGenerator::EmitReturnSequence() {
}
FullCodeGenerator::ConstantOperand FullCodeGenerator::GetConstantOperand(
Token::Value op, Expression* left, Expression* right) {
ASSERT(ShouldInlineSmiCase(op));
if (op == Token::DIV || op == Token::MOD || op == Token::MUL) {
// We never generate inlined constant smi operations for these.
return kNoConstants;
} else if (right->IsSmiLiteral()) {
return kRightConstant;
} else if (left->IsSmiLiteral() && !Token::IsShiftOp(op)) {
// Don't inline shifts with constant left hand side.
return kLeftConstant;
} else {
return kNoConstants;
}
}
void FullCodeGenerator::EffectContext::Plug(Slot* slot) const {
}
......@@ -1572,14 +1555,8 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
}
Token::Value op = expr->binary_op();
ConstantOperand constant = ShouldInlineSmiCase(op)
? GetConstantOperand(op, expr->target(), expr->value())
: kNoConstants;
ASSERT(constant == kRightConstant || constant == kNoConstants);
if (constant == kNoConstants) {
__ push(eax); // Left operand goes on the stack.
VisitForAccumulatorValue(expr->value());
}
__ push(eax); // Left operand goes on the stack.
VisitForAccumulatorValue(expr->value());
OverwriteMode mode = expr->value()->ResultOverwriteAllowed()
? OVERWRITE_RIGHT
......@@ -1591,8 +1568,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
op,
mode,
expr->target(),
expr->value(),
constant);
expr->value());
} else {
EmitBinaryOp(op, mode);
}
......@@ -1640,220 +1616,11 @@ void FullCodeGenerator::EmitKeyedPropertyLoad(Property* prop) {
}
void FullCodeGenerator::EmitConstantSmiAdd(Expression* expr,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value) {
NearLabel call_stub, done;
// Optimistically add smi value with unknown object. If result overflows or is
// not a smi then we had either a smi overflow or added a smi with a tagged
// pointer.
__ add(Operand(eax), Immediate(value));
__ j(overflow, &call_stub);
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfSmi(eax, &done);
// Undo the optimistic add operation and call the shared stub.
__ bind(&call_stub);
__ sub(Operand(eax), Immediate(value));
TypeRecordingBinaryOpStub stub(Token::ADD, mode);
if (left_is_constant_smi) {
__ mov(edx, Immediate(value));
} else {
__ mov(edx, eax);
__ mov(eax, Immediate(value));
}
EmitCallIC(stub.GetCode(), &patch_site);
__ bind(&done);
context()->Plug(eax);
}
void FullCodeGenerator::EmitConstantSmiSub(Expression* expr,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value) {
NearLabel call_stub, done;
// Optimistically subtract smi value with unknown object. If result overflows
// or is not a smi then we had either a smi overflow or added a smi with a
// tagged pointer.
if (left_is_constant_smi) {
__ mov(ecx, eax);
__ mov(eax, Immediate(value));
__ sub(Operand(eax), ecx);
} else {
__ sub(Operand(eax), Immediate(value));
}
__ j(overflow, &call_stub);
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfSmi(eax, &done);
__ bind(&call_stub);
if (left_is_constant_smi) {
__ mov(edx, Immediate(value));
__ mov(eax, ecx);
} else {
__ add(Operand(eax), Immediate(value)); // Undo the subtraction.
__ mov(edx, eax);
__ mov(eax, Immediate(value));
}
TypeRecordingBinaryOpStub stub(Token::SUB, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ bind(&done);
context()->Plug(eax);
}
void FullCodeGenerator::EmitConstantSmiShiftOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Smi* value) {
NearLabel call_stub, smi_case, done;
int shift_value = value->value() & 0x1f;
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfSmi(eax, &smi_case);
// Call stub.
__ bind(&call_stub);
__ mov(edx, eax);
__ mov(eax, Immediate(value));
TypeRecordingBinaryOpStub stub(op, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ jmp(&done);
// Smi case.
__ bind(&smi_case);
switch (op) {
case Token::SHL:
if (shift_value != 0) {
__ mov(edx, eax);
if (shift_value > 1) {
__ shl(edx, shift_value - 1);
}
// Convert int result to smi, checking that it is in int range.
STATIC_ASSERT(kSmiTagSize == 1); // Adjust code if not the case.
__ add(edx, Operand(edx));
__ j(overflow, &call_stub);
__ mov(eax, edx); // Put result back into eax.
}
break;
case Token::SAR:
if (shift_value != 0) {
__ sar(eax, shift_value);
__ and_(eax, ~kSmiTagMask);
}
break;
case Token::SHR:
// SHR must return a positive value. When shifting by 0 or 1 we need to
// check that smi tagging the result will not create a negative value.
if (shift_value < 2) {
__ mov(edx, eax);
__ SmiUntag(edx);
__ shr(edx, shift_value);
__ test(edx, Immediate(0xc0000000));
__ j(not_zero, &call_stub);
__ SmiTag(edx);
__ mov(eax, edx); // Put result back into eax.
} else {
__ SmiUntag(eax);
__ shr(eax, shift_value);
__ SmiTag(eax);
}
break;
default:
UNREACHABLE();
}
__ bind(&done);
context()->Plug(eax);
}
void FullCodeGenerator::EmitConstantSmiBitOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Smi* value) {
NearLabel smi_case, done;
JumpPatchSite patch_site(masm_);
patch_site.EmitJumpIfSmi(eax, &smi_case);
// The order of the arguments does not matter for bit-ops with a
// constant operand.
__ mov(edx, Immediate(value));
TypeRecordingBinaryOpStub stub(op, mode);
EmitCallIC(stub.GetCode(), &patch_site);
__ jmp(&done);
// Smi case.
__ bind(&smi_case);
switch (op) {
case Token::BIT_OR:
__ or_(Operand(eax), Immediate(value));
break;
case Token::BIT_XOR:
__ xor_(Operand(eax), Immediate(value));
break;
case Token::BIT_AND:
__ and_(Operand(eax), Immediate(value));
break;
default:
UNREACHABLE();
}
__ bind(&done);
context()->Plug(eax);
}
void FullCodeGenerator::EmitConstantSmiBinaryOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
bool left_is_constant_smi,
Smi* value) {
switch (op) {
case Token::BIT_OR:
case Token::BIT_XOR:
case Token::BIT_AND:
EmitConstantSmiBitOp(expr, op, mode, value);
break;
case Token::SHL:
case Token::SAR:
case Token::SHR:
ASSERT(!left_is_constant_smi);
EmitConstantSmiShiftOp(expr, op, mode, value);
break;
case Token::ADD:
EmitConstantSmiAdd(expr, mode, left_is_constant_smi, value);
break;
case Token::SUB:
EmitConstantSmiSub(expr, mode, left_is_constant_smi, value);
break;
default:
UNREACHABLE();
}
}
void FullCodeGenerator::EmitInlineSmiBinaryOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Expression* left,
Expression* right,
ConstantOperand constant) {
if (constant == kRightConstant) {
Smi* value = Smi::cast(*right->AsLiteral()->handle());
EmitConstantSmiBinaryOp(expr, op, mode, false, value);
return;
} else if (constant == kLeftConstant) {
Smi* value = Smi::cast(*left->AsLiteral()->handle());
EmitConstantSmiBinaryOp(expr, op, mode, true, value);
return;
}
Expression* right) {
// Do combined smi check of the operands. Left operand is on the
// stack. Right operand is in eax.
NearLabel done, smi_case, stub_call;
......
src/x64/full-codegen-x64.cc
View file @ 5a9c4aad
......@@ -327,13 +327,6 @@ void FullCodeGenerator::EmitReturnSequence() {
}
FullCodeGenerator::ConstantOperand FullCodeGenerator::GetConstantOperand(
Token::Value op, Expression* left, Expression* right) {
ASSERT(ShouldInlineSmiCase(op));
return kNoConstants;
}
void FullCodeGenerator::EffectContext::Plug(Slot* slot) const {
}
......@@ -1579,14 +1572,8 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
}
Token::Value op = expr->binary_op();
ConstantOperand constant = ShouldInlineSmiCase(op)
? GetConstantOperand(op, expr->target(), expr->value())
: kNoConstants;
ASSERT(constant == kRightConstant || constant == kNoConstants);
if (constant == kNoConstants) {
__ push(rax); // Left operand goes on the stack.
VisitForAccumulatorValue(expr->value());
}
__ push(rax); // Left operand goes on the stack.
VisitForAccumulatorValue(expr->value());
OverwriteMode mode = expr->value()->ResultOverwriteAllowed()
? OVERWRITE_RIGHT
......@@ -1598,8 +1585,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
op,
mode,
expr->target(),
expr->value(),
constant);
expr->value());
} else {
EmitBinaryOp(op, mode);
}
......@@ -1650,10 +1636,7 @@ void FullCodeGenerator::EmitInlineSmiBinaryOp(Expression* expr,
Token::Value op,
OverwriteMode mode,
Expression* left,
Expression* right,
ConstantOperand constant) {
ASSERT(constant == kNoConstants); // Only handled case.
Expression* right) {
// Do combined smi check of the operands. Left operand is on the
// stack (popped into rdx). Right operand is in rax but moved into
// rcx to make the shifts easier.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment