Much better to do this at graph construction. Comments below.
http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5793 src/hydrogen.cc:5793: void HGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr, I'd prefer to use more specific types here. The casts below will always succeed in this change, but it needs nonlocal reasoning to see that (and relies on the assertion to preserve it in the face of changes to the helper functions). void HGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr, HTypeof* typeof_expr, Handle<String> blah) { ... http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5797 src/hydrogen.cc:5797: Handle<String> blah = Handle<String>::cast(HConstant::cast(check)->handle()); There must be a better name. http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5801 src/hydrogen.cc:5801: typeof_expr->DeleteAndReplaceWith(instr); I don't think this is right. instr is a control instruction, and I think we're relying on having no uses for those. It should be the case that there is no use of typeof_expr, so you can replace it with NULL. An alternative that I like a bit better, because it covers more cases, is to make HTypeof::Canonicalize() return NULL if there are no uses (provided we convince ourselves that typeof cannot have side effects). http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5829 src/hydrogen.cc:5829: *typeof_expr = left; If you take my suggestion above, this is where you'd cast: *typeof_expr = HTypeof::cast(left); *check = Handle<String>::cast(HConstant::cast(right)->handle()); http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5891 src/hydrogen.cc:5891: HValue* typeof_expr = NULL; And HTypeof* typeof_expr = NULL; Handle<String> check; http://codereview.chromium.org/8242002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
