Alternative approach to using type feedback for Symbol-keyed properties

This patch only treats non-private symbols as valid feedback, thus
avoiding the need to switch to Oddballs for the feedback sentinels
and avoiding breaking the use of private own symbols.

Crankshaft will also optimize these symbol loads into a named load,
just as it does for string keyed loads with type feedback.

BUG=

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

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

src/crankshaft/hydrogen-instructions.cc

@@ -4680,13 +4680,13 @@ std::ostream& operator<<(std::ostream& os, const HObjectAccess& access) {
       break;
     case HObjectAccess::kDouble:  // fall through
     case HObjectAccess::kInobject:
-      if (!access.name().is_null()) {
+      if (!access.name().is_null() && access.name()->IsString()) {
         os << Handle<String>::cast(access.name())->ToCString().get();
       }
       os << "[in-object]";
       break;
     case HObjectAccess::kBackingStore:
-      if (!access.name().is_null()) {
+      if (!access.name().is_null() && access.name()->IsString()) {
         os << Handle<String>::cast(access.name())->ToCString().get();
       }
       os << "[backing-store]";

src/crankshaft/hydrogen.cc

@@ -6615,7 +6615,7 @@ HValue* HOptimizedGraphBuilder::BuildMonomorphicAccess(
 void HOptimizedGraphBuilder::HandlePolymorphicNamedFieldAccess(
     PropertyAccessType access_type, Expression* expr, FeedbackVectorSlot slot,
     BailoutId ast_id, BailoutId return_id, HValue* object, HValue* value,
-    SmallMapList* maps, Handle<String> name) {
+    SmallMapList* maps, Handle<Name> name) {
   // Something did not match; must use a polymorphic load.
   int count = 0;
   HBasicBlock* join = NULL;
@@ -7630,19 +7630,42 @@ HValue* HOptimizedGraphBuilder::HandleKeyedElementAccess(
     HValue* obj, HValue* key, HValue* val, Expression* expr,
     FeedbackVectorSlot slot, BailoutId ast_id, BailoutId return_id,
     PropertyAccessType access_type, bool* has_side_effects) {
-  if (key->ActualValue()->IsConstant()) {
+  // A keyed name access with type feedback may contain the name.
+  Handle<TypeFeedbackVector> vector =
+      handle(current_feedback_vector(), isolate());
+  HValue* expected_key = key;
+  if (!key->ActualValue()->IsConstant()) {
+    Name* name = nullptr;
+    if (access_type == LOAD) {
+      KeyedLoadICNexus nexus(vector, slot);
+      name = nexus.FindFirstName();
+    } else if (FLAG_vector_stores) {
+      KeyedStoreICNexus nexus(vector, slot);
+      name = nexus.FindFirstName();
+    }
+    if (name != nullptr) {
+      Handle<Name> handle_name(name);
+      expected_key = Add<HConstant>(handle_name);
+      // We need a check against the key.
+      bool in_new_space = isolate()->heap()->InNewSpace(*handle_name);
+      Unique<Name> unique_name = Unique<Name>::CreateUninitialized(handle_name);
+      Add<HCheckValue>(key, unique_name, in_new_space);
+    }
+  }
+  if (expected_key->ActualValue()->IsConstant()) {
     Handle<Object> constant =
-        HConstant::cast(key->ActualValue())->handle(isolate());
+        HConstant::cast(expected_key->ActualValue())->handle(isolate());
     uint32_t array_index;
-    if (constant->IsString() &&
-        !Handle<String>::cast(constant)->AsArrayIndex(&array_index)) {
+    if ((constant->IsString() &&
+         !Handle<String>::cast(constant)->AsArrayIndex(&array_index)) ||
+        constant->IsSymbol()) {
       if (!constant->IsUniqueName()) {
         constant = isolate()->factory()->InternalizeString(
             Handle<String>::cast(constant));
       }
       HValue* access =
           BuildNamedAccess(access_type, ast_id, return_id, expr, slot, obj,
-                           Handle<String>::cast(constant), val, false);
+                           Handle<Name>::cast(constant), val, false);
       if (access == NULL || access->IsPhi() ||
           HInstruction::cast(access)->IsLinked()) {
         *has_side_effects = false;
@@ -7814,7 +7837,7 @@ bool HOptimizedGraphBuilder::TryArgumentsAccess(Property* expr) {
 HValue* HOptimizedGraphBuilder::BuildNamedAccess(
     PropertyAccessType access, BailoutId ast_id, BailoutId return_id,
     Expression* expr, FeedbackVectorSlot slot, HValue* object,
-    Handle<String> name, HValue* value, bool is_uninitialized) {
+    Handle<Name> name, HValue* value, bool is_uninitialized) {
   SmallMapList* maps;
   ComputeReceiverTypes(expr, object, &maps, zone());
   DCHECK(maps != NULL);

src/crankshaft/hydrogen.h

@@ -2493,7 +2493,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   void HandlePolymorphicNamedFieldAccess(
       PropertyAccessType access_type, Expression* expr, FeedbackVectorSlot slot,
       BailoutId ast_id, BailoutId return_id, HValue* object, HValue* value,
-      SmallMapList* types, Handle<String> name);
+      SmallMapList* types, Handle<Name> name);
 
   HValue* BuildAllocateExternalElements(
       ExternalArrayType array_type,
@@ -2743,7 +2743,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   HValue* BuildNamedAccess(PropertyAccessType access, BailoutId ast_id,
                            BailoutId reutrn_id, Expression* expr,
                            FeedbackVectorSlot slot, HValue* object,
-                           Handle<String> name, HValue* value,
+                           Handle<Name> name, HValue* value,
                            bool is_uninitialized = false);
 
   void HandlePolymorphicCallNamed(Call* expr,

src/type-feedback-vector.cc

@@ -13,6 +13,13 @@
 namespace v8 {
 namespace internal {
 
+
+static bool IsPropertyNameFeedback(Object* feedback) {
+  return feedback->IsString() ||
+         (feedback->IsSymbol() && !Symbol::cast(feedback)->is_private());
+}
+
+
 std::ostream& operator<<(std::ostream& os, FeedbackVectorSlotKind kind) {
   return os << TypeFeedbackMetadata::Kind2String(kind);
 }
@@ -650,9 +657,10 @@ void KeyedStoreICNexus::ConfigurePolymorphic(MapHandleList* maps,
 int FeedbackNexus::ExtractMaps(MapHandleList* maps) const {
   Isolate* isolate = GetIsolate();
   Object* feedback = GetFeedback();
-  if (feedback->IsFixedArray() || feedback->IsString()) {
+  bool is_named_feedback = IsPropertyNameFeedback(feedback);
+  if (feedback->IsFixedArray() || is_named_feedback) {
     int found = 0;
-    if (feedback->IsString()) {
+    if (is_named_feedback) {
       feedback = GetFeedbackExtra();
     }
     FixedArray* array = FixedArray::cast(feedback);
@@ -687,8 +695,9 @@ int FeedbackNexus::ExtractMaps(MapHandleList* maps) const {
 
 MaybeHandle<Code> FeedbackNexus::FindHandlerForMap(Handle<Map> map) const {
   Object* feedback = GetFeedback();
-  if (feedback->IsFixedArray() || feedback->IsString()) {
-    if (feedback->IsString()) {
+  bool is_named_feedback = IsPropertyNameFeedback(feedback);
+  if (feedback->IsFixedArray() || is_named_feedback) {
+    if (is_named_feedback) {
       feedback = GetFeedbackExtra();
     }
     FixedArray* array = FixedArray::cast(feedback);
@@ -725,8 +734,9 @@ MaybeHandle<Code> FeedbackNexus::FindHandlerForMap(Handle<Map> map) const {
 bool FeedbackNexus::FindHandlers(CodeHandleList* code_list, int length) const {
   Object* feedback = GetFeedback();
   int count = 0;
-  if (feedback->IsFixedArray() || feedback->IsString()) {
-    if (feedback->IsString()) {
+  bool is_named_feedback = IsPropertyNameFeedback(feedback);
+  if (feedback->IsFixedArray() || is_named_feedback) {
+    if (is_named_feedback) {
       feedback = GetFeedbackExtra();
     }
     FixedArray* array = FixedArray::cast(feedback);
@@ -770,7 +780,7 @@ void KeyedLoadICNexus::Clear(Code* host) {
 
 Name* KeyedLoadICNexus::FindFirstName() const {
   Object* feedback = GetFeedback();
-  if (feedback->IsString()) {
+  if (IsPropertyNameFeedback(feedback)) {
     return Name::cast(feedback);
   }
   return NULL;
@@ -779,7 +789,7 @@ Name* KeyedLoadICNexus::FindFirstName() const {
 
 Name* KeyedStoreICNexus::FindFirstName() const {
   Object* feedback = GetFeedback();
-  if (feedback->IsString()) {
+  if (IsPropertyNameFeedback(feedback)) {
     return Name::cast(feedback);
   }
   return NULL;

test/mjsunit/keyed-load-with-symbol-key.js

+// Copyright 2014 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
+var s = Symbol("foo");
+
+var o = {
+  [s]: "bar",
+}
+
+function get(obj, key) {
+  return obj[key];
+}
+
+assertEquals("bar", get(o, s));
+get(o, s);
+get(o, s);
+
+%OptimizeFunctionOnNextCall(get);
+assertEquals("bar", get(o, s));
+
+assertOptimized(get);
+
+get(o, "funny");
+assertUnoptimized(get);