LGTM, but I still think the comments are murky about what's going on in the switch statements.
BTW, the reason to factor out (at least) the constant folding part of operations is that they are (hopefully) not platform-specific and their implementations can be shared. I guess that can come as a separate refactoring change. http://codereview.chromium.org/21211/diff/5/2002 File src/codegen-ia32.cc (right): http://codereview.chromium.org/21211/diff/5/2002#newcode2024 Line 2024: // The test is compiled, and linked with the previous and next tests. This comment still seems wrong. It is a ways away from the line where the test is compiled, and the test is only compiled conditionally, and it doesn't have anything to do with linking to the next test. What I think the reader cares about is why this code is conditionally guarded, because knowing that depends on reasoning about the previous loop iterations. Try something like "Unless this is the first (non-default) case, some previous case was unconditionally true, or the immediately previous (non-default) case had its body compiled, there is a dangling jump to the test." http://codereview.chromium.org/21211/diff/5/2002#newcode2032 Line 2032: // If the switch value is a constant, we may have unconditional I wouldn't be so specific about implementation details about why we might have unconditional control flow (ie, don't say "constant" because we might base decisions on known type-tags for non-constants or value-range propagation or something, and it isn't really relevant here anyway). The important thing that is difficult for the reader to reason out seems to me to be in which cases the frame can be valid (first non-default case or the immediately previous test that was compiled was not unconditionally true) and which cases invalid (the previous compiled test was unconditionally true). http://codereview.chromium.org/21211 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
