It already looks pretty good - the only issue I have is that it is maybe too
restrictive (see comments below). How hard would it be to combine it with a
more
generic fall-back?
http://codereview.chromium.org/5812005/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/5812005/diff/1/src/hydrogen.cc#newcode2656
src/hydrogen.cc:2656: last_false_block->Finish(new HDeoptimize);
It is safe to deoptimize after the last smi-compare, but it seems overly
restricting, if there are reachable smi-compares afterwards. e.g.
switch(x) {
case 0: A(); break;
case 1: B(); break;
default: C(); break;
}
If the inputs are only 0 and 2 we would deoptimize after the first
smi-compare with 0, and not optimize the default-clause.
http://codereview.chromium.org/5812005/diff/1/src/hydrogen.cc#newcode2681
src/hydrogen.cc:2681:
compare_graphs.at(i)->exit_block()->end()->FirstSuccessor();
My suggestion would be to insert HDeoptimize in the empty-block here for
all non-smi-compares. It's not as compact as your approach, but it
handles all cases.
Maybe you can fall back to the more generic solution if the smi-compares
are not a prefix-set.
http://codereview.chromium.org/5812005/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
