Sorry for the delay, but we're getting there. My main concern is still the complexity which may be avoidable, so my comments are about making the code simpler where possible.
http://codereview.chromium.org/8373029/diff/43002/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): http://codereview.chromium.org/8373029/diff/43002/src/hydrogen-instructions.h#newcode2762 src/hydrogen-instructions.h:2762: class HCompareGenericAndBranch: public HTemplateControlInstruction<2, 3> { This should rather be HStringCompareAndBranch. It can't be used for generic compares in general due to deoptimization. http://codereview.chromium.org/8373029/diff/43002/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/8373029/diff/43002/src/hydrogen.cc#newcode2762 src/hydrogen.cc:2762: iterations = iterations * 2; If it does not yield a big improvement, I'd rather generate 1 pass in any case. It seems there are 3 cases: 1. only symbols, 2. only strings, 3. generic other stuff (e.g. oddballs) (1) uses CompareObjectEqAndBranch (2), (3) use StringCompareAndBranch. There would need to be imho a strong performance argument to justify generating 2 passes, because it makes the CFG larger and complicates the graph construction. http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-codegen-ia32.cc#newcode1798 src/ia32/lithium-codegen-ia32.cc:1798: Handle<Code> ic = CompareIC::GetUninitialized(op); Use StringCompareStub instead. StringCompareStub should also be fast for symbols, so maybe it would be fine performance-wise to just do 1 pass of comparisons using this stub. http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-ia32.cc File src/ia32/lithium-ia32.cc (right): http://codereview.chromium.org/8373029/diff/43002/src/ia32/lithium-ia32.cc#newcode1548 src/ia32/lithium-ia32.cc:1548: return AssignEnvironment(MarkAsCall(result, instr)); This looks fishy. We should not need an environment if there is neither eager nor lazy deoptimization. http://codereview.chromium.org/8373029/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
