LGTM, only nits.
https://codereview.chromium.org/144423010/diff/1/src/hydrogen-gvn.h
File src/hydrogen-gvn.h (right):
https://codereview.chromium.org/144423010/diff/1/src/hydrogen-gvn.h#newcode39
src/hydrogen-gvn.h:39: class SideEffects V8_FINAL {
A comment that this is merely a GVNFlagSet plus "special" dynamic side
effects and why we need that would be nice.
https://codereview.chromium.org/144423010/diff/1/src/hydrogen-gvn.h#newcode77
src/hydrogen-gvn.h:77: class SideEffectsScope V8_FINAL BASE_EMBEDDED {
Can we have a few comments above this class to explain what it is used
for? Also the name is misleading IMHO, as it sounds like several of
these scopes are stacked (which they aren't). It is only used to track
finer-grained side-effects for loads and stores. How about calling it
SideEffectsMap (my favorite), SideEffectsCache or SideEffectsTracker
instead?
https://codereview.chromium.org/144423010/
--
--
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.
