I don't really like our const semantics, but I think we should find a way to support a bit more of it than this change does.
http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode2956 src/hydrogen.cc:2956: BAILOUT("unsupported reference to const variable"); ast_context()->ReturnValue(graph()->GetConstantUndefined()); http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode3377 src/hydrogen.cc:3377: if (var->mode() == Variable::CONST) BAILOUT("unsupported const assignment"); Instead of statically bailing out, how about visiting the operation and then guarding the assignment with something like if (var->mode() != Variable::CONST) .... http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode3484 src/hydrogen.cc:3484: BAILOUT("unsupported const assignment"); Instead of statically bailing out for non-INIT_CONST assignments to CONST variables, you could just ignore them. http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode3488 src/hydrogen.cc:3488: HValue* old_value = environment()->Lookup(var); It's a little indirect to add this use and detect it in a postprocessing pass. It seems more direct (though a bit more implementation effort) to keep track of a loop nesting depth and bailout if non-zero. I'm also not thrilled with InitConst as an instruction. It's not initializing anything and it's odd that it refers to the old value and not the new. It's more like UseValue or something. http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode4565 src/hydrogen.cc:4565: if (var->mode() == Variable::CONST) BAILOUT("unsupported count operation"); Same comment. http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode4920 src/hydrogen.cc:4920: // functions, variables with slot type LOOKUP Fix the comment so it's still a sentence (add "and" and a period). http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode4929 src/hydrogen.cc:4929: if (var->mode() == Variable::CONST) { I don't think you should change the test against decl->mode() to one against var->mode(). Declaration::mode() can only be VAR or CONST, while Variable::mode() could be all kinds of other stuff (DYNAMIC, DYNAMIC_GLOBAL, etc). I'm worried that things will break as we support more stuff if we don't handle constants more or less like the baseline compiler. http://codereview.chromium.org/6026006/diff/15001/src/hydrogen.cc#newcode4930 src/hydrogen.cc:4930: environment()->Bind(var, graph()->GetConstantHole()); I'm surprised you don't have to check that the variable is stack allocated here. http://codereview.chromium.org/6026006/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
