Avoid redundant smi check in x64 loading of floats into SSE2 registers.

Avoid redundant smi check in x64 loading of floats into SSE2 registers.

Committed: http://code.google.com/p/v8/source/detail?r=4903

[Lasse Reichstein, 2010-06-16 07:48:42]

LGTM w/ fix.

http://codereview.chromium.org/2862004/diff/4001/5001
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9940
src/x64/codegen-x64.cc:9940: __ jmp(&load_rax);
Maybe convert this jmp to JumpIfNotSmi(rax, &load_rax_heapnumber), to utilize
the fallthrough.

But then we might as well duplicate the load of rax's value, it's just one
instruction.

It's slightly larger, but looks like it should have better "flow".

Do we sometimes know that not both can be smis?

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9962
src/x64/codegen-x64.cc:9962: __ cmpq(FieldOperand(rax, HeapObject::kMapOffset),
rcx);
rcx might not have been loaded here if rdx was a Smi.
Put the LoadRoot before the JumpIfSmi(rdx...).
It will be unused if both operands are smis, but most of the time we shouldn't
be converting to double if both are smis anyway.

This might be the cause of going to runtime more, since we misrecognize the
HeapNumber.

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9963
src/x64/codegen-x64.cc:9963: __ j(equal, &load_float_rax);
Why not put load_float_rax here, with a jmp to done after it, instead of a
conditional jump followed by an unconditional one?

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9964
src/x64/codegen-x64.cc:9964: __ jmp(not_numbers);  // Argument in eax is not a
number.
eax->rax

http://codereview.chromium.org/2862004/diff/4001/5001#newcode10409
src/x64/codegen-x64.cc:10409: if (static_operands_type_.IsSmi()) {
If we have HasSmiCodeInStub, this case should have been handled, and otherwise
we wouldn't call the stub at all with two smis. Could we check if this case is
ever hit?

[William Hesse, 2010-06-16 16:19:20]

Changes made.

http://codereview.chromium.org/2862004/diff/4001/5001
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9940
src/x64/codegen-x64.cc:9940: __ jmp(&load_rax);
On 2010/06/16 07:48:43, Lasse Reichstein wrote:
> Maybe convert this jmp to JumpIfNotSmi(rax, &load_rax_heapnumber), to utilize
> the fallthrough.
> 
> But then we might as well duplicate the load of rax's value, it's just one
> instruction.
> 
> It's slightly larger, but looks like it should have better "flow".
> 
> Do we sometimes know that not both can be smis?

Done.  We don't duplicate the load, since we can't use the fall through in
either place.  We do often know that they are not both smis.  We can specialize
for that in a separate change.

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9962
src/x64/codegen-x64.cc:9962: __ cmpq(FieldOperand(rax, HeapObject::kMapOffset),
rcx);
On 2010/06/16 07:48:43, Lasse Reichstein wrote:
> rcx might not have been loaded here if rdx was a Smi.
> Put the LoadRoot before the JumpIfSmi(rdx...).
> It will be unused if both operands are smis, but most of the time we shouldn't
> be converting to double if both are smis anyway.
> 
> This might be the cause of going to runtime more, since we misrecognize the
> HeapNumber.

Done.

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9963
src/x64/codegen-x64.cc:9963: __ j(equal, &load_float_rax);
On 2010/06/16 07:48:43, Lasse Reichstein wrote:
> Why not put load_float_rax here, with a jmp to done after it, instead of a
> conditional jump followed by an unconditional one?

Done.

http://codereview.chromium.org/2862004/diff/4001/5001#newcode9964
src/x64/codegen-x64.cc:9964: __ jmp(not_numbers);  // Argument in eax is not a
number.
On 2010/06/16 07:48:43, Lasse Reichstein wrote:
> eax->rax

Done.

http://codereview.chromium.org/2862004/diff/4001/5001#newcode10409
src/x64/codegen-x64.cc:10409: if (static_operands_type_.IsSmi()) {
On 2010/06/16 07:48:43, Lasse Reichstein wrote:
> If we have HasSmiCodeInStub, this case should have been handled, and otherwise
> we wouldn't call the stub at all with two smis. Could we check if this case is
> ever hit?
> 

Yes, it should never be hit.
Changed to an assert.