From 426ae4265e539da2932ea2cb2cc1aeed97788b41 Mon Sep 17 00:00:00 2001
From: Benedikt Meurer <bmeurer@chromium.org>
Date: Thu, 3 Aug 2017 07:20:48 +0200
Subject: [PATCH] [csa] Extend TryToName to also implicitly convert Oddball
 keys.

Previously TryToName bailed out for Oddball keys (i.e. when passing true
or false as the key), which meant that for example the generic
KeyedLoadIC would always bail out to the %KeyedGetProperty runtime
function. But handling Oddball keys is fairly easy, since every oddball
value carries it's unique string representation. Adding just this case
to the CodeStubAssembler::TryToName method boosts this simple
micro-benchmark by a factor of 4x:

  const n = 1e7;
  const obj = {};
  const key = true;

  console.time('foo');
  for (let i = 0; i < n; ++i) {
    if (obj[key] === undefined) {
      obj[key] = key;
    }
  }
  console.timeEnd('foo');

It also shows on the ARES-6 ML benchmark and on several Speedometer
tests, where objects are being used as dictionaries and the developers
rely on the implicit ToString conversions on the property accesses.
In the ARES-6 ML benchmark, the number of calls to %KeyedGetProperty
is reduced by 137,758.

Bug: v8:6278, v8:6344, v8:6670
Change-Id: Iaa965e30be4c247682a67ec09543655df9b761d2
Reviewed-on: https://chromium-review.googlesource.com/599527
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47105}
---
 src/code-stub-assembler.cc              | 11 ++++++++--
 test/cctest/test-code-stub-assembler.cc | 29 +++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc
index f6b9ebfb90..79d405f54e 100644
--- a/src/code-stub-assembler.cc
+++ b/src/code-stub-assembler.cc
@@ -5104,7 +5104,8 @@ void CodeStubAssembler::TryToName(Node* key, Label* if_keyisindex,
   DCHECK_EQ(MachineRepresentation::kTagged, var_unique->rep());
   Comment("TryToName");
 
-  Label if_hascachedindex(this), if_keyisnotindex(this), if_thinstring(this);
+  Label if_hascachedindex(this), if_keyisnotindex(this), if_thinstring(this),
+      if_keyisother(this, Label::kDeferred);
   // Handle Smi and HeapNumber keys.
   var_index->Bind(TryToIntptr(key, &if_keyisnotindex));
   Goto(if_keyisindex);
@@ -5117,7 +5118,8 @@ void CodeStubAssembler::TryToName(Node* key, Label* if_keyisindex,
   Node* key_instance_type = LoadMapInstanceType(key_map);
   // Miss if |key| is not a String.
   STATIC_ASSERT(FIRST_NAME_TYPE == FIRST_TYPE);
-  GotoIfNot(IsStringInstanceType(key_instance_type), if_bailout);
+  GotoIfNot(IsStringInstanceType(key_instance_type), &if_keyisother);
+
   // |key| is a String. Check if it has a cached array index.
   Node* hash = LoadNameHashField(key);
   GotoIf(IsClearWord32(hash, Name::kDoesNotContainCachedArrayIndexMask),
@@ -5144,6 +5146,11 @@ void CodeStubAssembler::TryToName(Node* key, Label* if_keyisindex,
   BIND(&if_hascachedindex);
   var_index->Bind(DecodeWordFromWord32<Name::ArrayIndexValueBits>(hash));
   Goto(if_keyisindex);
+
+  BIND(&if_keyisother);
+  GotoIfNot(InstanceTypeEqual(key_instance_type, ODDBALL_TYPE), if_bailout);
+  var_unique->Bind(LoadObjectField(key, Oddball::kToStringOffset));
+  Goto(if_keyisunique);
 }
 
 void CodeStubAssembler::TryInternalizeString(
diff --git a/test/cctest/test-code-stub-assembler.cc b/test/cctest/test-code-stub-assembler.cc
index 689bd3b5d4..ea2a64dcc6 100644
--- a/test/cctest/test-code-stub-assembler.cc
+++ b/test/cctest/test-code-stub-assembler.cc
@@ -395,6 +395,35 @@ TEST(TryToName) {
     ft.CheckTrue(key, expect_index, index);
   }
 
+  {
+    // TryToName(<true>) => if_keyisunique: "true".
+    Handle<Object> key = isolate->factory()->true_value();
+    Handle<Object> unique = isolate->factory()->InternalizeUtf8String("true");
+    ft.CheckTrue(key, expect_unique, unique);
+  }
+
+  {
+    // TryToName(<false>) => if_keyisunique: "false".
+    Handle<Object> key = isolate->factory()->false_value();
+    Handle<Object> unique = isolate->factory()->InternalizeUtf8String("false");
+    ft.CheckTrue(key, expect_unique, unique);
+  }
+
+  {
+    // TryToName(<null>) => if_keyisunique: "null".
+    Handle<Object> key = isolate->factory()->null_value();
+    Handle<Object> unique = isolate->factory()->InternalizeUtf8String("null");
+    ft.CheckTrue(key, expect_unique, unique);
+  }
+
+  {
+    // TryToName(<undefined>) => if_keyisunique: "undefined".
+    Handle<Object> key = isolate->factory()->undefined_value();
+    Handle<Object> unique =
+        isolate->factory()->InternalizeUtf8String("undefined");
+    ft.CheckTrue(key, expect_unique, unique);
+  }
+
   {
     // TryToName(<symbol>) => if_keyisunique: <symbol>.
     Handle<Object> key = isolate->factory()->NewSymbol();