On 2013/09/26 09:09:55, Michael Starzinger wrote:
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();


Done.


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.


I'm now doing load elimination for the map field based on the tracked maps for the objects. I've fixed the HCheckValue to Canonicalize(), because the checked
value might become a constant.


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?

I assert it in Kill() now.



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.

Reply via email to