LGTM with comments.
https://codereview.chromium.org/23866016/diff/1/src/hydrogen-check-elimination.cc
File src/hydrogen-check-elimination.cc (right):
https://codereview.chromium.org/23866016/diff/1/src/hydrogen-check-elimination.cc#newcode53
src/hydrogen-check-elimination.cc:53: HValue* object =
instr->ActualValue();
Even though HCheckMaps serves as an informative definition of it's
target object, this should always make sure to use the target object. I
think the following is safer:
HValue* object = instr->value()->ActualValue();
https://codereview.chromium.org/23866016/diff/1/src/hydrogen-check-elimination.cc#newcode95
src/hydrogen-check-elimination.cc:95: // and the map is known to be
exactly the given constant.
Technically this only holds if the load has happened before any
subsequent map-changes to the object. We might get lucky because we
don't generate map-loads which are used over longer live-ranges. This
seems fragile.
https://codereview.chromium.org/23866016/diff/1/src/hydrogen-check-elimination.cc#newcode235
src/hydrogen-check-elimination.cc:235: for (int i = 0; i <
kMaxTrackedObjects; i++) {
Can we ASSERT(Find(object) < 0) here?
https://codereview.chromium.org/23866016/
--
--
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.
