https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.cc
File src/hydrogen-escape-analysis.cc (right):
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.cc#newcode74
src/hydrogen-escape-analysis.cc:74: for (int i = 0; i <
graph()->blocks()->length(); i++) {
You can start i at allocate_block->id(), since the allocation cannot
dominate blocks that come before.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.cc#newcode99
src/hydrogen-escape-analysis.cc:99: for (int index = 0; index <
number_of_values_; index++) {
You're creating a lot of phis here. You only need to create a phi if
this block is a loop header, or if the predecessors do not have the same
HValue for a tracked field.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.cc#newcode117
src/hydrogen-escape-analysis.cc:117: int index = load->access().offset()
/ kPointerSize;
Are you only tracking in-object properties? Otherwise, you need to check
the in-objectness of the access as well.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.cc#newcode189
src/hydrogen-escape-analysis.cc:189: SetStateAt(block, state);
You should forward-propagate the block state to all successors of this
block; then you can delete (or even reuse) the block state.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.cc#newcode206
src/hydrogen-escape-analysis.cc:206: for (int index = 0; index <
number_of_values_; index++) {
This is somewhat brittle because the phi input indexes must match up
with the predecessor indexes, and since your edges are coming off a
stack, I'm really surprised you were so lucky. This needs to be more
robust somehow.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.h
File src/hydrogen-escape-analysis.h (right):
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.h#newcode61
src/hydrogen-escape-analysis.h:61: HCapturedObject*
NewState(HInstruction* previous) {
Not sure why these New* methods are in the header. They seem pretty
complicated, too.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-escape-analysis.h#newcode146
src/hydrogen-escape-analysis.h:146: BitVector loops_affected_by_stores_;
As I understand it, you were going to track which loops had stores in a
later CL. If so, you can just leave this guy out for now.
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen-instructions.h#newcode3451
src/hydrogen-instructions.h:3451: ZoneList<HValue*> values_;
How is this values_ array indexed? Does it store only in-object
properties and is indexed by field index?
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/21055011/diff/3001/src/hydrogen.cc#newcode2952
src/hydrogen.cc:2952:
whitespace change
https://codereview.chromium.org/21055011/diff/3001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://codereview.chromium.org/21055011/diff/3001/src/ia32/lithium-codegen-ia32.cc#newcode785
src/ia32/lithium-codegen-ia32.cc:785: if
(environment->ObjectIsDuplicateAt(object_index)) {
ObjectDuplicate means...object reference?
https://codereview.chromium.org/21055011/diff/3001/src/lithium.h
File src/lithium.h (right):
https://codereview.chromium.org/21055011/diff/3001/src/lithium.h#newcode624
src/lithium.h:624: static LOperand* materialization_marker() { return
NULL; }
NULL seems like it could get mixed up sometime. Maybe -1?
https://codereview.chromium.org/21055011/
--
--
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.
