Use existing global cell status as a hint when generating loads.

Review URL: http://codereview.chromium.org/3537003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5569 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/ic-arm.cc

@@ -969,7 +969,8 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
 
 bool LoadIC::PatchInlinedContextualLoad(Address address,
                                         Object* map,
-                                        Object* cell) {
+                                        Object* cell,
+                                        bool is_dont_delete) {
   // TODO(<bug#>): implement this.
   return false;
 }

src/ia32/codegen-ia32.cc

@@ -9149,7 +9149,8 @@ class DeferredReferenceGetNamedValue: public DeferredCode {
       : dst_(dst),
         receiver_(receiver),
         name_(name),
-        is_contextual_(is_contextual) {
+        is_contextual_(is_contextual),
+        is_dont_delete_(false) {
     set_comment(is_contextual
                 ? "[ DeferredReferenceGetNamedValue (contextual)"
                 : "[ DeferredReferenceGetNamedValue");
@@ -9159,12 +9160,18 @@ class DeferredReferenceGetNamedValue: public DeferredCode {
 
   Label* patch_site() { return &patch_site_; }
 
+  void set_is_dont_delete(bool value) {
+    ASSERT(is_contextual_);
+    is_dont_delete_ = value;
+  }
+
  private:
   Label patch_site_;
   Register dst_;
   Register receiver_;
   Handle<String> name_;
   bool is_contextual_;
+  bool is_dont_delete_;
 };
 
 
@@ -9181,8 +9188,8 @@ void DeferredReferenceGetNamedValue::Generate() {
   // The call must be followed by:
   // - a test eax instruction to indicate that the inobject property
   //   case was inlined.
-  // - a mov ecx instruction to indicate that the contextual property
-  //   load was inlined.
+  // - a mov ecx or mov edx instruction to indicate that the
+  //   contextual property load was inlined.
   //
   // Store the delta to the map check instruction here in the test
   // instruction.  Use masm_-> instead of the __ macro since the
@@ -9191,8 +9198,11 @@ void DeferredReferenceGetNamedValue::Generate() {
   // Here we use masm_-> instead of the __ macro because this is the
   // instruction that gets patched and coverage code gets in the way.
   if (is_contextual_) {
-    masm_->mov(ecx, -delta_to_patch_site);
+    masm_->mov(is_dont_delete_ ? edx : ecx, -delta_to_patch_site);
     __ IncrementCounter(&Counters::named_load_global_inline_miss, 1);
+    if (is_dont_delete_) {
+      __ IncrementCounter(&Counters::dont_delete_hint_miss, 1);
+    }
   } else {
     masm_->test(eax, Immediate(-delta_to_patch_site));
     __ IncrementCounter(&Counters::named_load_inline_miss, 1);
@@ -9436,9 +9446,34 @@ Result CodeGenerator::EmitNamedLoad(Handle<String> name, bool is_contextual) {
       }
       __ mov(result.reg(),
              FieldOperand(result.reg(), JSGlobalPropertyCell::kValueOffset));
-      __ cmp(result.reg(), Factory::the_hole_value());
-      deferred->Branch(equal);
+      bool is_dont_delete = false;
+      if (!info_->closure().is_null()) {
+        // When doing lazy compilation we can check if the global cell
+        // already exists and use its "don't delete" status as a hint.
+        AssertNoAllocation no_gc;
+        v8::internal::GlobalObject* global_object =
+            info_->closure()->context()->global();
+        LookupResult lookup;
+        global_object->LocalLookupRealNamedProperty(*name, &lookup);
+        if (lookup.IsProperty() && lookup.type() == NORMAL) {
+          ASSERT(lookup.holder() == global_object);
+          ASSERT(global_object->property_dictionary()->ValueAt(
+              lookup.GetDictionaryEntry())->IsJSGlobalPropertyCell());
+          is_dont_delete = lookup.IsDontDelete();
+        }
+      }
+      deferred->set_is_dont_delete(is_dont_delete);
+      if (!is_dont_delete) {
+        __ cmp(result.reg(), Factory::the_hole_value());
+        deferred->Branch(equal);
+      } else if (FLAG_debug_code) {
+        __ cmp(result.reg(), Factory::the_hole_value());
+        __ Check(not_equal, "DontDelete cells can't contain the hole");
+      }
       __ IncrementCounter(&Counters::named_load_global_inline, 1);
+      if (is_dont_delete) {
+        __ IncrementCounter(&Counters::dont_delete_hint_hit, 1);
+      }
     } else {
       // The initial (invalid) offset has to be large enough to force a 32-bit
       // instruction encoding to allow patching with an arbitrary offset.  Use

src/ia32/ic-ia32.cc

@@ -1662,17 +1662,37 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
 
 
 // One byte opcode for mov ecx,0xXXXXXXXX.
+// Marks inlined contextual loads using all kinds of cells. Generated
+// code has the hole check:
+//   mov reg, <cell>
+//   mov reg, (<cell>, value offset)
+//   cmp reg, <the hole>
+//   je  slow
+//   ;; use reg
 static const byte kMovEcxByte = 0xB9;
 
+// One byte opcode for mov edx,0xXXXXXXXX.
+// Marks inlined contextual loads using only "don't delete"
+// cells. Generated code doesn't have the hole check:
+//   mov reg, <cell>
+//   mov reg, (<cell>, value offset)
+//   ;; use reg
+static const byte kMovEdxByte = 0xBA;
+
 bool LoadIC::PatchInlinedContextualLoad(Address address,
                                         Object* map,
-                                        Object* cell) {
+                                        Object* cell,
+                                        bool is_dont_delete) {
   // The address of the instruction following the call.
   Address mov_instruction_address =
       address + Assembler::kCallTargetAddressOffset;
-  // If the instruction following the call is not a cmp eax, nothing
-  // was inlined.
-  if (*mov_instruction_address != kMovEcxByte) return false;
+  // If the instruction following the call is not a mov ecx/edx,
+  // nothing was inlined.
+  byte b = *mov_instruction_address;
+  if (b != kMovEcxByte && b != kMovEdxByte) return false;
+  // If we don't have the hole check generated, we can only support
+  // "don't delete" cells.
+  if (b == kMovEdxByte && !is_dont_delete) return false;
 
   Address delta_address = mov_instruction_address + 1;
   // The delta to the start of the map check instruction.

src/ic.cc

@@ -299,7 +299,10 @@ void LoadIC::ClearInlinedVersion(Address address) {
   // present) to guarantee failure by holding an invalid map (the null
   // value).  The offset can be patched to anything.
   PatchInlinedLoad(address, Heap::null_value(), 0);
-  PatchInlinedContextualLoad(address, Heap::null_value(), Heap::null_value());
+  PatchInlinedContextualLoad(address,
+                             Heap::null_value(),
+                             Heap::null_value(),
+                             true);
 }
 
 
@@ -848,7 +851,10 @@ Object* LoadIC::Load(State state, Handle<Object> object, Handle<String> name) {
     JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(
         lookup.holder()->property_dictionary()->ValueAt(
             lookup.GetDictionaryEntry()));
-    if (PatchInlinedContextualLoad(address(), map, cell)) {
+    if (PatchInlinedContextualLoad(address(),
+                                   map,
+                                   cell,
+                                   lookup.IsDontDelete())) {
       set_target(megamorphic_stub());
       TRACE_IC_NAMED("[LoadIC : inline contextual patch %s]\n", name);
       ASSERT(cell->value() != Heap::the_hole_value());

src/ic.h

@@ -300,7 +300,8 @@ class LoadIC: public IC {
 
   static bool PatchInlinedContextualLoad(Address address,
                                          Object* map,
-                                         Object* cell);
+                                         Object* cell,
+                                         bool is_dont_delete);
 
   friend class IC;
 };

src/v8-counters.h

@@ -161,6 +161,8 @@ namespace internal {
   SC(named_load_inline_miss, V8.NamedLoadInlineMiss)                  \
   SC(named_load_global_inline, V8.NamedLoadGlobalInline)              \
   SC(named_load_global_inline_miss, V8.NamedLoadGlobalInlineMiss)     \
+  SC(dont_delete_hint_hit, V8.DontDeleteHintHit)                      \
+  SC(dont_delete_hint_miss, V8.DontDeleteHintMiss)                    \
   SC(named_load_global_stub, V8.NamedLoadGlobalStub)                  \
   SC(named_load_global_stub_miss, V8.NamedLoadGlobalStubMiss)         \
   SC(keyed_store_field, V8.KeyedStoreField)                           \

src/x64/ic-x64.cc

@@ -1729,7 +1729,8 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
 
 bool LoadIC::PatchInlinedContextualLoad(Address address,
                                         Object* map,
-                                        Object* cell) {
+                                        Object* cell,
+                                        bool is_dont_delete) {
   // TODO(<bug#>): implement this.
   return false;
 }

test/cctest/test-api.cc

@@ -8099,7 +8099,7 @@ static int GetGlobalObjectsCount() {
 }
 
 
-static int GetSurvivingGlobalObjectsCount() {
+static void CheckSurvivingGlobalObjectsCount(int expected) {
   // We need to collect all garbage twice to be sure that everything
   // has been collected.  This is because inline caches are cleared in
   // the first garbage collection but some of the maps have already
@@ -8109,9 +8109,9 @@ static int GetSurvivingGlobalObjectsCount() {
   i::Heap::CollectAllGarbage(false);
   int count = GetGlobalObjectsCount();
 #ifdef DEBUG
-  if (count > 0) i::Heap::TracePathToGlobal();
+  if (count != expected) i::Heap::TracePathToGlobal();
 #endif
-  return count;
+  CHECK_EQ(expected, count);
 }
 
 
@@ -8120,25 +8120,23 @@ TEST(DontLeakGlobalObjects) {
 
   v8::V8::Initialize();
 
-  int count = GetSurvivingGlobalObjectsCount();
-
   for (int i = 0; i < 5; i++) {
     { v8::HandleScope scope;
       LocalContext context;
     }
-    CHECK_EQ(count, GetSurvivingGlobalObjectsCount());
+    CheckSurvivingGlobalObjectsCount(0);
 
     { v8::HandleScope scope;
       LocalContext context;
       v8_compile("Date")->Run();
     }
-    CHECK_EQ(count, GetSurvivingGlobalObjectsCount());
+    CheckSurvivingGlobalObjectsCount(0);
 
     { v8::HandleScope scope;
       LocalContext context;
       v8_compile("/aaa/")->Run();
     }
-    CHECK_EQ(count, GetSurvivingGlobalObjectsCount());
+    CheckSurvivingGlobalObjectsCount(0);
 
     { v8::HandleScope scope;
       const char* extension_list[] = { "v8/gc" };
@@ -8146,7 +8144,7 @@ TEST(DontLeakGlobalObjects) {
       LocalContext context(&extensions);
       v8_compile("gc();")->Run();
     }
-    CHECK_EQ(count, GetSurvivingGlobalObjectsCount());
+    CheckSurvivingGlobalObjectsCount(0);
   }
 }
 
@@ -11483,3 +11481,141 @@ TEST(BooleanCheckMultipleContexts) {
     ExpectString(code, "");
   }
 }
+
+
+TEST(DontDeleteCellLoadIC) {
+  const char* function_code =
+      "function readCell() { while (true) { return cell; } }";
+
+  {
+    // Run the code twice in the first context to initialize the load
+    // IC for a don't delete cell.
+    v8::HandleScope scope;
+    LocalContext context1;
+    CompileRun("var cell = \"first\";");
+    ExpectBoolean("delete cell", false);
+    CompileRun(function_code);
+    ExpectString("readCell()", "first");
+    ExpectString("readCell()", "first");
+  }
+
+  {
+    // Use a deletable cell in the second context.
+    v8::HandleScope scope;
+    LocalContext context2;
+    CompileRun("cell = \"second\";");
+    CompileRun(function_code);
+    ExpectString("readCell()", "second");
+    ExpectBoolean("delete cell", true);
+    ExpectString("(function() {"
+                 "  try {"
+                 "    return readCell();"
+                 "  } catch(e) {"
+                 "    return e.toString();"
+                 "  }"
+                 "})()",
+                 "ReferenceError: cell is not defined");
+    CompileRun("cell = \"new_second\";");
+    i::Heap::CollectAllGarbage(true);
+    ExpectString("readCell()", "new_second");
+    ExpectString("readCell()", "new_second");
+  }
+}
+
+
+TEST(DontDeleteCellLoadICForceDelete) {
+  const char* function_code =
+      "function readCell() { while (true) { return cell; } }";
+
+  // Run the code twice to initialize the load IC for a don't delete
+  // cell.
+  v8::HandleScope scope;
+  LocalContext context;
+  CompileRun("var cell = \"value\";");
+  ExpectBoolean("delete cell", false);
+  CompileRun(function_code);
+  ExpectString("readCell()", "value");
+  ExpectString("readCell()", "value");
+
+  // Delete the cell using the API and check the inlined code works
+  // correctly.
+  CHECK(context->Global()->ForceDelete(v8_str("cell")));
+  ExpectString("(function() {"
+               "  try {"
+               "    return readCell();"
+               "  } catch(e) {"
+               "    return e.toString();"
+               "  }"
+               "})()",
+               "ReferenceError: cell is not defined");
+}
+
+
+TEST(DontDeleteCellLoadICAPI) {
+  const char* function_code =
+      "function readCell() { while (true) { return cell; } }";
+
+  // Run the code twice to initialize the load IC for a don't delete
+  // cell created using the API.
+  v8::HandleScope scope;
+  LocalContext context;
+  context->Global()->Set(v8_str("cell"), v8_str("value"), v8::DontDelete);
+  ExpectBoolean("delete cell", false);
+  CompileRun(function_code);
+  ExpectString("readCell()", "value");
+  ExpectString("readCell()", "value");
+
+  // Delete the cell using the API and check the inlined code works
+  // correctly.
+  CHECK(context->Global()->ForceDelete(v8_str("cell")));
+  ExpectString("(function() {"
+               "  try {"
+               "    return readCell();"
+               "  } catch(e) {"
+               "    return e.toString();"
+               "  }"
+               "})()",
+               "ReferenceError: cell is not defined");
+}
+
+
+TEST(GlobalLoadICGC) {
+  const char* function_code =
+      "function readCell() { while (true) { return cell; } }";
+
+  // Check inline load code for a don't delete cell is cleared during
+  // GC.
+  {
+    v8::HandleScope scope;
+    LocalContext context;
+    CompileRun("var cell = \"value\";");
+    ExpectBoolean("delete cell", false);
+    CompileRun(function_code);
+    ExpectString("readCell()", "value");
+    ExpectString("readCell()", "value");
+  }
+  {
+    v8::HandleScope scope;
+    LocalContext context2;
+    // Hold the code object in the second context.
+    CompileRun(function_code);
+    CheckSurvivingGlobalObjectsCount(1);
+  }
+
+  // Check inline load code for a deletable cell is cleared during GC.
+  {
+    v8::HandleScope scope;
+    LocalContext context;
+    CompileRun("cell = \"value\";");
+    CompileRun(function_code);
+    ExpectString("readCell()", "value");
+    ExpectString("readCell()", "value");
+  }
+  {
+    v8::HandleScope scope;
+    LocalContext context2;
+    // Hold the code object in the second context.
+    CompileRun(function_code);
+    CheckSurvivingGlobalObjectsCount(1);
+  }
+}