lgtm, modulo nits and request-for-comments.
https://codereview.chromium.org/272513004/diff/110001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/272513004/diff/110001/src/code-stubs-hydrogen.cc#newcode649
src/code-stubs-hydrogen.cc:649: result =
BuildArraySingleArgumentConstructor(&array_builder);
It would be good if the single arg case avoided a frame too, does it run
into problems to add it there?
https://codereview.chromium.org/272513004/diff/110001/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/272513004/diff/110001/src/code-stubs.h#newcode281
src/code-stubs.h:281: Representation* register_param_representations_;
Could you add a comment that the default Representation is Tagged() if
the register_param_representations_ pointer is left in default (NULL)
state?
https://codereview.chromium.org/272513004/diff/110001/src/counters.h
File src/counters.h (right):
https://codereview.chromium.org/272513004/diff/110001/src/counters.h#newcode382
src/counters.h:382: SC(inlined_copyied_elements,
V8.InlinedCopiedElements) \
s/copyied/copied/
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen-instructions.h#newcode621
src/hydrogen-instructions.h:621: // Indicates an instruction shouldn't
be replaced by optimization.
Or alternatively, instead of commenting the usage of kCantBeReplaced in
hydrogen.cc, expand on this comment a bit to indicate why you might want
to use this (reload is cheaper than spilling, say).
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc#newcode2474
src/hydrogen.cc:2474: if (to == NULL) to =
AddLoadFixedArrayLength(elements);
nit: I'd rather see a { } around the assignment, the one line style is
fine for function call or "return;" or things like that.
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc#newcode2562
src/hydrogen.cc:2562: if (capacity == NULL) capacity =
AddLoadFixedArrayLength(to_elements);
nit: same here.
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc#newcode2712
src/hydrogen.cc:2712:
boilerplate_elements->SetFlag(HValue::kCantBeReplaced);
Can you comment the motivation for using kCantBeReplaced here? I think
this is to avoid a load from spill slot, right?
https://codereview.chromium.org/272513004/diff/110001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/272513004/diff/110001/src/ia32/full-codegen-ia32.cc#newcode1732
src/ia32/full-codegen-ia32.cc:1732: if (expr->depth() > 1) {
Awesome!
https://codereview.chromium.org/272513004/diff/110001/src/lithium.cc
File src/lithium.cc (right):
https://codereview.chromium.org/272513004/diff/110001/src/lithium.cc#newcode453
src/lithium.cc:453: generator.NeedsEagerFrame()));
+1.
https://codereview.chromium.org/272513004/
--
--
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.
