Commit 6b5949a8 authored by rmcilroy's avatar rmcilroy Committed by Commit bot

[Interpreter] Avoid accessing on-heap literal in VisitLiteral.

Move VisitLiteral to decide what type of literal is being emitted by
checking the raw ASTValue type, instead of the internalized on-heap
value. This is required for concurrent bytecode generation.

As part of this change, the NUMBER AstValue constructor is modified to
try to convert numbers without a dot to SMIs where possible. This is to
maintain the behavior in NewNumber where such numbers are internalized as
SMIs, and ensures that we still emit LdaSmi bytecodes for these values
in the generated bytecode.

BUG=v8:5203

Review-Url: https://codereview.chromium.org/2152853002
Cr-Commit-Position: refs/heads/master@{#37931}
parent 57981a48
...@@ -146,10 +146,8 @@ class AstValue : public ZoneObject { ...@@ -146,10 +146,8 @@ class AstValue : public ZoneObject {
bool ContainsDot() const { return type_ == NUMBER_WITH_DOT; } bool ContainsDot() const { return type_ == NUMBER_WITH_DOT; }
const AstRawString* AsString() const { const AstRawString* AsString() const {
if (type_ == STRING) CHECK_EQ(STRING, type_);
return string_; return string_;
UNREACHABLE();
return 0;
} }
double AsNumber() const { double AsNumber() const {
...@@ -161,6 +159,11 @@ class AstValue : public ZoneObject { ...@@ -161,6 +159,11 @@ class AstValue : public ZoneObject {
return 0; return 0;
} }
Smi* AsSmi() const {
CHECK_EQ(SMI, type_);
return Smi::FromInt(smi_);
}
bool EqualsString(const AstRawString* string) const { bool EqualsString(const AstRawString* string) const {
return type_ == STRING && string_ == string; return type_ == STRING && string_ == string;
} }
...@@ -169,7 +172,12 @@ class AstValue : public ZoneObject { ...@@ -169,7 +172,12 @@ class AstValue : public ZoneObject {
bool BooleanValue() const; bool BooleanValue() const;
bool IsSmi() const { return type_ == SMI; }
bool IsFalse() const { return type_ == BOOLEAN && !bool_; }
bool IsTrue() const { return type_ == BOOLEAN && bool_; }
bool IsUndefined() const { return type_ == UNDEFINED; }
bool IsTheHole() const { return type_ == THE_HOLE; } bool IsTheHole() const { return type_ == THE_HOLE; }
bool IsNull() const { return type_ == NULL_TYPE; }
void Internalize(Isolate* isolate); void Internalize(Isolate* isolate);
...@@ -204,10 +212,17 @@ class AstValue : public ZoneObject { ...@@ -204,10 +212,17 @@ class AstValue : public ZoneObject {
explicit AstValue(double n, bool with_dot) { explicit AstValue(double n, bool with_dot) {
if (with_dot) { if (with_dot) {
type_ = NUMBER_WITH_DOT; type_ = NUMBER_WITH_DOT;
number_ = n;
} else { } else {
type_ = NUMBER; int int_value;
if (DoubleToSmiInteger(n, &int_value)) {
type_ = SMI;
smi_ = int_value;
} else {
type_ = NUMBER;
number_ = n;
}
} }
number_ = n;
} }
AstValue(Type t, int i) : type_(t) { AstValue(Type t, int i) : type_(t) {
......
...@@ -97,6 +97,13 @@ int32_t DoubleToInt32(double x) { ...@@ -97,6 +97,13 @@ int32_t DoubleToInt32(double x) {
} }
} }
bool DoubleToSmiInteger(double value, int* smi_int_value) {
if (IsMinusZero(value)) return false;
int i = FastD2IChecked(value);
if (value != i || !Smi::IsValid(i)) return false;
*smi_int_value = i;
return true;
}
bool IsSmiDouble(double value) { bool IsSmiDouble(double value) {
return !IsMinusZero(value) && value >= Smi::kMinValue && return !IsMinusZero(value) && value >= Smi::kMinValue &&
......
...@@ -147,27 +147,26 @@ char* DoubleToExponentialCString(double value, int f); ...@@ -147,27 +147,26 @@ char* DoubleToExponentialCString(double value, int f);
char* DoubleToPrecisionCString(double value, int f); char* DoubleToPrecisionCString(double value, int f);
char* DoubleToRadixCString(double value, int radix); char* DoubleToRadixCString(double value, int radix);
static inline bool IsMinusZero(double value) { static inline bool IsMinusZero(double value) {
return bit_cast<int64_t>(value) == bit_cast<int64_t>(-0.0); return bit_cast<int64_t>(value) == bit_cast<int64_t>(-0.0);
} }
// Returns true if value can be converted to a SMI, and returns the resulting
// integer value of the SMI in |smi_int_value|.
inline bool DoubleToSmiInteger(double value, int* smi_int_value);
inline bool IsSmiDouble(double value); inline bool IsSmiDouble(double value);
// Integer32 is an integer that can be represented as a signed 32-bit // Integer32 is an integer that can be represented as a signed 32-bit
// integer. It has to be in the range [-2^31, 2^31 - 1]. // integer. It has to be in the range [-2^31, 2^31 - 1].
// We also have to check for negative 0 as it is not an Integer32. // We also have to check for negative 0 as it is not an Integer32.
inline bool IsInt32Double(double value); inline bool IsInt32Double(double value);
// UInteger32 is an integer that can be represented as an unsigned 32-bit // UInteger32 is an integer that can be represented as an unsigned 32-bit
// integer. It has to be in the range [0, 2^32 - 1]. // integer. It has to be in the range [0, 2^32 - 1].
// We also have to check for negative 0 as it is not a UInteger32. // We also have to check for negative 0 as it is not a UInteger32.
inline bool IsUint32Double(double value); inline bool IsUint32Double(double value);
// Convert from Number object to C integer. // Convert from Number object to C integer.
inline int32_t NumberToInt32(Object* number); inline int32_t NumberToInt32(Object* number);
inline uint32_t NumberToUint32(Object* number); inline uint32_t NumberToUint32(Object* number);
......
...@@ -1094,13 +1094,9 @@ Handle<FixedDoubleArray> Factory::CopyFixedDoubleArray( ...@@ -1094,13 +1094,9 @@ Handle<FixedDoubleArray> Factory::CopyFixedDoubleArray(
Handle<Object> Factory::NewNumber(double value, Handle<Object> Factory::NewNumber(double value,
PretenureFlag pretenure) { PretenureFlag pretenure) {
// We need to distinguish the minus zero value and this cannot be // Materialize as a SMI if possible
// done after conversion to int. Doing this by comparing bit int32_t int_value;
// patterns is faster than using fpclassify() et al. if (DoubleToSmiInteger(value, &int_value)) {
if (IsMinusZero(value)) return NewHeapNumber(-0.0, IMMUTABLE, pretenure);
int int_value = FastD2IChecked(value);
if (value == int_value && Smi::IsValid(int_value)) {
return handle(Smi::FromInt(int_value), isolate()); return handle(Smi::FromInt(int_value), isolate());
} }
......
...@@ -549,7 +549,7 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info) ...@@ -549,7 +549,7 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
register_allocator_(nullptr), register_allocator_(nullptr),
generator_resume_points_(info->literal()->yield_count(), info->zone()), generator_resume_points_(info->literal()->yield_count(), info->zone()),
generator_state_() { generator_state_() {
InitializeAstVisitor(isolate()); InitializeAstVisitor(isolate()->stack_guard()->real_climit());
} }
Handle<BytecodeArray> BytecodeGenerator::MakeBytecode() { Handle<BytecodeArray> BytecodeGenerator::MakeBytecode() {
...@@ -1484,21 +1484,21 @@ void BytecodeGenerator::VisitConditional(Conditional* expr) { ...@@ -1484,21 +1484,21 @@ void BytecodeGenerator::VisitConditional(Conditional* expr) {
void BytecodeGenerator::VisitLiteral(Literal* expr) { void BytecodeGenerator::VisitLiteral(Literal* expr) {
if (!execution_result()->IsEffect()) { if (!execution_result()->IsEffect()) {
Handle<Object> value = expr->value(); const AstValue* raw_value = expr->raw_value();
if (value->IsSmi()) { if (raw_value->IsSmi()) {
builder()->LoadLiteral(Smi::cast(*value)); builder()->LoadLiteral(raw_value->AsSmi());
} else if (value->IsUndefined(isolate())) { } else if (raw_value->IsUndefined()) {
builder()->LoadUndefined(); builder()->LoadUndefined();
} else if (value->IsTrue(isolate())) { } else if (raw_value->IsTrue()) {
builder()->LoadTrue(); builder()->LoadTrue();
} else if (value->IsFalse(isolate())) { } else if (raw_value->IsFalse()) {
builder()->LoadFalse(); builder()->LoadFalse();
} else if (value->IsNull(isolate())) { } else if (raw_value->IsNull()) {
builder()->LoadNull(); builder()->LoadNull();
} else if (value->IsTheHole(isolate())) { } else if (raw_value->IsTheHole()) {
builder()->LoadTheHole(); builder()->LoadTheHole();
} else { } else {
builder()->LoadLiteral(value); builder()->LoadLiteral(raw_value->value());
} }
execution_result()->SetResultInAccumulator(); execution_result()->SetResultInAccumulator();
} }
...@@ -2861,6 +2861,8 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) { ...@@ -2861,6 +2861,8 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
} }
void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) { void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) {
// TODO(rmcilroy): Special case "x * 1.0" and "x * -1" which are generated for
// +x and -x by the parser.
Register lhs = VisitForRegisterValue(expr->left()); Register lhs = VisitForRegisterValue(expr->left());
VisitForAccumulatorValue(expr->right()); VisitForAccumulatorValue(expr->right());
builder()->BinaryOperation(expr->op(), lhs); builder()->BinaryOperation(expr->op(), lhs);
......
...@@ -176,3 +176,21 @@ constant pool: [ ...@@ -176,3 +176,21 @@ constant pool: [
handlers: [ handlers: [
] ]
---
snippet: "
return 2.0;
"
frame size: 0
parameter count: 1
bytecode array length: 4
bytecodes: [
/* 30 E> */ B(StackCheck),
/* 34 S> */ B(LdaConstant), U8(0),
/* 46 S> */ B(Return),
]
constant pool: [
2,
]
handlers: [
]
...@@ -150,11 +150,12 @@ bytecodes: [ ...@@ -150,11 +150,12 @@ bytecodes: [
/* 30 E> */ B(StackCheck), /* 30 E> */ B(StackCheck),
/* 42 S> */ B(LdaSmi), U8(13), /* 42 S> */ B(LdaSmi), U8(13),
B(Star), R(0), B(Star), R(0),
/* 46 S> */ B(LdaSmi), U8(1), /* 46 S> */ B(LdaConstant), U8(0),
B(Mul), R(0), B(Mul), R(0),
/* 57 S> */ B(Return), /* 57 S> */ B(Return),
] ]
constant pool: [ constant pool: [
1,
] ]
handlers: [ handlers: [
] ]
......
...@@ -177,6 +177,8 @@ TEST(PrimitiveReturnStatements) { ...@@ -177,6 +177,8 @@ TEST(PrimitiveReturnStatements) {
"return +127;\n", "return +127;\n",
"return -128;\n", "return -128;\n",
"return 2.0;\n",
}; };
CHECK(CompareTexts(BuildActual(printer, snippets), CHECK(CompareTexts(BuildActual(printer, snippets),
......
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