Hi guys,
Addressed Michi's comments, did ports, fixed a bug.
Thanks much,
--Michael
https://codereview.chromium.org/1321993004/diff/40001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1321993004/diff/40001/src/ast.h#newcode1429
src/ast.h:1429: int ic_slot_count_;
On 2015/09/07 08:31:13, Michael Starzinger wrote:
As discussed offline, please move these fields either to before the
"kind" or
after the boolean flags, so that a compiler can put "kind" and
booleans into one
word.
Also as discussed offline, lets make sure the memory overhead of these
two new
fields are acceptable by measuring/calculating AST memory footprint on
common
workloads.
Excellent. I've reduced the two new words to one, and am trying to get a
picture of the overall size increase.
https://codereview.chromium.org/1321993004/diff/40001/src/compiler/ast-graph-builder.cc
File src/compiler/ast-graph-builder.cc (right):
https://codereview.chromium.org/1321993004/diff/40001/src/compiler/ast-graph-builder.cc#newcode1705
src/compiler/ast-graph-builder.cc:1705: void
AstGraphBuilder::BuildAccessor(Node* home_object,
On 2015/09/07 08:31:13, Michael Starzinger wrote:
As discussed offline, this is not really a builder, but a visitation
method.
Let's call it "VisitObjectLiteralAccessor" instead. The reasoning
behind this is
explained in the comments in the ast-graph-builder.h header file.
Right on, done.
https://codereview.chromium.org/1321993004/diff/40001/src/compiler/ast-graph-builder.h
File src/compiler/ast-graph-builder.h (right):
https://codereview.chromium.org/1321993004/diff/40001/src/compiler/ast-graph-builder.h#newcode256
src/compiler/ast-graph-builder.h:256: void BuildAccessor(Node*
home_object, ObjectLiteralProperty* property);
On 2015/09/07 08:31:13, Michael Starzinger wrote:
nit: After addressing the comment about naming, this should go down
into the
visitation section (preferably with a comment "Dispatched from
VisitObjectLiteral").
Done.
https://codereview.chromium.org/1321993004/diff/60001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1321993004/diff/60001/src/ast.h#newcode1510
src/ast.h:1510: // Expression* setter;
On 2015/09/07 11:11:50, Michael Starzinger wrote:
nit: Still look like a left-over.
Done.
https://codereview.chromium.org/1321993004/diff/60001/src/compiler/ast-graph-builder.cc
File src/compiler/ast-graph-builder.cc (right):
https://codereview.chromium.org/1321993004/diff/60001/src/compiler/ast-graph-builder.cc#newcode1900
src/compiler/ast-graph-builder.cc:1900: if (property == NULL) {
On 2015/09/07 11:11:50, Michael Starzinger wrote:
nit: s/NULL/nullptr/
Done.
https://codereview.chromium.org/1321993004/diff/60001/src/compiler/ast-graph-builder.cc#newcode1901
src/compiler/ast-graph-builder.cc:1901: VisitForValueOrNull(NULL);
On 2015/09/07 11:11:50, Michael Starzinger wrote:
nit: s/NULL/nullptr/
Done.
https://codereview.chromium.org/1321993004/diff/60001/src/compiler/ast-graph-builder.cc#newcode1904
src/compiler/ast-graph-builder.cc:1904: VisitForValueOrNull(value);
On 2015/09/07 11:11:50, Michael Starzinger wrote:
nit: Just inline "VisitForValueOrNull(property->value())" here.
Heh, done. :)
https://codereview.chromium.org/1321993004/
--
--
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.
