[ic] Don't transition to premonomorphic state

We used to use premonomorphic state to delay initializing the ICs.
This optimization was to avoid the cost of setting up handlers if the
code executed only once. With lazy feedback allocation we no longer
need this.

This cl also renames LoadIC_Uninitialized to LoadIC_Nofeedback and
StoreIC_Uninitialized to StoreIC_Nofeedback since we now miss to
runtime in the uninitialized state and use the builtin when there
is no feedback.


Change-Id: I1633e61ea74664da51348e362c34c47a017a264a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1683525
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63020}

src/builtins/builtins-definitions.h

@@ -222,9 +222,9 @@ namespace internal {
   TFH(LoadIC_Slow, LoadWithVector)                                             \
   TFH(LoadIC_StringLength, LoadWithVector)                                     \
   TFH(LoadIC_StringWrapperLength, LoadWithVector)                              \
-  TFH(LoadIC_Uninitialized, LoadWithVector)                                    \
+  TFH(LoadIC_Nofeedback, Load)                                                 \
   TFH(StoreGlobalIC_Slow, StoreWithVector)                                     \
-  TFH(StoreIC_Uninitialized, StoreWithVector)                                  \
+  TFH(StoreIC_Nofeedback, Store)                                               \
   TFH(StoreInArrayLiteralIC_Slow, StoreWithVector)                             \
   TFH(KeyedLoadIC_SloppyArguments, LoadWithVector)                             \
   TFH(LoadIndexedInterceptorIC, LoadWithVector)                                \

src/builtins/builtins-handler-gen.cc

@@ -66,9 +66,9 @@ void Builtins::Generate_KeyedStoreIC_Megamorphic(
   KeyedStoreGenericGenerator::Generate(state);
 }
 
-void Builtins::Generate_StoreIC_Uninitialized(
+void Builtins::Generate_StoreIC_Nofeedback(
     compiler::CodeAssemblerState* state) {
-  StoreICUninitializedGenerator::Generate(state);
+  StoreICNofeedbackGenerator::Generate(state);
 }
 
 // TODO(mythria): Check if we can remove feedback vector and slot parameters in

src/builtins/builtins-ic-gen.cc

@@ -23,7 +23,7 @@ namespace internal {
 IC_BUILTIN(LoadIC)
 IC_BUILTIN(LoadIC_Megamorphic)
 IC_BUILTIN(LoadIC_Noninlined)
-IC_BUILTIN(LoadIC_Uninitialized)
+IC_BUILTIN(LoadIC_Nofeedback)
 IC_BUILTIN(LoadICTrampoline)
 IC_BUILTIN(LoadICTrampoline_Megamorphic)
 IC_BUILTIN(KeyedLoadIC)

src/ic/accessor-assembler.cc

@@ -2524,8 +2524,8 @@ void AccessorAssembler::LoadIC_BytecodeHandler(const LazyLoadICParameters* p,
     Comment("LoadIC_BytecodeHandler_nofeedback");
     // Call into the stub that implements the non-inlined parts of LoadIC.
     exit_point->ReturnCallStub(
-        Builtins::CallableFor(isolate(), Builtins::kLoadIC_Uninitialized),
-        p->context(), p->receiver(), p->name(), p->slot(), p->vector());
+        Builtins::CallableFor(isolate(), Builtins::kLoadIC_Nofeedback),
+        p->context(), p->receiver(), p->name(), p->slot());
   }
 
   BIND(&miss);
@@ -2589,8 +2589,6 @@ void AccessorAssembler::LoadIC_Noninlined(const LoadICParameters* p,
                                           TVariable<MaybeObject>* var_handler,
                                           Label* if_handler, Label* miss,
                                           ExitPoint* exit_point) {
-  Label try_uninitialized(this, Label::kDeferred);
-
   // Neither deprecated map nor monomorphic. These cases are handled in the
   // bytecode handler.
   CSA_ASSERT(this, Word32BinaryNot(IsDeprecatedMap(receiver_map)));
@@ -2601,41 +2599,20 @@ void AccessorAssembler::LoadIC_Noninlined(const LoadICParameters* p,
   {
     // Check megamorphic case.
     GotoIfNot(WordEqual(feedback, LoadRoot(RootIndex::kmegamorphic_symbol)),
-              &try_uninitialized);
+              miss);
 
     TryProbeStubCache(isolate()->load_stub_cache(), p->receiver(), p->name(),
                       if_handler, var_handler, miss);
   }
-
-  BIND(&try_uninitialized);
-  {
-    // Check uninitialized case.
-    GotoIfNot(WordEqual(feedback, LoadRoot(RootIndex::kuninitialized_symbol)),
-              miss);
-    exit_point->ReturnCallStub(
-        Builtins::CallableFor(isolate(), Builtins::kLoadIC_Uninitialized),
-        p->context(), p->receiver(), p->name(), p->slot(), p->vector());
-  }
 }
 
-void AccessorAssembler::LoadIC_Uninitialized(const LoadICParameters* p) {
-  Label miss(this, Label::kDeferred),
-      check_function_prototype(this);
+void AccessorAssembler::LoadIC_Nofeedback(const LoadICParameters* p) {
+  Label miss(this, Label::kDeferred);
   Node* receiver = p->receiver();
   GotoIf(TaggedIsSmi(receiver), &miss);
   Node* receiver_map = LoadMap(receiver);
   Node* instance_type = LoadMapInstanceType(receiver_map);
 
-  GotoIf(IsUndefined(p->vector()), &check_function_prototype);
-  // Optimistically write the state transition to the vector.
-  StoreFeedbackVectorSlot(p->vector(), p->slot(),
-                          LoadRoot(RootIndex::kpremonomorphic_symbol),
-                          SKIP_WRITE_BARRIER, 0, SMI_PARAMETERS);
-  StoreWeakReferenceInFeedbackVector(p->vector(), p->slot(), receiver_map,
-                                     kTaggedSize, SMI_PARAMETERS);
-  Goto(&check_function_prototype);
-
-  BIND(&check_function_prototype);
   {
     // Special case for Function.prototype load, because it's very common
     // for ICs that are only executed once (MyFunc.prototype.foo = ...).
@@ -2655,15 +2632,6 @@ void AccessorAssembler::LoadIC_Uninitialized(const LoadICParameters* p) {
 
   BIND(&miss);
   {
-    Label call_runtime(this, Label::kDeferred);
-    GotoIf(IsUndefined(p->vector()), &call_runtime);
-    // Undo the optimistic state transition.
-    StoreFeedbackVectorSlot(p->vector(), p->slot(),
-                            LoadRoot(RootIndex::kuninitialized_symbol),
-                            SKIP_WRITE_BARRIER, 0, SMI_PARAMETERS);
-    Goto(&call_runtime);
-
-    BIND(&call_runtime);
     TailCallRuntime(Runtime::kLoadIC_Miss, p->context(), p->receiver(),
                     p->name(), p->slot(), p->vector());
   }
@@ -2772,6 +2740,7 @@ void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p,
   TVARIABLE(MaybeObject, var_handler);
   Label if_handler(this, &var_handler), try_polymorphic(this, Label::kDeferred),
       try_megamorphic(this, Label::kDeferred),
+      try_uninitialized(this, Label::kDeferred),
       try_polymorphic_name(this, Label::kDeferred),
       miss(this, Label::kDeferred), generic(this, Label::kDeferred);
 
@@ -2808,7 +2777,7 @@ void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p,
     // Check megamorphic case.
     Comment("KeyedLoadIC_try_megamorphic");
     Branch(WordEqual(strong_feedback, LoadRoot(RootIndex::kmegamorphic_symbol)),
-           &generic, &try_polymorphic_name);
+           &generic, &try_uninitialized);
   }
 
   BIND(&generic);
@@ -2821,6 +2790,15 @@ void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p,
                     p->vector());
   }
 
+  BIND(&try_uninitialized);
+  {
+    // Check uninitialized case.
+    Comment("KeyedLoadIC_try_uninitialized");
+    Branch(
+        WordEqual(strong_feedback, LoadRoot(RootIndex::kuninitialized_symbol)),
+        &miss, &try_polymorphic_name);
+  }
+
   BIND(&try_polymorphic_name);
   {
     // We might have a name in feedback, and a weak fixed array in the next
@@ -3014,8 +2992,7 @@ void AccessorAssembler::StoreIC(const StoreICParameters* p) {
   Label if_handler(this, &var_handler),
       if_handler_from_stub_cache(this, &var_handler, Label::kDeferred),
       try_polymorphic(this, Label::kDeferred),
-      try_megamorphic(this, Label::kDeferred),
-      try_uninitialized(this, Label::kDeferred), miss(this, Label::kDeferred),
+      try_megamorphic(this, Label::kDeferred), miss(this, Label::kDeferred),
       no_feedback(this, Label::kDeferred);
 
   Node* receiver_map = LoadReceiverMap(p->receiver());
@@ -3049,24 +3026,16 @@ void AccessorAssembler::StoreIC(const StoreICParameters* p) {
     // Check megamorphic case.
     GotoIfNot(
         WordEqual(strong_feedback, LoadRoot(RootIndex::kmegamorphic_symbol)),
-        &try_uninitialized);
+        &miss);
 
     TryProbeStubCache(isolate()->store_stub_cache(), p->receiver(), p->name(),
                       &if_handler, &var_handler, &miss);
   }
-  BIND(&try_uninitialized);
-  {
-    // Check uninitialized case.
-    Branch(
-        WordEqual(strong_feedback, LoadRoot(RootIndex::kuninitialized_symbol)),
-        &no_feedback, &miss);
-  }
 
   BIND(&no_feedback);
   {
-    TailCallBuiltin(Builtins::kStoreIC_Uninitialized, p->context(),
-                    p->receiver(), p->name(), p->value(), p->slot(),
-                    p->vector());
+    TailCallBuiltin(Builtins::kStoreIC_Nofeedback, p->context(), p->receiver(),
+                    p->name(), p->value(), p->slot());
   }
 
   BIND(&miss);
@@ -3085,6 +3054,10 @@ void AccessorAssembler::StoreGlobalIC(const StoreICParameters* pp) {
   BIND(&if_heapobject);
   {
     Label try_handler(this), miss(this, Label::kDeferred);
+    // We use pre-monomorphic state for global stores that run into
+    // interceptors because the property doesn't exist yet. Using
+    // pre-monomorphic state gives it a chance to find more information the
+    // second time.
     GotoIf(
         WordEqual(maybe_weak_ref, LoadRoot(RootIndex::kpremonomorphic_symbol)),
         &miss);
@@ -3432,17 +3405,16 @@ void AccessorAssembler::GenerateLoadIC_Noninlined() {
                                 slot, vector);
 }
 
-void AccessorAssembler::GenerateLoadIC_Uninitialized() {
-  using Descriptor = LoadWithVectorDescriptor;
+void AccessorAssembler::GenerateLoadIC_Nofeedback() {
+  using Descriptor = LoadDescriptor;
 
   Node* receiver = Parameter(Descriptor::kReceiver);
   Node* name = Parameter(Descriptor::kName);
   Node* slot = Parameter(Descriptor::kSlot);
-  Node* vector = Parameter(Descriptor::kVector);
   TNode<Context> context = CAST(Parameter(Descriptor::kContext));
 
-  LoadICParameters p(context, receiver, name, slot, vector);
-  LoadIC_Uninitialized(&p);
+  LoadICParameters p(context, receiver, name, slot, UndefinedConstant());
+  LoadIC_Nofeedback(&p);
 }
 
 void AccessorAssembler::GenerateLoadICTrampoline() {

src/ic/accessor-assembler.h

@@ -30,7 +30,7 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
   void GenerateLoadIC();
   void GenerateLoadIC_Megamorphic();
   void GenerateLoadIC_Noninlined();
-  void GenerateLoadIC_Uninitialized();
+  void GenerateLoadIC_Nofeedback();
   void GenerateLoadICTrampoline();
   void GenerateLoadICTrampoline_Megamorphic();
   void GenerateKeyedLoadIC();
@@ -214,7 +214,7 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
   TNode<MaybeObject> LoadDescriptorValueOrFieldType(
       TNode<Map> map, TNode<IntPtrT> descriptor_entry);
 
-  void LoadIC_Uninitialized(const LoadICParameters* p);
+  void LoadIC_Nofeedback(const LoadICParameters* p);
 
   void KeyedLoadIC(const LoadICParameters* p, LoadAccessMode access_mode);
   void KeyedLoadICGeneric(const LoadICParameters* p);

src/ic/ic.cc

@@ -400,10 +400,8 @@ MaybeHandle<Object> LoadIC::Load(Handle<Object> object, Handle<Name> name) {
 
   if (MigrateDeprecated(isolate(), object)) use_ic = false;
 
-  if (state() != UNINITIALIZED) {
-    JSObject::MakePrototypesFast(object, kStartAtReceiver, isolate());
-    update_receiver_map(object);
-  }
+  JSObject::MakePrototypesFast(object, kStartAtReceiver, isolate());
+  update_receiver_map(object);
 
   LookupIterator it(isolate(), object, name);
 
@@ -648,15 +646,6 @@ void IC::PatchCache(Handle<Name> name, const MaybeObjectHandle& handler) {
 }
 
 void LoadIC::UpdateCaches(LookupIterator* lookup) {
-  if (state() == UNINITIALIZED && !IsLoadGlobalIC()) {
-    // This is the first time we execute this inline cache. Set the target to
-    // the pre monomorphic stub to delay setting the monomorphic state.
-    TRACE_HANDLER_STATS(isolate(), LoadIC_Premonomorphic);
-    ConfigureVectorState(receiver_map());
-    TraceIC("LoadIC", lookup->name());
-    return;
-  }
-
   Handle<Object> code;
   if (lookup->state() == LookupIterator::ACCESS_CHECK) {
     code = slow_stub();
@@ -1415,9 +1404,7 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> name,
     return TypeError(MessageTemplate::kNonObjectPropertyStore, object, name);
   }
 
-  if (state() != UNINITIALIZED) {
-    JSObject::MakePrototypesFast(object, kStartAtPrototype, isolate());
-  }
+  JSObject::MakePrototypesFast(object, kStartAtPrototype, isolate());
   LookupIterator it(isolate(), object, name);
 
   if (name->IsPrivate()) {
@@ -1442,15 +1429,6 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object, Handle<Name> name,
 
 void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
                            StoreOrigin store_origin) {
-  if (state() == UNINITIALIZED && !IsStoreGlobalIC()) {
-    // This is the first time we execute this inline cache. Transition
-    // to premonomorphic state to delay setting the monomorphic state.
-    TRACE_HANDLER_STATS(isolate(), StoreIC_Premonomorphic);
-    ConfigureVectorState(receiver_map());
-    TraceIC("StoreIC", lookup->name());
-    return;
-  }
-
   MaybeObjectHandle handler;
   if (LookupForWrite(lookup, value, store_origin)) {
     if (IsStoreGlobalIC()) {
@@ -1810,10 +1788,8 @@ void KeyedStoreIC::UpdateStoreElement(Handle<Map> receiver_map,
   handlers.reserve(target_receiver_maps.size());
   StoreElementPolymorphicHandlers(&target_receiver_maps, &handlers, store_mode);
   if (target_receiver_maps.size() == 0) {
-    // Transition to PREMONOMORPHIC state here and remember a weak-reference
-    // to the {receiver_map} in case TurboFan sees this function before the
-    // IC can transition further.
-    ConfigureVectorState(receiver_map);
+    Handle<Object> handler = StoreElementHandler(receiver_map, store_mode);
+    ConfigureVectorState(Handle<Name>(), receiver_map, handler);
   } else if (target_receiver_maps.size() == 1) {
     ConfigureVectorState(Handle<Name>(), target_receiver_maps[0], handlers[0]);
   } else {

src/ic/keyed-store-generic.cc

@@ -30,7 +30,7 @@ class KeyedStoreGenericAssembler : public AccessorAssembler {
 
   void KeyedStoreGeneric();
 
-  void StoreIC_Uninitialized();
+  void StoreIC_Nofeedback();
 
   // Generates code for [[Set]] operation, the |unique_name| is supposed to be
   // unique otherwise this code will always go to runtime.
@@ -138,10 +138,9 @@ void KeyedStoreGenericGenerator::Generate(compiler::CodeAssemblerState* state) {
   assembler.KeyedStoreGeneric();
 }
 
-void StoreICUninitializedGenerator::Generate(
-    compiler::CodeAssemblerState* state) {
+void StoreICNofeedbackGenerator::Generate(compiler::CodeAssemblerState* state) {
   KeyedStoreGenericAssembler assembler(state, StoreMode::kOrdinary);
-  assembler.StoreIC_Uninitialized();
+  assembler.StoreIC_Nofeedback();
 }
 
 void KeyedStoreGenericGenerator::SetProperty(
@@ -1043,14 +1042,13 @@ void KeyedStoreGenericAssembler::SetProperty(TNode<Context> context,
   KeyedStoreGeneric(context, receiver, key, value, Just(language_mode));
 }
 
-void KeyedStoreGenericAssembler::StoreIC_Uninitialized() {
-  using Descriptor = StoreWithVectorDescriptor;
+void KeyedStoreGenericAssembler::StoreIC_Nofeedback() {
+  using Descriptor = StoreDescriptor;
 
   Node* receiver = Parameter(Descriptor::kReceiver);
   Node* name = Parameter(Descriptor::kName);
   Node* value = Parameter(Descriptor::kValue);
   Node* slot = Parameter(Descriptor::kSlot);
-  Node* vector = Parameter(Descriptor::kVector);
   Node* context = Parameter(Descriptor::kContext);
 
   Label miss(this, Label::kDeferred), store_property(this);
@@ -1061,33 +1059,16 @@ void KeyedStoreGenericAssembler::StoreIC_Uninitialized() {
   // Receivers requiring non-standard element accesses (interceptors, access
   // checks, strings and string wrappers, proxies) are handled in the runtime.
   GotoIf(IsSpecialReceiverInstanceType(instance_type), &miss);
-
-  // Optimistically write the state transition to the vector.
-  GotoIf(IsUndefined(vector), &store_property);
-  StoreFeedbackVectorSlot(vector, slot,
-                          LoadRoot(RootIndex::kpremonomorphic_symbol),
-                          SKIP_WRITE_BARRIER, 0, SMI_PARAMETERS);
-  Goto(&store_property);
-
-  BIND(&store_property);
   {
-    StoreICParameters p(CAST(context), receiver, name, value, slot, vector);
+    StoreICParameters p(CAST(context), receiver, name, value, slot,
+                        UndefinedConstant());
     EmitGenericPropertyStore(receiver, receiver_map, &p, &miss);
   }
 
   BIND(&miss);
   {
-    Label call_runtime(this);
-    // Undo the optimistic state transition.
-    GotoIf(IsUndefined(vector), &call_runtime);
-    StoreFeedbackVectorSlot(vector, slot,
-                            LoadRoot(RootIndex::kuninitialized_symbol),
-                            SKIP_WRITE_BARRIER, 0, SMI_PARAMETERS);
-    Goto(&call_runtime);
-
-    BIND(&call_runtime);
-    TailCallRuntime(Runtime::kStoreIC_Miss, context, value, slot, vector,
-                    receiver, name);
+    TailCallRuntime(Runtime::kStoreIC_Miss, context, value, slot,
+                    UndefinedConstant(), receiver, name);
   }
 }
 

src/ic/keyed-store-generic.h

@@ -37,7 +37,7 @@ class KeyedStoreGenericGenerator {
                                    TNode<Object> value);
 };
 
-class StoreICUninitializedGenerator {
+class StoreICNofeedbackGenerator {
  public:
   static void Generate(compiler::CodeAssemblerState* state);
 };

test/cctest/test-feedback-vector.cc

@@ -438,9 +438,7 @@ TEST(VectorLoadICStates) {
       Handle<FeedbackVector>(f->feedback_vector(), isolate);
   FeedbackSlot slot(0);
   FeedbackNexus nexus(feedback_vector, slot);
-  CHECK_EQ(PREMONOMORPHIC, nexus.ic_state());
 
-  CompileRun("f(o)");
   CHECK_EQ(MONOMORPHIC, nexus.ic_state());
   // Verify that the monomorphic map is the one we expect.
   v8::MaybeLocal<v8::Value> v8_o =
@@ -526,16 +524,13 @@ TEST(VectorLoadICOnSmi) {
   CompileRun(
       "var o = { foo: 3 };"
       "%EnsureFeedbackVectorForFunction(f);"
-      "function f(a) { return a.foo; } f(o);");
+      "function f(a) { return a.foo; } f(34);");
   Handle<JSFunction> f = GetFunction("f");
   // There should be one IC.
   Handle<FeedbackVector> feedback_vector =
       Handle<FeedbackVector>(f->feedback_vector(), isolate);
   FeedbackSlot slot(0);
   FeedbackNexus nexus(feedback_vector, slot);
-  CHECK_EQ(PREMONOMORPHIC, nexus.ic_state());
-
-  CompileRun("f(34)");
   CHECK_EQ(MONOMORPHIC, nexus.ic_state());
   // Verify that the monomorphic map is the one we expect.
   Map number_map = ReadOnlyRoots(heap).heap_number_map();

test/mjsunit/es6/large-classes-properties.js

@@ -8,14 +8,14 @@
   // This is to test for dictionary mode when there more than
   // kMaxNumberOfDescriptors (1024) properties.
   const kLimit = 1030;
-  let evalString = "(function(i) { " +
+  let evalString = "function f(i) { " +
       "let clazz = class { " +
       "   constructor(i) { this.value = i;";
   for (let i = 0; i < kLimit ; i++) {
     evalString  += "this.property"+i +" = "+i+"; "
   }
   evalString += "}};" +
-      " return (new clazz(i)); })";
+      " return (new clazz(i)); }; f;";
 
   let fn = eval(evalString);
   %PrepareFunctionForOptimization(fn);