not lgtm.
https://codereview.chromium.org/22915007/diff/10001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/22915007/diff/10001/src/hydrogen-instructions.cc#newcode3249
src/hydrogen-instructions.cc:3249: if
(AllocationSite::CanTrack(dominator_allocate_instr->instance_type()) &&
This doesn't work properly for nested JSArray objects that are all
folded together. Imagine the following sequence ...
v1 = HAllocate(JS_OBJECT_TYPE);
v2 = HAllocate(JS_ARRAY_TYPE + ALLOCATION_SITE);
v3 = HAllocate(WHATEVER);
Here "v1" isn't tracked with an allocation site, so this path never
triggers. On the other hand "v2" is tracked with an allocation site, but
garbage that happens to be at the position of "v3" is not smashed.
https://codereview.chromium.org/22915007/diff/10001/test/mjsunit/allocation-folding.js
File test/mjsunit/allocation-folding.js (right):
https://codereview.chromium.org/22915007/diff/10001/test/mjsunit/allocation-folding.js#newcode59
test/mjsunit/allocation-folding.js:59: result = doubles();
suggestion: If you properly scope each test in a separate function
scope, you could have local variables without accidentally re-declaring
or re-using previous ones.
https://codereview.chromium.org/22915007/diff/10001/test/mjsunit/allocation-folding.js#newcode81
test/mjsunit/allocation-folding.js:81: // Test allocation folding of
over a branch.
nit: s/of//
https://codereview.chromium.org/22915007/
--
--
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/groups/opt_out.
