Inline ordered relative compares of mixed double/undefined values.

Inline ordered relational compares of mixed double/undefined values.

Allow Crankshaft to inline ordered relational comparisons (<, >, <=, >=) that have undefined arguments in addition to double value arguments (rather than calling the generic Compare stub).

R=fschneider@chromium.org
TEST=test/mjsunit/comparison-ops-and-undefined.js

Committed: https://code.google.com/p/v8/source/detail?r=10905

[fschneider, 2012-03-02 12:39:15]

lgtm

https://chromiumcodereview.appspot.com/9584006/diff/1/src/hydrogen-instructio...
File src/hydrogen-instructions.cc (right):

https://chromiumcodereview.appspot.com/9584006/diff/1/src/hydrogen-instructio...
src/hydrogen-instructions.cc:1460: // ==, === and != have special handling of
undefined, specifically undeifned
s/undeifned/undefined

https://chromiumcodereview.appspot.com/9584006/diff/1/src/hydrogen-instructio...
src/hydrogen-instructions.cc:1467: // See v8:1434
I'd also refer to the corresponding sections in the ES5 spec (11.9.3, 11.8.5)
and use the same terms "relational comparison" and "equality comparison".

https://chromiumcodereview.appspot.com/9584006/diff/1/src/hydrogen-instructio...
src/hydrogen-instructions.cc:1468: if (!Token::IsOrderedCompareOp(token_)) {
maybe rename to IsRelationalCompareOp.

https://chromiumcodereview.appspot.com/9584006/diff/1/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

https://chromiumcodereview.appspot.com/9584006/diff/1/src/ia32/ic-ia32.cc#new...
src/ia32/ic-ia32.cc:1730: JavaScriptFrame::PrintTop(stdout, false, true);
Leftover from debugging?

https://chromiumcodereview.appspot.com/9584006/diff/1/test/mjsunit/comparison...
File test/mjsunit/comparison-ops-and-undefined.js (right):

https://chromiumcodereview.appspot.com/9584006/diff/1/test/mjsunit/comparison...
test/mjsunit/comparison-ops-and-undefined.js:44: assertFalse(less_2(.5, .5));
You could compress this into a helper

function test_helper(fun, b1, b2, b3, b4) {
  assertEquals(b1, fun(.5, .5));
  assertEquals(b2, fun(.5, undefined));
  assertEquals(b3, fun(undefined, .5));
  assertEquals(b4, fun(undefined, undefined));
}

and call it here like 

test_helper(less_2, false, false, false, false);

Then it becomes also easy to add tests for different typefeedback in addition to
heap numbers (.5, .5): smis, int32, undefined.

[danno, 2012-03-02 13:28:45]

Feedback addressed, landing

http://codereview.chromium.org/9584006/diff/1/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):

http://codereview.chromium.org/9584006/diff/1/src/hydrogen-instructions.cc#ne...
src/hydrogen-instructions.cc:1460: // ==, === and != have special handling of
undefined, specifically undeifned
On 2012/03/02 12:39:15, fschneider wrote:
> s/undeifned/undefined

Done.

http://codereview.chromium.org/9584006/diff/1/src/hydrogen-instructions.cc#ne...
src/hydrogen-instructions.cc:1467: // See v8:1434
On 2012/03/02 12:39:15, fschneider wrote:
> I'd also refer to the corresponding sections in the ES5 spec (11.9.3, 11.8.5)
> and use the same terms "relational comparison" and "equality comparison".

Done.

http://codereview.chromium.org/9584006/diff/1/src/hydrogen-instructions.cc#ne...
src/hydrogen-instructions.cc:1468: if (!Token::IsOrderedCompareOp(token_)) {
On 2012/03/02 12:39:15, fschneider wrote:
> maybe rename to IsRelationalCompareOp.

Done.

http://codereview.chromium.org/9584006/diff/1/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/9584006/diff/1/src/ia32/ic-ia32.cc#newcode1730
src/ia32/ic-ia32.cc:1730: JavaScriptFrame::PrintTop(stdout, false, true);
On 2012/03/02 12:39:15, fschneider wrote:
> Leftover from debugging?

Done.

http://codereview.chromium.org/9584006/diff/1/test/mjsunit/comparison-ops-and...
File test/mjsunit/comparison-ops-and-undefined.js (right):

http://codereview.chromium.org/9584006/diff/1/test/mjsunit/comparison-ops-and...
test/mjsunit/comparison-ops-and-undefined.js:44: assertFalse(less_2(.5, .5));
On 2012/03/02 12:39:15, fschneider wrote:
> You could compress this into a helper
> 
> function test_helper(fun, b1, b2, b3, b4) {
>   assertEquals(b1, fun(.5, .5));
>   assertEquals(b2, fun(.5, undefined));
>   assertEquals(b3, fun(undefined, .5));
>   assertEquals(b4, fun(undefined, undefined));
> }
> 
> and call it here like 
> 
> test_helper(less_2, false, false, false, false);
> 
> Then it becomes also easy to add tests for different typefeedback in addition
to
> heap numbers (.5, .5): smis, int32, undefined.

Done.