Addressed comment + fixed lint errors.
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> {};
On 2010/02/15 13:02:09, Kevin Millikin wrote:
Could you change the type to "bool" on this field and the next?
Done.
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();
On 2010/02/15 13:02:09, Kevin Millikin wrote:
This should probably be renamed to left_is_non_smi_constant.
Done.
http://codereview.chromium.org/545007/diff/16001/17002#newcode1059
src/ia32/codegen-ia32.cc:1059: if (only_numbers)
On 2010/02/15 13:02:09, Kevin Millikin wrote:
All on one line like
if (only_numbers) info = NumberInfo::kNumber;
or else braces like
if (only_numbers) {
info = NumberInfo::kNumber;
}
Done.
http://codereview.chromium.org/545007/diff/16001/17002#newcode7573
src/ia32/codegen-ia32.cc:7573: if (only_numbers_in_stub_) {
On 2010/02/15 13:02:09, Kevin Millikin wrote:
You could also optimize the non-sse version, no?
Done. I also added a macro assembler function to emit the debug-code to
avoid code duplication for this check.
http://codereview.chromium.org/545007/diff/16001/17002#newcode7604
src/ia32/codegen-ia32.cc:7604: GenerateHeapResultAllocation(masm,
&call_runtime);
On 2010/02/15 13:02:09, Kevin Millikin wrote:
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.
Doing as a separate change.
http://codereview.chromium.org/545007/diff/16001/17002#newcode8079
src/ia32/codegen-ia32.cc:8079: void
FloatingPointHelper::LoadSSE2Operands(MacroAssembler* masm) {
On 2010/02/15 13:02:09, Kevin Millikin wrote:
If this takes a scratch register you can use the scratch register for
smi
untagging, then you don't have to retag.
Good point. I'll do this as a separate change.
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(),
On 2010/02/15 13:02:09, Kevin Millikin wrote:
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.
Done.
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_
On 2010/02/15 13:02:09, Kevin Millikin wrote:
The include guard should be V8_NUMBER_INFO_H_
Done.
http://codereview.chromium.org/545007/diff/21001/22006#newcode54
src/number-info.h:54: #endif // V8_kNumberTypeINFO_H_
On 2010/02/15 13:02:09, Kevin Millikin wrote:
Too much search and replace.
Indeed :)
Done.
http://codereview.chromium.org/545007
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
