LGTM. I'm curious about the performance improvements.
However we should keep an eye on the increased allocation and compile time. If this causes a big degradation, we should consider optimizing the underlying data
structures in gvn. http://codereview.chromium.org/6690006/diff/1/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): http://codereview.chromium.org/6690006/diff/1/src/hydrogen-instructions.h#newcode2950 src/hydrogen-instructions.h:2950: return (offset_ << 1) || (is_in_object_ ? 1 : 0); Shouldn't be this bitwise-or "|"? http://codereview.chromium.org/6690006/diff/1/src/hydrogen-instructions.h#newcode3123 src/hydrogen-instructions.h:3123: return (offset_ << 1) || (is_in_object_ ? 1 : 0); -> use "|" here as well. http://codereview.chromium.org/6690006/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/6690006/diff/1/src/hydrogen.cc#newcode1010 src/hydrogen.cc:1010: HEffectSet() : parameters_(0) { Since Initialize() also initializes paramters_, there is no need to mention it in the initializer list. http://codereview.chromium.org/6690006/diff/1/src/hydrogen.cc#newcode1014 src/hydrogen.cc:1014: explicit HEffectSet(int flags) : parameters_(0) { Same here. http://codereview.chromium.org/6690006/diff/1/src/hydrogen.cc#newcode1039 src/hydrogen.cc:1039: int flag = i << 1; You could move this line to after the continue. http://codereview.chromium.org/6690006/diff/1/src/hydrogen.cc#newcode1102 src/hydrogen.cc:1102: if (parameters_.length() <= flag_index) { I find (flag_index >= parameters_.length()) better. http://codereview.chromium.org/6690006/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
