http://codereview.chromium.org/669155/diff/32/1032
File src/data-flow.cc (right):
http://codereview.chromium.org/669155/diff/32/1032#newcode714
src/data-flow.cc:714: Visit(stmt->condition());
On 2010/03/08 17:26:47, fschneider wrote:
I could write:
result.Union(av_);
av_.Clear();
I don't think you'd write that. You mean:
av_.Union(result)
My comment is that you *should* write that. In fact you might even have
void ProcessExpression(Expression* expr) {
BitVector saved(num_vars);
saved.CopyFrom(av_);
av_.Clear();
Visit(expr);
av_.Union(saved);
}
void VisitIfStatement(IfStatement stmt) {
ProcessExpression(stmt->condition());
Visit(stmt->then_statement());
Visit(stmt->else_statement());
}
So I only do union/clear before
visiting expressions (where I also assert that av_ is empty) and in
for-loops
where I need the result of from the loop body.
My suggestion was not to do it more than was needed. My suggestion was
to do it *exactly* where needed.
http://codereview.chromium.org/669155/diff/32/1032#newcode782
src/data-flow.cc:782: Visit(stmt->cond());
On 2010/03/08 17:26:47, fschneider wrote:
On 2010/03/08 10:55:32, Kevin Millikin wrote:
> Union after this Visit?
As above no need before visiting statements.
There is also no need to have the accumulator reset for the entire loop.
http://codereview.chromium.org/669155/diff/32/1032#newcode801
src/data-flow.cc:801: av_.Clear();
Here is another place where the implementation seems too complicated.
void VisitForStatement(ForStatement* stmt) {
BitVector saved(num_vars_);
if (stmt->init() != NULL) Visit(stmt->init());
if (stmt->cond() != NULL) {
// ProcessExpression
saved.CopyFrom(av_);
av_.Clear();
Visit(stmt->cond());
av_.Union(saved);
}
if (stmt->next() != NULL) Visit(stmt->next());
saved.CopyFrom(av_);
av_.Clear();
Visit(stmt->body());
....
http://codereview.chromium.org/669155
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
