On 2013/12/04 16:31:59, titzer wrote:
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.
Hi Ben,
The pattern of CompareObjectEqAndBranch is as below for object "=="
operation
t13 CheckHeapObject t4
t14 CheckMaps t4
t15 CheckHeapObject t3
t16 CheckMaps t3
v17 CompareObjectEqAndBranch t16 t14
As Danno said, we don't need to check both. One check for either left or
right
is enough. What I do is to match this pattern and remove one redundant
check to
improve the performance. When CompareObjectEqAndBranch appears in the loop,
I
hope to keep loop invariant object's map check and remove the other, so
that the
remaining map check could be hoisted out of loop. That could bring the
biggest
performance improvement.
The patch set 2 is a refined implementation based on your comments. Could
you
please have a look again?
Thanks
-Weiliang
https://codereview.chromium.org/104843002/
--
--
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.
