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

Review URL: https://chromiumcodereview.appspot.com/9584006

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10905 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/code-stubs-arm.cc

@@ -6560,15 +6560,15 @@ void ICCompareStub::GenerateHeapNumbers(MacroAssembler* masm) {
   ASSERT(state_ == CompareIC::HEAP_NUMBERS);
 
   Label generic_stub;
-  Label unordered;
+  Label unordered, maybe_undefined1, maybe_undefined2;
   Label miss;
   __ and_(r2, r1, Operand(r0));
   __ JumpIfSmi(r2, &generic_stub);
 
   __ CompareObjectType(r0, r2, r2, HEAP_NUMBER_TYPE);
-  __ b(ne, &miss);
+  __ b(ne, &maybe_undefined1);
   __ CompareObjectType(r1, r2, r2, HEAP_NUMBER_TYPE);
-  __ b(ne, &miss);
+  __ b(ne, &maybe_undefined2);
 
   // Inlining the double comparison and falling back to the general compare
   // stub if NaN is involved or VFP3 is unsupported.
@@ -6592,14 +6592,28 @@ void ICCompareStub::GenerateHeapNumbers(MacroAssembler* masm) {
     __ mov(r0, Operand(LESS), LeaveCC, lt);
     __ mov(r0, Operand(GREATER), LeaveCC, gt);
     __ Ret();
-
-    __ bind(&unordered);
   }
 
+  __ bind(&unordered);
   CompareStub stub(GetCondition(), strict(), NO_COMPARE_FLAGS, r1, r0);
   __ bind(&generic_stub);
   __ Jump(stub.GetCode(), RelocInfo::CODE_TARGET);
 
+  __ bind(&maybe_undefined1);
+  if (Token::IsOrderedRelationalCompareOp(op_)) {
+    __ CompareRoot(r0, Heap::kUndefinedValueRootIndex);
+    __ b(ne, &miss);
+    __ CompareObjectType(r1, r2, r2, HEAP_NUMBER_TYPE);
+    __ b(ne, &maybe_undefined2);
+    __ jmp(&unordered);
+  }
+
+  __ bind(&maybe_undefined2);
+  if (Token::IsOrderedRelationalCompareOp(op_)) {
+    __ CompareRoot(r1, Heap::kUndefinedValueRootIndex);
+    __ b(eq, &unordered);
+  }
+
   __ bind(&miss);
   GenerateMiss(masm);
 }

src/hydrogen-instructions.cc

@@ -1479,7 +1479,22 @@ void HGoto::PrintDataTo(StringStream* stream) {
 void HCompareIDAndBranch::SetInputRepresentation(Representation r) {
   input_representation_ = r;
   if (r.IsDouble()) {
+    // According to the ES5 spec (11.9.3, 11.8.5), Equality comparisons (==, ===
+    // and !=) have special handling of undefined, e.g. undefined == undefined
+    // is 'true'. Relational comparisons have a different semantic, first
+    // calling ToPrimitive() on their arguments.  The standard Crankshaft
+    // tagged-to-double conversion to ensure the HCompareIDAndBranch's inputs
+    // are doubles caused 'undefined' to be converted to NaN. That's compatible
+    // out-of-the box with ordered relational comparisons (<, >, <=,
+    // >=). However, for equality comparisons (and for 'in' and 'instanceof'),
+    // it is not consistent with the spec. For example, it would cause undefined
+    // == undefined (should be true) to be evaluated as NaN == NaN
+    // (false). Therefore, any comparisons other than ordered relational
+    // comparisons must cause a deopt when one of their arguments is undefined.
+    // See also v8:1434
+    if (!Token::IsOrderedRelationalCompareOp(token_)) {
       SetFlag(kDeoptimizeOnUndefined);
+    }
   } else {
     ASSERT(r.IsInteger32());
   }

src/ia32/code-stubs-ia32.cc

@@ -6572,16 +6572,16 @@ void ICCompareStub::GenerateHeapNumbers(MacroAssembler* masm) {
   ASSERT(state_ == CompareIC::HEAP_NUMBERS);
 
   Label generic_stub;
-  Label unordered;
+  Label unordered, maybe_undefined1, maybe_undefined2;
   Label miss;
   __ mov(ecx, edx);
   __ and_(ecx, eax);
   __ JumpIfSmi(ecx, &generic_stub, Label::kNear);
 
   __ CmpObjectType(eax, HEAP_NUMBER_TYPE, ecx);
-  __ j(not_equal, &miss, Label::kNear);
+  __ j(not_equal, &maybe_undefined1, Label::kNear);
   __ CmpObjectType(edx, HEAP_NUMBER_TYPE, ecx);
-  __ j(not_equal, &miss, Label::kNear);
+  __ j(not_equal, &maybe_undefined2, Label::kNear);
 
   // Inlining the double comparison and falling back to the general compare
   // stub if NaN is involved or SS2 or CMOV is unsupported.
@@ -6607,14 +6607,28 @@ void ICCompareStub::GenerateHeapNumbers(MacroAssembler* masm) {
     __ mov(ecx, Immediate(Smi::FromInt(-1)));
     __ cmov(below, eax, ecx);
     __ ret(0);
-
-    __ bind(&unordered);
   }
 
+  __ bind(&unordered);
   CompareStub stub(GetCondition(), strict(), NO_COMPARE_FLAGS);
   __ bind(&generic_stub);
   __ jmp(stub.GetCode(), RelocInfo::CODE_TARGET);
 
+  __ bind(&maybe_undefined1);
+  if (Token::IsOrderedRelationalCompareOp(op_)) {
+    __ cmp(eax, Immediate(masm->isolate()->factory()->undefined_value()));
+    __ j(not_equal, &miss);
+    __ CmpObjectType(edx, HEAP_NUMBER_TYPE, ecx);
+    __ j(not_equal, &maybe_undefined2, Label::kNear);
+    __ jmp(&unordered);
+  }
+
+  __ bind(&maybe_undefined2);
+  if (Token::IsOrderedRelationalCompareOp(op_)) {
+    __ cmp(edx, Immediate(masm->isolate()->factory()->undefined_value()));
+    __ j(equal, &unordered);
+  }
+
   __ bind(&miss);
   GenerateMiss(masm);
 }

src/ic.cc

@@ -2482,6 +2482,14 @@ CompareIC::State CompareIC::TargetState(State state,
     case UNINITIALIZED:
       if (x->IsSmi() && y->IsSmi()) return SMIS;
       if (x->IsNumber() && y->IsNumber()) return HEAP_NUMBERS;
+      if (Token::IsOrderedRelationalCompareOp(op_)) {
+        // Ordered comparisons treat undefined as NaN, so the
+        // HEAP_NUMBER stub will do the right thing.
+        if ((x->IsNumber() && y->IsUndefined()) ||
+            (y->IsNumber() && x->IsUndefined())) {
+          return HEAP_NUMBERS;
+        }
+      }
       if (!Token::IsEqualityOp(op_)) return GENERIC;
       if (x->IsSymbol() && y->IsSymbol()) return SYMBOLS;
       if (x->IsString() && y->IsString()) return STRINGS;

src/token.h

-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -215,7 +215,7 @@ class Token {
     return EQ <= op && op <= IN;
   }
 
-  static bool IsOrderedCompareOp(Value op) {
+  static bool IsOrderedRelationalCompareOp(Value op) {
     return op == LT || op == LTE || op == GT || op == GTE;
   }
 

src/x64/code-stubs-x64.cc

@@ -5522,15 +5522,15 @@ void ICCompareStub::GenerateHeapNumbers(MacroAssembler* masm) {
   ASSERT(state_ == CompareIC::HEAP_NUMBERS);
 
   Label generic_stub;
-  Label unordered;
+  Label unordered, maybe_undefined1, maybe_undefined2;
   Label miss;
   Condition either_smi = masm->CheckEitherSmi(rax, rdx);
   __ j(either_smi, &generic_stub, Label::kNear);
 
   __ CmpObjectType(rax, HEAP_NUMBER_TYPE, rcx);
-  __ j(not_equal, &miss, Label::kNear);
+  __ j(not_equal, &maybe_undefined1, Label::kNear);
   __ CmpObjectType(rdx, HEAP_NUMBER_TYPE, rcx);
-  __ j(not_equal, &miss, Label::kNear);
+  __ j(not_equal, &maybe_undefined2, Label::kNear);
 
   // Load left and right operand
   __ movsd(xmm0, FieldOperand(rdx, HeapNumber::kValueOffset));
@@ -5551,11 +5551,25 @@ void ICCompareStub::GenerateHeapNumbers(MacroAssembler* masm) {
   __ ret(0);
 
   __ bind(&unordered);
-
   CompareStub stub(GetCondition(), strict(), NO_COMPARE_FLAGS);
   __ bind(&generic_stub);
   __ jmp(stub.GetCode(), RelocInfo::CODE_TARGET);
 
+  __ bind(&maybe_undefined1);
+  if (Token::IsOrderedRelationalCompareOp(op_)) {
+    __ Cmp(rax, masm->isolate()->factory()->undefined_value());
+    __ j(not_equal, &miss);
+    __ CmpObjectType(rdx, HEAP_NUMBER_TYPE, rcx);
+    __ j(not_equal, &maybe_undefined2, Label::kNear);
+    __ jmp(&unordered);
+  }
+
+  __ bind(&maybe_undefined2);
+  if (Token::IsOrderedRelationalCompareOp(op_)) {
+    __ Cmp(rdx, masm->isolate()->factory()->undefined_value());
+    __ j(equal, &unordered);
+  }
+
   __ bind(&miss);
   GenerateMiss(masm);
 }

test/mjsunit/comparison-ops-and-undefined.js

+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+function test_helper_for_ics(func, b1, b2, b3, b4) {
+  assertEquals(b1, func(.5, .5));
+  assertEquals(b2, func(.5, undefined));
+  assertEquals(b3, func(undefined, .5));
+  assertEquals(b4, func(undefined, undefined));
+}
+
+function test_helper_for_crankshaft(func, b1, b2, b3, b4) {
+  assertEquals(b1, func(.5, .5));
+  %OptimizeFunctionOnNextCall(func);
+  assertEquals(b1, func(.5, .5));
+  assertEquals(b2, func(.5, undefined));
+  assertEquals(b3, func(undefined, .5));
+  assertEquals(b4, func(undefined, undefined));
+}
+
+function less_1(a, b) {
+  return a < b;
+}
+
+test_helper_for_ics(less_1, false, false, false, false);
+
+function less_2(a, b) {
+  return a < b;
+}
+
+test_helper_for_crankshaft(less_1, false, false, false, false);
+
+function greater_1(a, b) {
+  return a > b;
+}
+
+test_helper_for_ics(greater_1, false, false, false, false);
+
+function greater_2(a, b) {
+  return a > b;
+}
+
+test_helper_for_crankshaft(greater_1, false, false, false, false);
+
+function less_equal_1(a, b) {
+  return a <= b;
+}
+
+test_helper_for_ics(less_equal_1, true, false, false, false);
+
+function less_equal_2(a, b) {
+  return a <= b;
+}
+
+test_helper_for_crankshaft(less_equal_1, true, false, false, false);
+
+function greater_equal_1(a, b) {
+  return a >= b;
+}
+
+test_helper_for_ics(greater_equal_1, true, false, false, false);
+
+function greater_equal_2(a, b) {
+  return a >= b;
+}
+
+test_helper_for_crankshaft(greater_equal_1, true, false, false, false);
+
+function equal_1(a, b) {
+  return a == b;
+}
+
+test_helper_for_ics(equal_1, true, false, false, true);
+
+function equal_2(a, b) {
+  return a == b;
+}
+
+test_helper_for_crankshaft(equal_2, true, false, false, true);
+
+function strict_equal_1(a, b) {
+  return a === b;
+}
+
+test_helper_for_ics(strict_equal_1, true, false, false, true);
+
+function strict_equal_2(a, b) {
+  return a === b;
+}
+
+test_helper_for_crankshaft(strict_equal_2, true, false, false, true);
+
+function not_equal_1(a, b) {
+  return a != b;
+}
+
+test_helper_for_ics(not_equal_1, false, true, true, false);
+
+function not_equal_2(a, b) {
+  return a != b;
+}
+
+test_helper_for_crankshaft(not_equal_2, false, true, true, false);