[ic] Also collect known map for relational comparison.
authorbmeurer <bmeurer@chromium.org>
Mon, 21 Sep 2015 16:05:27 +0000 (09:05 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 21 Sep 2015 16:05:43 +0000 (16:05 +0000)
Previously we only collected the known map for equality comparisons. But
if we also collect it for relational comparisons, we can inline a fast
path of ToPrimitive on the objects, which is especially interesting
since both sides have the same map.

For now we only inline a very limited subset of ToPrimitive in
Crankshaft, which is when the receiver map (and its prototype chain)
doesn't have @@toPrimitive, and both valueOf and toString are the
default versions on the %ObjectPrototype%. In this case the relational
comparison would reduce to a string comparison of "[object CLASS]" with
itself and so we can reduce that to a boolean constant plus map checks
on both left and right hand side, plus code dependencies on the
prototype chain. This repairs the regression on box2d.

R=jkummerow@chromium.org
BUG=chromium:534200
LOG=n

Review URL: https://codereview.chromium.org/1355113002

Cr-Commit-Position: refs/heads/master@{#30852}

13 files changed:
src/arm/code-stubs-arm.cc
src/arm64/code-stubs-arm64.cc
src/code-stubs.cc
src/contexts.h
src/hydrogen.cc
src/ia32/code-stubs-ia32.cc
src/ic/ic-state.cc
src/mips/code-stubs-mips.cc
src/mips64/code-stubs-mips64.cc
src/v8natives.js
src/x64/code-stubs-x64.cc
test/mjsunit/compare-known-objects-slow.js
test/mjsunit/compare-known-objects.js

index cb7d15f..154db4f 100644 (file)
@@ -3672,8 +3672,20 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) {
   __ cmp(r3, r4);
   __ b(ne, &miss);
 
-  __ sub(r0, r0, Operand(r1));
-  __ Ret();
+  if (Token::IsEqualityOp(op())) {
+    __ sub(r0, r0, Operand(r1));
+    __ Ret();
+  } else if (is_strong(strength())) {
+    __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1);
+  } else {
+    if (op() == Token::LT || op() == Token::LTE) {
+      __ mov(r2, Operand(Smi::FromInt(GREATER)));
+    } else {
+      __ mov(r2, Operand(Smi::FromInt(LESS)));
+    }
+    __ Push(r1, r0, r2);
+    __ TailCallRuntime(Runtime::kCompare, 3, 1);
+  }
 
   __ bind(&miss);
   GenerateMiss(masm);
index e93fe44..9def803 100644 (file)
@@ -3567,8 +3567,21 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) {
   __ Cmp(lhs_map, map);
   __ B(ne, &miss);
 
+  if (Token::IsEqualityOp(op())) {
   __ Sub(result, rhs, lhs);
   __ Ret();
+  } else if (is_strong(strength())) {
+    __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1);
+  } else {
+    Register ncr = x2;
+    if (op() == Token::LT || op() == Token::LTE) {
+      __ Mov(ncr, Smi::FromInt(GREATER));
+    } else {
+      __ Mov(ncr, Smi::FromInt(LESS));
+    }
+    __ Push(lhs, rhs, ncr);
+    __ TailCallRuntime(Runtime::kCompare, 3, 1);
+  }
 
   __ Bind(&miss);
   GenerateMiss(masm);
index 3e1b548..8bd7c54 100644 (file)
@@ -383,7 +383,6 @@ bool CompareICStub::FindCodeInSpecialCache(Code** code_out) {
   Code::Flags flags = Code::ComputeFlags(
       GetCodeKind(),
       UNINITIALIZED);
-  DCHECK(op() == Token::EQ || op() == Token::EQ_STRICT);
   Handle<Object> probe(
       known_map_->FindInCodeCache(
         strict() ?
index fb83923..1692038 100644 (file)
@@ -132,6 +132,8 @@ enum BindingFlags {
   V(NATIVE_OBJECT_OBSERVE_INDEX, JSFunction, native_object_observe)           \
   V(NO_SIDE_EFFECT_TO_STRING_FUN_INDEX, JSFunction,                           \
     no_side_effect_to_string_fun)                                             \
+  V(OBJECT_VALUE_OF, JSFunction, object_value_of)                             \
+  V(OBJECT_TO_STRING, JSFunction, object_to_string)                           \
   V(OBJECT_DEFINE_OWN_PROPERTY_INDEX, JSFunction, object_define_own_property) \
   V(OBJECT_GET_OWN_PROPERTY_DESCROPTOR_INDEX, JSFunction,                     \
     object_get_own_property_descriptor)                                       \
index 5ea5776..2796754 100644 (file)
@@ -11578,6 +11578,46 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction(
         return result;
       }
     } else {
+      if (combined_type->IsClass()) {
+        // TODO(bmeurer): This is an optimized version of an x < y, x > y,
+        // x <= y or x >= y, where both x and y are spec objects with the
+        // same map. The CompareIC collects this map for us. So if we know
+        // that there's no @@toPrimitive on the map (including the prototype
+        // chain), and both valueOf and toString are the default initial
+        // implementations (on the %ObjectPrototype%), then we can reduce
+        // the comparison to map checks on x and y, because the comparison
+        // will turn into a comparison of "[object CLASS]" to itself (the
+        // default outcome of toString, since valueOf returns a spec object).
+        // This is pretty much adhoc, so in TurboFan we could do a lot better
+        // and inline the interesting parts of ToPrimitive (actually we could
+        // even do that in Crankshaft but we don't want to waste too much
+        // time on this now).
+        DCHECK(Token::IsOrderedRelationalCompareOp(op));
+        Handle<Map> map = combined_type->AsClass()->Map();
+        PropertyAccessInfo value_of(this, LOAD, map,
+                                    isolate()->factory()->valueOf_string());
+        PropertyAccessInfo to_primitive(
+            this, LOAD, map, isolate()->factory()->to_primitive_symbol());
+        PropertyAccessInfo to_string(this, LOAD, map,
+                                     isolate()->factory()->toString_string());
+        if (to_primitive.CanAccessMonomorphic() && !to_primitive.IsFound() &&
+            value_of.CanAccessMonomorphic() && value_of.IsDataConstant() &&
+            value_of.constant().is_identical_to(isolate()->object_value_of()) &&
+            to_string.CanAccessMonomorphic() && to_string.IsDataConstant() &&
+            to_string.constant().is_identical_to(
+                isolate()->object_to_string())) {
+          // We depend on the prototype chain to stay the same, because we
+          // also need to deoptimize when someone installs @@toPrimitive
+          // somewhere in the prototype chain.
+          BuildCheckPrototypeMaps(handle(JSObject::cast(map->prototype())),
+                                  Handle<JSObject>::null());
+          AddCheckMap(left, map);
+          AddCheckMap(right, map);
+          // The caller expects a branch instruction, so make it happy.
+          return New<HBranch>(
+              graph()->GetConstantBool(op == Token::LTE || op == Token::GTE));
+        }
+      }
       Bailout(kUnsupportedNonPrimitiveCompare);
       return NULL;
     }
index 3cb99eb..7da0779 100644 (file)
@@ -3744,15 +3744,24 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) {
   __ JumpIfSmi(ecx, &miss, Label::kNear);
 
   __ GetWeakValue(edi, cell);
-  __ mov(ecx, FieldOperand(eax, HeapObject::kMapOffset));
-  __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset));
-  __ cmp(ecx, edi);
+  __ cmp(edi, FieldOperand(eax, HeapObject::kMapOffset));
   __ j(not_equal, &miss, Label::kNear);
-  __ cmp(ebx, edi);
+  __ cmp(edi, FieldOperand(edx, HeapObject::kMapOffset));
   __ j(not_equal, &miss, Label::kNear);
 
-  __ sub(eax, edx);
-  __ ret(0);
+  if (Token::IsEqualityOp(op())) {
+    __ sub(eax, edx);
+    __ ret(0);
+  } else if (is_strong(strength())) {
+    __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1);
+  } else {
+    __ PopReturnAddressTo(ecx);
+    __ Push(edx);
+    __ Push(eax);
+    __ Push(Immediate(Smi::FromInt(NegativeComparisonResult(GetCondition()))));
+    __ PushReturnAddressFrom(ecx);
+    __ TailCallRuntime(Runtime::kCompare, 3, 1);
+  }
 
   __ bind(&miss);
   GenerateMiss(masm);
index fc33c80..79e2f14 100644 (file)
@@ -470,16 +470,16 @@ CompareICState::State CompareICState::TargetState(
         return Token::IsEqualityOp(op) ? INTERNALIZED_STRING : STRING;
       }
       if (x->IsString() && y->IsString()) return STRING;
-      if (!Token::IsEqualityOp(op)) return GENERIC;
-      if (x->IsUniqueName() && y->IsUniqueName()) return UNIQUE_NAME;
       if (x->IsJSObject() && y->IsJSObject()) {
         if (Handle<JSObject>::cast(x)->map() ==
             Handle<JSObject>::cast(y)->map()) {
           return KNOWN_OBJECT;
         } else {
-          return OBJECT;
+          return Token::IsEqualityOp(op) ? OBJECT : GENERIC;
         }
       }
+      if (!Token::IsEqualityOp(op)) return GENERIC;
+      if (x->IsUniqueName() && y->IsUniqueName()) return UNIQUE_NAME;
       return GENERIC;
     case SMI:
       return x->IsNumber() && y->IsNumber() ? NUMBER : GENERIC;
@@ -496,9 +496,8 @@ CompareICState::State CompareICState::TargetState(
       if (old_right == SMI && y->IsHeapNumber()) return NUMBER;
       return GENERIC;
     case KNOWN_OBJECT:
-      DCHECK(Token::IsEqualityOp(op));
       if (x->IsJSObject() && y->IsJSObject()) {
-        return OBJECT;
+        return Token::IsEqualityOp(op) ? OBJECT : GENERIC;
       }
       return GENERIC;
     case STRING:
index 78c24cf..bdb3823 100644 (file)
@@ -3859,8 +3859,20 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) {
   __ Branch(&miss, ne, a2, Operand(t0));
   __ Branch(&miss, ne, a3, Operand(t0));
 
-  __ Ret(USE_DELAY_SLOT);
-  __ subu(v0, a0, a1);
+  if (Token::IsEqualityOp(op())) {
+    __ Ret(USE_DELAY_SLOT);
+    __ subu(v0, a0, a1);
+  } else if (is_strong(strength())) {
+    __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1);
+  } else {
+    if (op() == Token::LT || op() == Token::LTE) {
+      __ li(a2, Operand(Smi::FromInt(GREATER)));
+    } else {
+      __ li(a2, Operand(Smi::FromInt(LESS)));
+    }
+    __ Push(a1, a0, a2);
+    __ TailCallRuntime(Runtime::kCompare, 3, 1);
+  }
 
   __ bind(&miss);
   GenerateMiss(masm);
index 9df14e3..f139636 100644 (file)
@@ -3891,8 +3891,20 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) {
   __ Branch(&miss, ne, a2, Operand(a4));
   __ Branch(&miss, ne, a3, Operand(a4));
 
-  __ Ret(USE_DELAY_SLOT);
-  __ dsubu(v0, a0, a1);
+  if (Token::IsEqualityOp(op())) {
+    __ Ret(USE_DELAY_SLOT);
+    __ dsubu(v0, a0, a1);
+  } else if (is_strong(strength())) {
+    __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1);
+  } else {
+    if (op() == Token::LT || op() == Token::LTE) {
+      __ li(a2, Operand(Smi::FromInt(GREATER)));
+    } else {
+      __ li(a2, Operand(Smi::FromInt(LESS)));
+    }
+    __ Push(a1, a0, a2);
+    __ TailCallRuntime(Runtime::kCompare, 3, 1);
+  }
 
   __ bind(&miss);
   GenerateMiss(masm);
index 63cf0a0..2a235a2 100644 (file)
@@ -1844,6 +1844,8 @@ utils.Export(function(to) {
 
 %InstallToContext([
   "global_eval_fun", GlobalEval,
+  "object_value_of", ObjectValueOf,
+  "object_to_string", ObjectToString,
   "object_define_own_property", DefineOwnPropertyFromAPI,
   "object_get_own_property_descriptor", ObjectGetOwnPropertyDescriptor,
   "to_complete_property_descriptor", ToCompletePropertyDescriptor,
index 86cca89..cda5d4f 100644 (file)
@@ -3683,15 +3683,24 @@ void CompareICStub::GenerateKnownObjects(MacroAssembler* masm) {
   __ j(either_smi, &miss, Label::kNear);
 
   __ GetWeakValue(rdi, cell);
-  __ movp(rcx, FieldOperand(rax, HeapObject::kMapOffset));
-  __ movp(rbx, FieldOperand(rdx, HeapObject::kMapOffset));
-  __ cmpp(rcx, rdi);
+  __ cmpp(FieldOperand(rdx, HeapObject::kMapOffset), rdi);
   __ j(not_equal, &miss, Label::kNear);
-  __ cmpp(rbx, rdi);
+  __ cmpp(FieldOperand(rax, HeapObject::kMapOffset), rdi);
   __ j(not_equal, &miss, Label::kNear);
 
-  __ subp(rax, rdx);
-  __ ret(0);
+  if (Token::IsEqualityOp(op())) {
+    __ subp(rax, rdx);
+    __ ret(0);
+  } else if (is_strong(strength())) {
+    __ TailCallRuntime(Runtime::kThrowStrongModeImplicitConversion, 0, 1);
+  } else {
+    __ PopReturnAddressTo(rcx);
+    __ Push(rdx);
+    __ Push(rax);
+    __ Push(Smi::FromInt(NegativeComparisonResult(GetCondition())));
+    __ PushReturnAddressFrom(rcx);
+    __ TailCallRuntime(Runtime::kCompare, 3, 1);
+  }
 
   __ bind(&miss);
   GenerateMiss(masm);
index afa198f..41e9fb4 100644 (file)
@@ -39,6 +39,22 @@ function eq_strict(a, b) {
   return a === b;
 }
 
+function le(a, b) {
+  return a <= b;
+}
+
+function lt(a, b) {
+  return a < b;
+}
+
+function ge(a, b) {
+  return a >= b;
+}
+
+function gt(a, b) {
+  return a > b;
+}
+
 function test(a, b) {
   // Check CompareIC for equality of known objects.
   assertTrue(eq(a, a));
@@ -48,6 +64,22 @@ function test(a, b) {
   assertTrue(eq_strict(a, a));
   assertTrue(eq_strict(b, b));
   assertFalse(eq_strict(a, b));
+  // Check CompareIC for less than or equal of known objects.
+  assertTrue(le(a, a));
+  assertTrue(le(a, b));
+  assertTrue(le(b, a));
+  // Check CompareIC for less than of known objects.
+  assertFalse(lt(a, a));
+  assertFalse(lt(a, b));
+  assertFalse(lt(b, a));
+  // Check CompareIC for greater than or equal of known objects.
+  assertTrue(ge(a, a));
+  assertTrue(ge(a, b));
+  assertTrue(ge(b, a));
+  // Check CompareIC for greater than of known objects.
+  assertFalse(gt(a, a));
+  assertFalse(gt(a, b));
+  assertFalse(gt(b, a));
 }
 
 // Prepare two objects in slow mode that have the same map.
index afffc07..051b12d 100644 (file)
@@ -39,6 +39,22 @@ function eq_strict(a, b) {
   return a === b;
 }
 
+function le(a, b) {
+  return a <= b;
+}
+
+function lt(a, b) {
+  return a < b;
+}
+
+function ge(a, b) {
+  return a >= b;
+}
+
+function gt(a, b) {
+  return a > b;
+}
+
 function test(a, b) {
   // Check CompareIC for equality of known objects.
   assertTrue(eq(a, a));
@@ -48,6 +64,22 @@ function test(a, b) {
   assertTrue(eq_strict(a, a));
   assertTrue(eq_strict(b, b));
   assertFalse(eq_strict(a, b));
+  // Check CompareIC for less than or equal of known objects.
+  assertTrue(le(a, a));
+  assertTrue(le(a, b));
+  assertTrue(le(b, a));
+  // Check CompareIC for less than of known objects.
+  assertFalse(lt(a, a));
+  assertFalse(lt(a, b));
+  assertFalse(lt(b, a));
+  // Check CompareIC for greater than or equal of known objects.
+  assertTrue(ge(a, a));
+  assertTrue(ge(a, b));
+  assertTrue(ge(b, a));
+  // Check CompareIC for greater than of known objects.
+  assertFalse(gt(a, a));
+  assertFalse(gt(a, b));
+  assertFalse(gt(b, a));
 }
 
 function O(){};