Move inlined comparisons with null constant to use virtual frame constants.

Move inlined comparisons with null constant to use virtual frame constants.

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

[William Hesse, 2009-02-19 12:42:58]

A small changelist.

[Kevin Millikin (Chromium), 2009-02-19 13:00:45]

LGTM.

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

http://codereview.chromium.org/21509/diff/1/2#newcode1334
Line 1334: (left_side.is_constant() && left_side.handle()->IsNull() ||
This condition should be aligned with the previous one, cc == equal.

http://codereview.chromium.org/21509/diff/1/2#newcode1341
Line 1341: (left_side.is_constant() && left_side.handle()->IsNull()) ?
I don't like breaking the ternary operator like this.  I'd rather see the ? and
: at the beginning of the following line.

You might consider just checking up front if left and right are null, at the
same time you check for smi constants, ie:

bool left_is_constant_smi = false;
bool left_is_constant_null = false;
if (left_side.is_constant()) {
  left_is_constant_smi = left_side.handle()->IsSmi();
  left_is_constant_null = left_side.handle()->IsNull();
}

Then eliminating the repeated && expression.

[William Hesse, 2009-02-19 13:20:50]

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

http://codereview.chromium.org/21509/diff/1/2#newcode1334
Line 1334: (left_side.is_constant() && left_side.handle()->IsNull() ||
On 2009/02/19 13:00:45, Kevin Millikin wrote:
> This condition should be aligned with the previous one, cc == equal.

Done.

http://codereview.chromium.org/21509/diff/1/2#newcode1341
Line 1341: (left_side.is_constant() && left_side.handle()->IsNull()) ?
On 2009/02/19 13:00:45, Kevin Millikin wrote:
> I don't like breaking the ternary operator like this.  I'd rather see the ?
and
> : at the beginning of the following line.
> 
> You might consider just checking up front if left and right are null, at the
> same time you check for smi constants, ie:
> 
> bool left_is_constant_smi = false;
> bool left_is_constant_null = false;
> if (left_side.is_constant()) {
>   left_is_constant_smi = left_side.handle()->IsSmi();
>   left_is_constant_null = left_side.handle()->IsNull();
> }
> 
> Then eliminating the repeated && expression.

Done.