NOT LGTM.
https://codereview.chromium.org/104843002/diff/1/src/hydrogen-check-elimination.cc
File src/hydrogen-check-elimination.cc (right):
https://codereview.chromium.org/104843002/diff/1/src/hydrogen-check-elimination.cc#newcode292
src/hydrogen-check-elimination.cc:292:
removed->DeleteAndReplaceWith(removed->ActualValue());
This transformation is completely incorrect. First, you haven't even
checked whether the two CheckMaps refer to the same object. Second, you
haven't checked the relationship between the CheckMaps either (does one
entail another?). Third, an object's map can change in between these
instructions. Fourth, removing the check heap object in between is only
legal if the CheckMaps refer to the the same object. Fourth, I have no
idea why you choose to match this pattern at the
CompareObjectEqAndBranch instruction.
But, you did bring one optimization opportunity to mind. We can remove
some CheckHeapObject instructions for objects whose maps may no longer
be known, but they are still known to be heap objects.
https://codereview.chromium.org/104843002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
