Thanks. I addressed the comments.
I also added marking trivial receivers of property stores in the analysis,
but
do not make use of it yet in the code generator.
http://codereview.chromium.org/669155/diff/25001/11009
File src/ast.h (right):
http://codereview.chromium.org/669155/diff/25001/11009#newcode128
src/ast.h:128: virtual Block* AsBlock() { return NULL; }
On 2010/03/10 14:37:06, Kevin Millikin wrote:
Is this used?
No. Removed.
http://codereview.chromium.org/669155/diff/25001/11009#newcode144
src/ast.h:144: virtual Assignment* AsSimpleAssignment() { return NULL; }
On 2010/03/10 14:37:06, Kevin Millikin wrote:
Maybe this should be a function on Statement instead of AstNode, so
it's clearly
not AsAssignment().
Done. As discussed now a Statement function.
http://codereview.chromium.org/669155/diff/25001/11009#newcode339
src/ast.h:339: bool IsSimpleInitializerBlock();
On 2010/03/10 14:37:06, Kevin Millikin wrote:
Is this needed?
No. Removed.
http://codereview.chromium.org/669155/diff/25001/11009#newcode531
src/ast.h:531: virtual CountOperation* AsCountOperation() {
On 2010/03/10 14:37:06, Kevin Millikin wrote:
Maybe this should be something like
Statement::StatementAsCountOperation.
Done.
http://codereview.chromium.org/669155/diff/25001/11002
File src/data-flow.cc (right):
http://codereview.chromium.org/669155/diff/25001/11002#newcode1123
src/data-flow.cc:1123: VariableProxy* proxy =
init->target()->AsVariableProxy();
On 2010/03/10 14:37:06, Kevin Millikin wrote:
I think this function should be rewritten to use Variable* instead of
VariableProxy* where possible. It seems more direct that way.
Done. (also for the other parts dealing with the loop variable in this
function)
http://codereview.chromium.org/669155/diff/25001/11002#newcode1134
src/data-flow.cc:1134: // The condition must be a comparison.
On 2010/03/10 14:37:06, Kevin Millikin wrote:
This reads a bit better if you write:
CompareOperation* cond = stmt->cond()->AsCompareOperation();
if (cond == NULL) return NULL;
if (cond->op() ...) ...
Done.
http://codereview.chromium.org/669155/diff/25001/11002#newcode1153
src/data-flow.cc:1153: // The update expression must be a count
operation.
On 2010/03/10 14:37:06, Kevin Millikin wrote:
This reads a bit better if you write:
CountOperation* update = stmt->next()->AsCountOperation();
if (update == NULL) return NULL;
...
Done.
http://codereview.chromium.org/669155/diff/25001/11002#newcode1193
src/data-flow.cc:1193: if (var != NULL &&
On 2010/03/10 14:37:06, Kevin Millikin wrote:
if (var != NULL && var->IsStackAllocated()) ...
Done. Also in some other places.
http://codereview.chromium.org/669155/diff/25001/11002#newcode1262
src/data-flow.cc:1262: BitVector result(av_);
On 2010/03/10 14:37:06, Kevin Millikin wrote:
Maybe call this something other than result, since it's not the result
that's
actually returned. "accumulator"?
Agree. It is used differently for different ast node types. I changed it
to either "result" where I use to as accumulator for the final result of
a node or "saved_av" where just save the state before call to visit.
http://codereview.chromium.org/669155/diff/25001/11002#newcode1294
src/data-flow.cc:1294: if (stmt->cond() != NULL) {
On 2010/03/10 14:37:06, Kevin Millikin wrote:
Why does this one have braces? Too long for one line?
Done.
http://codereview.chromium.org/669155/diff/25001/11002#newcode1302
src/data-flow.cc:1302: BitVector result(av_);
On 2010/03/10 14:37:06, Kevin Millikin wrote:
This one isn't the result that's returned either. Maybe "saved"?
Done.
http://codereview.chromium.org/669155/diff/25001/11002#newcode1360
src/data-flow.cc:1360: BitVector result(av_);
On 2010/03/10 14:37:06, Kevin Millikin wrote:
accumulator?
Done.
http://codereview.chromium.org/669155/diff/25001/11011
File src/data-flow.h (right):
http://codereview.chromium.org/669155/diff/25001/11011#newcode509
src/data-flow.h:509:
On 2010/03/10 14:37:06, Kevin Millikin wrote:
It looks like an extra blank line after private:.
Done.
http://codereview.chromium.org/669155
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
