[ic] Internalize strings on the fly in KeyedLoadICGeneric.

This turns on the existing --internalize_on_the_fly flag for the
MEGAMORPHIC KeyedLoadIC to properly internalize strings before
looking up the property. This avoids the otherwise taken runtime
call to %KeyedGetProperty, which is definitely slower.

Initially the --internalize_on_the_fly flag was turned off because
internalizing strings on the fly causes too much traffic on the
megamorphic stub cache. We avoid this problem here by not probing
the stub cache in that case, which still gives the benefit of not
having to go to the runtime.

This improves the babylon test on the web-tooling-benchmark by around
2-3% and will probably also help with several tests (like React or
Ember) on the Speedometer benchmark.

If this CL causes trouble (i.e. tanks something important), we can
just turn off the --internalize_on_the_fly flag again.

Bug: v8:6936, v8:7026
Change-Id: If295ed3fd013f8b0ff031f9979e7df21dab817b6
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/751464Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49093}

src/flag-definitions.h

@@ -304,7 +304,7 @@ DEFINE_BOOL(trace_block_coverage, false,
 DEFINE_BOOL(feedback_normalization, false,
             "feed back normalization to constructors")
 // TODO(jkummerow): This currently adds too much load on the stub cache.
-DEFINE_BOOL_READONLY(internalize_on_the_fly, false,
+DEFINE_BOOL_READONLY(internalize_on_the_fly, true,
                      "internalize string keys for generic keyed ICs on the fly")
 
 // Flags for optimization types.

src/ic/accessor-assembler.cc

@@ -2400,14 +2400,26 @@ void AccessorAssembler::KeyedLoadICGeneric(const LoadICParameters* p) {
   BIND(&if_notunique);
   {
     if (FLAG_internalize_on_the_fly) {
-      Label not_in_string_table(this);
-      TryInternalizeString(p->name, &if_index, &var_index, &if_unique_name,
-                           &var_unique, &not_in_string_table, &slow);
-
-      BIND(&not_in_string_table);
-      // If the string was not found in the string table, then no object can
-      // have a property with that name.
-      Return(UndefinedConstant());
+      // Ideally we could return undefined directly here if the name is not
+      // found in the string table, i.e. it was never internalized, but that
+      // invariant doesn't hold with named property interceptors (at this
+      // point), so we take the {slow} path instead.
+      Label if_in_string_table(this);
+      TryInternalizeString(p->name, &if_index, &var_index, &if_in_string_table,
+                           &var_unique, &slow, &slow);
+
+      BIND(&if_in_string_table);
+      {
+        // TODO(bmeurer): We currently use a version of GenericPropertyLoad
+        // here, where we don't try to probe the megamorphic stub cache after
+        // successfully internalizing the incoming string. Past experiments
+        // with this have shown that it causes too much traffic on the stub
+        // cache. We may want to re-evaluate that in the future.
+        LoadICParameters pp = *p;
+        pp.name = var_unique.value();
+        GenericPropertyLoad(receiver, receiver_map, instance_type, &pp, &slow,
+                            kDontUseStubCache);
+      }
     } else {
       Goto(&slow);
     }

test/mjsunit/regress/regress-7026.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+function foo(o, k) { return o[k]; }
+
+const a = "a";
+foo([1], 0);
+foo({a:1}, a);
+
+const p = new Proxy({}, {
+  get(target, name) {
+    return name;
+  }
+});
+
+assertEquals(a + "b", foo(p, a + "b"));