A few comments below. It basically LGTM.
http://codereview.chromium.org/545007/diff/16001/17008 File src/frame-element.h (right): http://codereview.chromium.org/545007/diff/16001/17008#newcode240 src/frame-element.h:240: class CopiedField: public BitField<uint32_t, 3, 1> {}; Could you change the type to "bool" on this field and the next? http://codereview.chromium.org/545007/diff/16001/17002 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/545007/diff/16001/17002#newcode1005 src/ia32/codegen-ia32.cc:1005: bool left_is_non_smi = left.is_constant() && !left.handle()->IsSmi(); This should probably be renamed to left_is_non_smi_constant. http://codereview.chromium.org/545007/diff/16001/17002#newcode1059 src/ia32/codegen-ia32.cc:1059: if (only_numbers) All on one line like if (only_numbers) info = NumberInfo::kNumber; or else braces like if (only_numbers) { info = NumberInfo::kNumber; } http://codereview.chromium.org/545007/diff/16001/17002#newcode7573 src/ia32/codegen-ia32.cc:7573: if (only_numbers_in_stub_) { You could also optimize the non-sse version, no? http://codereview.chromium.org/545007/diff/16001/17002#newcode7604 src/ia32/codegen-ia32.cc:7604: GenerateHeapResultAllocation(masm, &call_runtime); In the only numbers case, you can do the allocation before the XMM register load (because the xmm load can't fail). I think you would have to preserve eax for the heap allocation or (better yet) pass a desired result register to GenerateHeapResultAllocation. http://codereview.chromium.org/545007/diff/16001/17002#newcode8079 src/ia32/codegen-ia32.cc:8079: void FloatingPointHelper::LoadSSE2Operands(MacroAssembler* masm) { If this takes a scratch register you can use the scratch register for smi untagging, then you don't have to retag. http://codereview.chromium.org/545007/diff/21001/22007 File src/jump-target.cc (right): http://codereview.chromium.org/545007/diff/21001/22007#newcode109 src/jump-target.cc:109: element->set_number_info(NumberInfo::Combine(element->number_info(), I'd like a short comment here to point out that we're changing the number info on one of the incoming frames but that it's safe because we only use that frame for emitting merge code. http://codereview.chromium.org/545007/diff/21001/22006 File src/number-info.h (right): http://codereview.chromium.org/545007/diff/21001/22006#newcode28 src/number-info.h:28: #ifndef V8_NUMBERINFO_H_ The include guard should be V8_NUMBER_INFO_H_ http://codereview.chromium.org/545007/diff/21001/22006#newcode54 src/number-info.h:54: #endif // V8_kNumberTypeINFO_H_ Too much search and replace. http://codereview.chromium.org/545007 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
