Revision: 10905
Author: [email protected]
Date: Fri Mar 2 05:40:14 2012
Log: 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).
[email protected]
TEST=test/mjsunit/comparison-ops-and-undefined.js
Review URL: https://chromiumcodereview.appspot.com/9584006
http://code.google.com/p/v8/source/detail?r=10905
Added:
/branches/bleeding_edge/test/mjsunit/comparison-ops-and-undefined.js
Modified:
/branches/bleeding_edge/src/arm/code-stubs-arm.cc
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/token.h
/branches/bleeding_edge/src/x64/code-stubs-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/comparison-ops-and-undefined.js
Fri Mar 2 05:40:14 2012
@@ -0,0 +1,128 @@
+// 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);
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Tue Feb 28 00:32:44
2012
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Fri Mar 2 05:40:14
2012
@@ -6560,15 +6560,15 @@
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,13 +6592,27 @@
__ 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);
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Wed Feb 29
05:41:18 2012
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Fri Mar 2
05:40:14 2012
@@ -1479,7 +1479,22 @@
void HCompareIDAndBranch::SetInputRepresentation(Representation r) {
input_representation_ = r;
if (r.IsDouble()) {
- SetFlag(kDeoptimizeOnUndefined);
+ // 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());
}
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Tue Feb 28 00:32:44
2012
+++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Fri Mar 2 05:40:14
2012
@@ -6572,16 +6572,16 @@
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,13 +6607,27 @@
__ 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);
=======================================
--- /branches/bleeding_edge/src/ic.cc Mon Feb 27 00:08:14 2012
+++ /branches/bleeding_edge/src/ic.cc Fri Mar 2 05:40:14 2012
@@ -2482,6 +2482,14 @@
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;
=======================================
--- /branches/bleeding_edge/src/token.h Fri Feb 24 07:53:09 2012
+++ /branches/bleeding_edge/src/token.h Fri Mar 2 05:40:14 2012
@@ -1,4 +1,4 @@
-// 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 @@
return EQ <= op && op <= IN;
}
- static bool IsOrderedCompareOp(Value op) {
+ static bool IsOrderedRelationalCompareOp(Value op) {
return op == LT || op == LTE || op == GT || op == GTE;
}
=======================================
--- /branches/bleeding_edge/src/x64/code-stubs-x64.cc Tue Feb 28 00:32:44
2012
+++ /branches/bleeding_edge/src/x64/code-stubs-x64.cc Fri Mar 2 05:40:14
2012
@@ -5522,15 +5522,15 @@
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,10 +5551,24 @@
__ 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);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
