Reviewers: rmcilroy,
Message:
Thanks! All done/ack'ed.
https://codereview.chromium.org/1230753004/diff/40001/src/factory.cc
File src/factory.cc (right):
https://codereview.chromium.org/1230753004/diff/40001/src/factory.cc#newcode880
src/factory.cc:880: length, start, pretenure),
On 2015/07/15 13:33:37, rmcilroy wrote:
strange indentation here - did you run "git cl format"?
Done. git cl bogon.
https://codereview.chromium.org/1230753004/diff/40001/src/factory.h
File src/factory.h (right):
https://codereview.chromium.org/1230753004/diff/40001/src/factory.h#newcode286
src/factory.h:286: Handle<BytecodeArray> NewBytecodeArray(int length,
const byte* start,
On 2015/07/15 13:33:37, rmcilroy wrote:
nit - /s/start/raw_bytecodes.
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/factory.h#newcode287
src/factory.h:287: PretenureFlag pretenure = NOT_TENURED);
On 2015/07/15 13:33:37, rmcilroy wrote:
Let's remove the pretenure flag and just always tenure the bytecode -
it is
referenced by the SharedFunctionInfo which is also pretenured
(allocated in the
old space) and should always live long enough that it would always get
promoted
anyway.
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):
https://codereview.chromium.org/1230753004/diff/40001/src/flag-definitions.h#newcode280
src/flag-definitions.h:280: DEFINE_BOOL(ignition, false, "use ignition
interpreter")
On 2015/07/15 13:33:37, rmcilroy wrote:
I'll change this in my CL so it should disappear once you've rebased.
Acknowledged.
https://codereview.chromium.org/1230753004/diff/40001/src/heap/heap.h
File src/heap/heap.h (right):
https://codereview.chromium.org/1230753004/diff/40001/src/heap/heap.h#newcode1675
src/heap/heap.h:1675: // Allocates a bytecode array with given contents
On 2015/07/15 13:33:38, rmcilroy wrote:
nit - fullstop at the end of the comment.
Done and renamed 'start' to 'raw_bytecodes'.
https://codereview.chromium.org/1230753004/diff/40001/src/interpreter/bytecode-emitter.cc
File src/interpreter/bytecode-emitter.cc (right):
https://codereview.chromium.org/1230753004/diff/40001/src/interpreter/bytecode-emitter.cc#newcode28
src/interpreter/bytecode-emitter.cc:28: void
BytecodeEmitter::VisitBlock(Block* node) {}
On 2015/07/15 13:33:38, rmcilroy wrote:
Could you please add UNIMPLEMENTED(); to each of these blocks so that
we know
when we hit one of the unimplemented AST blocks.
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/interpreter/bytecode-emitter.h
File src/interpreter/bytecode-emitter.h (right):
https://codereview.chromium.org/1230753004/diff/40001/src/interpreter/bytecode-emitter.h#newcode15
src/interpreter/bytecode-emitter.h:15: class BytecodeEmitter : public
AstVisitor {
On 2015/07/15 13:33:38, rmcilroy wrote:
Let's move this off to a separate CL and keep this one just for adding
the
BytecodeArray object.
Acknowledged.
https://codereview.chromium.org/1230753004/diff/40001/src/interpreter/interpreter.h
File src/interpreter/interpreter.h (right):
https://codereview.chromium.org/1230753004/diff/40001/src/interpreter/interpreter.h#newcode66
src/interpreter/interpreter.h:66: #endif //
V8_INTERPRETER_INTERPRETER_H_
On 2015/07/15 13:33:38, rmcilroy wrote:
good catch, I'll fix this, thanks :).
Acknowledged.
https://codereview.chromium.org/1230753004/diff/40001/src/objects-printer.cc
File src/objects-printer.cc (right):
https://codereview.chromium.org/1230753004/diff/40001/src/objects-printer.cc#newcode213
src/objects-printer.cc:213: << static_cast<interpreter::Bytecode>(bc);
On 2015/07/15 13:33:38, rmcilroy wrote:
How about we move this to a BytecodeArray::Dissassemble() function so
it can be
used from elsewhere if necessary.
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects-printer.cc#newcode783
src/objects-printer.cc:783: // TODO(oth): implement real bytecode
printer
On 2015/07/15 13:33:38, rmcilroy wrote:
Actually I think emitting the pointer here is fine for the
ShareFunctionInfo
object. I would remove the TODO. (someone can always call
BytecodeArrayPrint on
the pointer to get the bytecode pretty-printed.
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4267
src/objects.h:4267: // BytecodeArray represents a sequence interpreter
bytecodes.
On 2015/07/15 13:33:38, rmcilroy wrote:
/s/sequence interpreter/sequence of interpreter
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4277
src/objects.h:4277: // have.
On 2015/07/15 13:33:38, rmcilroy wrote:
Copy and pasted ByteArray comment here?
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4278
src/objects.h:4278: static int LengthFor(int size_in_bytes) {
On 2015/07/15 13:33:38, rmcilroy wrote:
Do we need this function? I think it's only necessary for ByteArray
not
BytecodeArray
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4285
src/objects.h:4285: inline Address GetDataStartAddress() {
On 2015/07/15 13:33:38, rmcilroy wrote:
nit - GetFirstBytecodeAddress() ?
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4290
src/objects.h:4290: static inline BytecodeArray*
FromDataStartAddress(Address address);
On 2015/07/15 13:33:38, rmcilroy wrote:
Looks like this isn't used, let's drop it until it's necessary.
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4295
src/objects.h:4295: inline int BytecodeArraySize() { return
SizeFor(this->length()); }
On 2015/07/15 13:33:38, rmcilroy wrote:
Same for this (also remove comment).
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode4303
src/objects.h:4303: static const int kMaxSize = 512 * MB;
On 2015/07/15 13:33:38, rmcilroy wrote:
nit - /s/512 * MB/ByteArray::kMaxSize/
Done.
https://codereview.chromium.org/1230753004/diff/40001/src/objects.h#newcode6718
src/objects.h:6718: // [bytecode_array]: Interpreter byte codes
On 2015/07/15 13:33:38, rmcilroy wrote:
nit - /s/byte codes/bytecodes ?
Done.
Description:
[Interpreter] Add BytecodeArray class and add to SharedFunctionInfo.
BUG=v8:4280
LOG=N
Please review this at https://codereview.chromium.org/1230753004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+127, -17 lines):
M include/v8.h
M src/factory.h
M src/factory.cc
M src/heap-snapshot-generator.cc
M src/heap/heap.h
M src/heap/heap.cc
M src/interpreter/interpreter.cc
M src/objects.h
M src/objects.cc
M src/objects-debug.cc
M src/objects-inl.h
M src/objects-printer.cc
--
--
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.
