Cleanup: use JumpIf[Not]Smi() whenever we can

Cleanup: use JumpIf[Not]Smi() whenever we can

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

[Jakob Kummerow, 2011-06-16 10:02:19]

PTAL. The changes are purely mechanical.

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1724: __ test(input, Immediate(kSmiTagMask));
Can't replace this because |input| is an Operand rather than a Register which
JumpIfSmi() expects.

[Lasse Reichstein, 2011-06-16 11:50:19]

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1724: __ test(input, Immediate(kSmiTagMask));
In such cases, the recommended approach is to add the missing macro to the
macro-assembler.
Please keep the implementation of smis away from the general code, so only the
Smi* macros need to know how a smi is implemented.

In this case, the CheckSmi(Operand) version already exists, so you can also do
the inline version of what a JumpIfSmi(Operand) macro would do:
  Condition if_smi = masm()->CheckSmi(input);
  __ j(if_smi, &done, Label::kNear);

[Jakob Kummerow, 2011-06-16 12:26:31]

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1724: __ test(input, Immediate(kSmiTagMask));
On 2011/06/16 11:50:19, Lasse Reichstein wrote:
> In such cases, the recommended approach is to add the missing macro to the
> macro-assembler.
> Please keep the implementation of smis away from the general code, so only the
> Smi* macros need to know how a smi is implemented.
> 
> In this case, the CheckSmi(Operand) version already exists, so you can also do
> the inline version of what a JumpIfSmi(Operand) macro would do:
>   Condition if_smi = masm()->CheckSmi(input);
>   __ j(if_smi, &done, Label::kNear);

Done.
CheckSmi() is only defined for the x64 macro assembler, so I introduced an
overloaded JumpIfSmi(Operand ...) method. OK?

[Lasse Reichstein, 2011-06-17 11:54:50]

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/7191007/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1724: __ test(input, Immediate(kSmiTagMask));
Fine with me.
I'm really only pedantic about X64 code :)

[Lasse Reichstein, 2011-06-17 11:59:40]

And LGTM