Reland "Introduce a PREMONOMORPHIC state to StoreGlobalIC"

(Fixed test failure in lite-mode)

Introduce a PREMONOMORPHIC state to StoreGlobalIC

It's used rather narrowly for now -- only when we run into an
interceptor during the lookup. After the call to SetProperty, we know
more. That is, the interceptor was only there because it's a new
property, and the call to SetProperty ends up creating it.

By delaying the initialization of the IC, we recognize the (now)
created property, and can provide good feedback downstream to
TurboFan.

TBR=ishell@chromium.org

Bug: v8:8712
Change-Id: Ieb79dcf1354ee294ad0f479a4a6c41a77f389850
Reviewed-on: https://chromium-review.googlesource.com/c/1460955Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59483}

src/counters.h

@@ -999,6 +999,7 @@ class RuntimeCallTimer final {
   V(LoadIC_StringWrapperLength)                   \
   V(StoreGlobalIC_SlowStub)                       \
   V(StoreGlobalIC_StoreScriptContextField)        \
+  V(StoreGlobalIC_Premonomorphic)                 \
   V(StoreIC_HandlerCacheHit_Accessor)             \
   V(StoreIC_NonReceiver)                          \
   V(StoreIC_Premonomorphic)                       \

src/feedback-vector.cc

@@ -570,6 +570,13 @@ InlineCacheState FeedbackNexus::StateFromFeedback() const {
     case FeedbackSlotKind::kLoadGlobalInsideTypeof: {
       if (feedback->IsSmi()) return MONOMORPHIC;
 
+      if (feedback == MaybeObject::FromObject(
+                          *FeedbackVector::PremonomorphicSentinel(isolate))) {
+        DCHECK(kind() == FeedbackSlotKind::kStoreGlobalSloppy ||
+               kind() == FeedbackSlotKind::kStoreGlobalStrict);
+        return PREMONOMORPHIC;
+      }
+
       DCHECK(feedback->IsWeakOrCleared());
       MaybeObject extra = GetFeedbackExtra();
       if (!feedback->IsCleared() ||

src/ic/accessor-assembler.cc

@@ -2895,14 +2895,18 @@ void AccessorAssembler::StoreIC(const StoreICParameters* p) {
 }
 
 void AccessorAssembler::StoreGlobalIC(const StoreICParameters* pp) {
-  Label if_lexical_var(this), if_property_cell(this);
+  Label if_lexical_var(this), if_heapobject(this);
   TNode<MaybeObject> maybe_weak_ref =
       LoadFeedbackVectorSlot(pp->vector, pp->slot, 0, SMI_PARAMETERS);
-  Branch(TaggedIsSmi(maybe_weak_ref), &if_lexical_var, &if_property_cell);
+  Branch(TaggedIsSmi(maybe_weak_ref), &if_lexical_var, &if_heapobject);
 
-  BIND(&if_property_cell);
+  BIND(&if_heapobject);
   {
     Label try_handler(this), miss(this, Label::kDeferred);
+    GotoIf(
+        WordEqual(maybe_weak_ref, LoadRoot(RootIndex::kpremonomorphic_symbol)),
+        &miss);
+
     CSA_ASSERT(this, IsWeakOrCleared(maybe_weak_ref));
     TNode<PropertyCell> property_cell =
         CAST(GetHeapObjectAssumeWeak(maybe_weak_ref, &try_handler));

src/ic/ic.cc

@@ -1471,6 +1471,24 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
     }
     handler = ComputeHandler(lookup);
   } else {
+    if (state() == UNINITIALIZED && IsStoreGlobalIC() &&
+        lookup->state() == LookupIterator::INTERCEPTOR) {
+      InterceptorInfo info =
+          lookup->GetHolder<JSObject>()->GetNamedInterceptor();
+      if (!lookup->HolderIsReceiverOrHiddenPrototype() &&
+          !info->getter()->IsUndefined(isolate())) {
+        // Utilize premonomorphic state for global store ics that run into
+        // an interceptor because the property doesn't exist yet.
+        // After we actually set the property, we'll have more information.
+        // Premonomorphism gives us a chance to find more information the
+        // second time.
+        TRACE_HANDLER_STATS(isolate(), StoreGlobalIC_Premonomorphic);
+        ConfigureVectorState(receiver_map());
+        TraceIC("StoreGlobalIC", lookup->name());
+        return;
+      }
+    }
+
     set_slow_stub_reason("LookupForWrite said 'false'");
     // TODO(marja): change slow_stub to return MaybeObjectHandle.
     handler = MaybeObjectHandle(slow_stub());

test/cctest/test-api.cc

@@ -45,6 +45,8 @@
 #include "src/compilation-cache.h"
 #include "src/debug/debug.h"
 #include "src/execution.h"
+#include "src/feedback-vector-inl.h"
+#include "src/feedback-vector.h"
 #include "src/futex-emulation.h"
 #include "src/global-handles.h"
 #include "src/heap/incremental-marking.h"
@@ -10951,7 +10953,6 @@ static void ShadowIndexedGet(uint32_t index,
 static void ShadowNamedGet(Local<Name> key,
                            const v8::PropertyCallbackInfo<v8::Value>&) {}
 
-
 THREADED_TEST(ShadowObject) {
   shadow_y = shadow_y_setter_call_count = shadow_y_getter_call_count = 0;
   v8::Isolate* isolate = CcTest::isolate();
@@ -11002,6 +11003,58 @@ THREADED_TEST(ShadowObject) {
   CHECK_EQ(42, value->Int32Value(context.local()).FromJust());
 }
 
+THREADED_TEST(ShadowObjectAndDataProperty) {
+  // Lite mode doesn't make use of feedback vectors, which is what we
+  // want to ensure has the correct form.
+  if (i::FLAG_lite_mode) return;
+  // This test mimics the kind of shadow property the Chromium embedder
+  // uses for undeclared globals. The IC subsystem has special handling
+  // for this case, using a PREMONOMORPHIC state to delay entering
+  // MONOMORPHIC state until enough information is available to support
+  // efficient access and good feedback for optimization.
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope handle_scope(isolate);
+
+  Local<ObjectTemplate> global_template = v8::ObjectTemplate::New(isolate);
+  LocalContext context(nullptr, global_template);
+
+  Local<v8::FunctionTemplate> t = v8::FunctionTemplate::New(isolate);
+  t->InstanceTemplate()->SetHandler(
+      v8::NamedPropertyHandlerConfiguration(ShadowNamedGet));
+
+  Local<Value> o = t->GetFunction(context.local())
+                       .ToLocalChecked()
+                       ->NewInstance(context.local())
+                       .ToLocalChecked();
+  CHECK(context->Global()
+            ->Set(context.local(), v8_str("__proto__"), o)
+            .FromJust());
+
+  CompileRun(
+      "function foo(x) { i = x; }"
+      "foo(0)");
+
+  i::Handle<i::JSFunction> foo(i::Handle<i::JSFunction>::cast(
+      v8::Utils::OpenHandle(*context->Global()
+                                 ->Get(context.local(), v8_str("foo"))
+                                 .ToLocalChecked())));
+  CHECK(foo->has_feedback_vector());
+  i::FeedbackSlot slot = i::FeedbackVector::ToSlot(0);
+  i::FeedbackNexus nexus(foo->feedback_vector(), slot);
+  CHECK_EQ(i::FeedbackSlotKind::kStoreGlobalSloppy, nexus.kind());
+  CHECK_EQ(i::PREMONOMORPHIC, nexus.StateFromFeedback());
+  CompileRun("foo(1)");
+  CHECK_EQ(i::MONOMORPHIC, nexus.StateFromFeedback());
+  // We go a bit further, checking that the form of monomorphism is
+  // a PropertyCell in the vector. This is because we want to make sure
+  // we didn't settle for a "poor man's monomorphism," such as a
+  // slow_stub bailout which would mean a trip to the runtime on all
+  // subsequent stores, and a lack of feedback for the optimizing
+  // compiler downstream.
+  i::HeapObject heap_object;
+  CHECK(nexus.GetFeedback().GetHeapObject(&heap_object));
+  CHECK(heap_object->IsPropertyCell());
+}
 
 THREADED_TEST(SetPrototype) {
   LocalContext context;