Reviewers: oth, ulan,
Message:
Orion: could you please take a look.
Ulan: could you please RS for OWNERS (any comments welcome too).
Description:
[interpreter] A couple of minor tweaks to BytecodeArray.
- Ensure frame_size is always set during allocation.
- Add DCHECKs that frame_size is a valid value
- Remove locals_count, which we don't need yet (possibly every)
BUG=v8:4280
LOG=N
Please review this at https://codereview.chromium.org/1254873002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+29, -23 lines):
M src/factory.h
M src/factory.cc
M src/heap/heap.h
M src/heap/heap.cc
M src/interpreter/interpreter.h
M src/objects.h
M src/objects.cc
M src/objects-inl.h
M test/cctest/test-heap.cc
Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index
4fb5ec4644f8c972505f71a81ab9c8f647ac5ba9..e2a5e05693a1202c1f1a41b1ccc7bb47d78580ee
100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -874,10 +874,11 @@ Handle<ByteArray> Factory::NewByteArray(int length,
PretenureFlag pretenure) {
Handle<BytecodeArray> Factory::NewBytecodeArray(int length,
- const byte* raw_bytecodes)
{
+ const byte* raw_bytecodes,
+ int frame_size) {
DCHECK(0 <= length);
CALL_HEAP_FUNCTION(isolate(), isolate()->heap()->AllocateBytecodeArray(
- length, raw_bytecodes),
+ length, raw_bytecodes, frame_size),
BytecodeArray);
}
Index: src/factory.h
diff --git a/src/factory.h b/src/factory.h
index
478a48e671ed9edaf2d15dcc31e0f2a9656db085..1253779a1bb3071185ebd9cb124ff9fd276dcb21
100644
--- a/src/factory.h
+++ b/src/factory.h
@@ -283,7 +283,8 @@ class Factory final {
Handle<ByteArray> NewByteArray(int length,
PretenureFlag pretenure = NOT_TENURED);
- Handle<BytecodeArray> NewBytecodeArray(int length, const byte*
raw_bytecodes);
+ Handle<BytecodeArray> NewBytecodeArray(int length, const byte*
raw_bytecodes,
+ int frame_size);
Handle<ExternalArray> NewExternalArray(
int length,
Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index
8fb4454c9bb094872c874f56c7da0c2f549ec91f..84ea2cc4c9f0091651bf3d90b4ba66d7581b668e
100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -3068,7 +3068,9 @@ bool Heap::CreateInitialMaps() {
set_empty_byte_array(byte_array);
BytecodeArray* bytecode_array;
- if (!AllocateBytecodeArray(0, nullptr).To(&bytecode_array)) {
+ AllocationResult allocation =
+ AllocateBytecodeArray(0, nullptr, kPointerSize);
+ if (!allocation.To(&bytecode_array)) {
return false;
}
set_empty_bytecode_array(bytecode_array);
@@ -3821,7 +3823,8 @@ AllocationResult Heap::AllocateByteArray(int length,
PretenureFlag pretenure) {
AllocationResult Heap::AllocateBytecodeArray(int length,
- const byte* const
raw_bytecodes) {
+ const byte* const
raw_bytecodes,
+ int frame_size) {
if (length < 0 || length > BytecodeArray::kMaxLength) {
v8::internal::Heap::FatalProcessOutOfMemory("invalid array length",
true);
}
@@ -3836,6 +3839,7 @@ AllocationResult Heap::AllocateBytecodeArray(int
length,
result->set_map_no_write_barrier(bytecode_array_map());
BytecodeArray* instance = BytecodeArray::cast(result);
instance->set_length(length);
+ instance->set_frame_size(frame_size);
CopyBytes(instance->GetFirstBytecodeAddress(), raw_bytecodes, length);
return result;
Index: src/heap/heap.h
diff --git a/src/heap/heap.h b/src/heap/heap.h
index
c98188418882a5e63c8bf17fd246a3c8178c013d..edc3d2c8dfe9b9359070b12fd91410e8c7b4ebfd
100644
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -1682,7 +1682,8 @@ class Heap {
// Allocates a bytecode array with given contents.
MUST_USE_RESULT AllocationResult
- AllocateBytecodeArray(int length, const byte* raw_bytecodes);
+ AllocateBytecodeArray(int length, const byte* raw_bytecodes,
+ int frame_size);
// Copy the code and scope info part of the code object, but insert
// the provided data as the relocation information.
Index: src/interpreter/interpreter.h
diff --git a/src/interpreter/interpreter.h b/src/interpreter/interpreter.h
index
b084ec92dc4443cbb145f2abb22d8d701f8b5747..9af55d997d755e309bf8710ebd063c69ee815fc4
100644
--- a/src/interpreter/interpreter.h
+++ b/src/interpreter/interpreter.h
@@ -30,8 +30,6 @@ class Interpreter {
void Initialize(bool create_heap_objects);
- static bool MakeBytecode(CompilationInfo* info);
-
private:
// Bytecode handler generator functions.
#define DECLARE_BYTECODE_HANDLER_GENERATOR(Name, _) \
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
2bd8b533ace8ac88d0801d2559a5f6ba33b420c1..1a0664f8887ff1b235f8558d3f2a0fe9fae050f7
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -3639,8 +3639,17 @@ void BytecodeArray::set(int index, byte value) {
}
-INT_ACCESSORS(BytecodeArray, frame_size, kFrameSizeOffset)
-INT_ACCESSORS(BytecodeArray, number_of_locals, kNumberOfLocalsOffset)
+void BytecodeArray::set_frame_size(int frame_size) {
+ // We need at least one stack slot for the return register.
+ DCHECK_GE(frame_size, kPointerSize);
+ DCHECK(IsAligned(frame_size, static_cast<unsigned>(kPointerSize)));
+ WRITE_INT_FIELD(this, kFrameSizeOffset, frame_size);
+}
+
+
+int BytecodeArray::frame_size() const {
+ return READ_INT_FIELD(this, kFrameSizeOffset);
+}
Address BytecodeArray::GetFirstBytecodeAddress() {
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
844887b2b76bef032236b77e6ec592b4f429c4e1..5e218e80bde24d323e57cd372099f9147a52d2e9
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -11618,8 +11618,7 @@ void Code::Disassemble(const char* name,
std::ostream& os) { // NOLINT
void BytecodeArray::Disassemble(std::ostream& os) {
- os << "Frame size " << frame_size()
- << ", number of locals = " << number_of_locals() << "\n";
+ os << "Frame size " << frame_size() << "\n";
Vector<char> buf = Vector<char>::New(50);
int bytecode_size = 0;
for (int i = 0; i < this->length(); i += bytecode_size) {
@@ -11635,7 +11634,7 @@ void BytecodeArray::Disassemble(std::ostream& os) {
for (int j = bytecode_size; j < interpreter::Bytecodes::kMaximumSize;
j++) {
os << " ";
}
- os << bytecode;
+ os << bytecode << "\n";
}
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
caf109623c73bed0aef918a18e3be1453a751f98..e124c0200975b550440f441848bdee33cce5ce37
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -4254,8 +4254,6 @@ class BytecodeArray : public FixedArrayBase {
// Accessors for frame size and the number of locals
inline int frame_size() const;
inline void set_frame_size(int value);
- inline int number_of_locals() const;
- inline void set_number_of_locals(int value);
DECLARE_CAST(BytecodeArray)
@@ -4269,8 +4267,7 @@ class BytecodeArray : public FixedArrayBase {
// Layout description.
static const int kFrameSizeOffset = FixedArrayBase::kHeaderSize;
- static const int kNumberOfLocalsOffset = kFrameSizeOffset + kIntSize;
- static const int kHeaderSize = kNumberOfLocalsOffset + kIntSize;
+ static const int kHeaderSize = kFrameSizeOffset + kIntSize;
static const int kAlignedSize = OBJECT_POINTER_ALIGN(kHeaderSize);
Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index
e0f758df2b90721ff579a0632d5f655d9b65e14f..3729917eedfce16b4cdbcb67017c044bba7d2fa0
100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -571,7 +571,6 @@ TEST(BytecodeArray) {
static const uint8_t kRawBytes[] = {0xc3, 0x7e, 0xa5, 0x5a};
static const int kRawBytesSize = sizeof(kRawBytes);
static const int kFrameSize = 32;
- static const int kNumberOfLocals = 20;
CcTest::InitializeVM();
Isolate* isolate = CcTest::i_isolate();
@@ -581,13 +580,11 @@ TEST(BytecodeArray) {
// Allocate and initialize BytecodeArray
Handle<BytecodeArray> array =
- factory->NewBytecodeArray(kRawBytesSize, kRawBytes);
-
- array->set_frame_size(kFrameSize);
- array->set_number_of_locals(kNumberOfLocals);
+ factory->NewBytecodeArray(kRawBytesSize, kRawBytes, kFrameSize);
CHECK(array->IsBytecodeArray());
CHECK_EQ(array->length(), (int)sizeof(kRawBytes));
+ CHECK_EQ(array->frame_size(), kFrameSize);
CHECK_LE(array->address(), array->GetFirstBytecodeAddress());
CHECK_GE(array->address() + array->BytecodeArraySize(),
array->GetFirstBytecodeAddress() + array->length());
@@ -601,7 +598,6 @@ TEST(BytecodeArray) {
// BytecodeArray should survive
CHECK_EQ(array->length(), kRawBytesSize);
- CHECK_EQ(array->number_of_locals(), kNumberOfLocals);
CHECK_EQ(array->frame_size(), kFrameSize);
for (int i = 0; i < kRawBytesSize; i++) {
--
--
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/d/optout.
