feedback addressed, landing...
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);
On 2014/05/09 13:16:49, mvstanton wrote:
It would be good if the single arg case avoided a frame too, does it
run into
problems to add it there?
Yes. Currently asserts, but I will fix this in a subsequent CL.
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_;
On 2014/05/09 13:16:49, mvstanton wrote:
Could you add a comment that the default Representation is Tagged() if
the
register_param_representations_ pointer is left in default (NULL)
state?
Done.
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) \
On 2014/05/09 13:16:49, mvstanton wrote:
s/copyied/copied/
Done.
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.
On 2014/05/09 13:16:49, mvstanton wrote:
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).
Done.
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);
On 2014/05/09 13:16:49, mvstanton wrote:
nit: I'd rather see a { } around the assignment, the one line style is
fine for
function call or "return;" or things like that.
Done.
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc#newcode2474
src/hydrogen.cc:2474: if (to == NULL) to =
AddLoadFixedArrayLength(elements);
On 2014/05/09 13:16:49, mvstanton wrote:
nit: I'd rather see a { } around the assignment, the one line style is
fine for
function call or "return;" or things like that.
Done.
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc#newcode2562
src/hydrogen.cc:2562: if (capacity == NULL) capacity =
AddLoadFixedArrayLength(to_elements);
On 2014/05/09 13:16:49, mvstanton wrote:
nit: same here.
Done.
https://codereview.chromium.org/272513004/diff/110001/src/hydrogen.cc#newcode2712
src/hydrogen.cc:2712:
boilerplate_elements->SetFlag(HValue::kCantBeReplaced);
On 2014/05/09 13:16:49, mvstanton wrote:
Can you comment the motivation for using kCantBeReplaced here? I think
this is
to avoid a load from spill slot, right?
Done.
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.
