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/09 06:03:47, Kevin Millikin wrote:
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.
Oh ok. That was a misunderstanding on my side. You meant moving
av_.Union(result)
and not inserting a new one.
But ProcessExpression is definitely a good idea. The only places where
this abstraction does not fit in are ast nodes that contains two or more
sub-expressions (e.g. for binary operation:
ProcessExpression(left);
ProcessExpression(right);
would too many copies. Instead I do
Visit(left);
ProcessExpression(right);
Also for call arguments, array literals, etc. I'll explicitly allocate a
temporary result bit-vector for those nodes to avoid excessive copying.
http://codereview.chromium.org/669155/diff/32/1032#newcode782
src/data-flow.cc:782: Visit(stmt->cond());
On 2010/03/09 06:03:47, Kevin Millikin wrote:
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.
Done. I'm calling ProcessExpression for the condition and Visit for the
body.
http://codereview.chromium.org/669155/diff/32/1032#newcode801
src/data-flow.cc:801: av_.Clear();
On 2010/03/09 06:03:47, Kevin Millikin wrote:
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());
....
Done. Also using ProcessExpression here.
http://codereview.chromium.org/669155
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
