Fix debugger after inlined keyed store change.
authorager@chromium.org <ager@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 12 Jun 2009 11:24:13 +0000 (11:24 +0000)
committerager@chromium.org <ager@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 12 Jun 2009 11:24:13 +0000 (11:24 +0000)
Make sure that the IC is always hit when debugging and make sure to
restore the fast case when leaving the debugger.
Review URL: http://codereview.chromium.org/125044

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

src/arm/ic-arm.cc
src/debug.cc
src/ia32/codegen-ia32.cc
src/ia32/ic-ia32.cc
src/ic.h
src/x64/ic-x64.cc
test/cctest/test-debug.cc

index 58e061a..8b4e087 100644 (file)
@@ -503,6 +503,12 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
   return false;
 }
 
+void KeyedStoreIC::ClearInlinedVersion(Address address) {}
+void KeyedStoreIC::RestoreInlinedVersion(Address address) {}
+bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) {
+  return false;
+}
+
 
 Object* KeyedLoadIC_Miss(Arguments args);
 
index 0daf564..e37bfb7 100644 (file)
@@ -382,6 +382,7 @@ void BreakLocationIterator::SetDebugBreakAtIC() {
     // the code copy and will therefore have no effect on the running code
     // keeping it from using the inlined code.
     if (code->is_keyed_load_stub()) KeyedLoadIC::ClearInlinedVersion(pc());
+    if (code->is_keyed_store_stub()) KeyedStoreIC::ClearInlinedVersion(pc());
   }
 }
 
@@ -389,6 +390,19 @@ void BreakLocationIterator::SetDebugBreakAtIC() {
 void BreakLocationIterator::ClearDebugBreakAtIC() {
   // Patch the code to the original invoke.
   rinfo()->set_target_address(original_rinfo()->target_address());
+
+  RelocInfo::Mode mode = rmode();
+  if (RelocInfo::IsCodeTarget(mode)) {
+    Address target = original_rinfo()->target_address();
+    Handle<Code> code(Code::GetCodeFromTargetAddress(target));
+
+    // Restore the inlined version of keyed stores to get back to the
+    // fast case.  We need to patch back the keyed store because no
+    // patching happens when running normally.  For keyed loads, the
+    // map check will get patched back when running normally after ICs
+    // have been cleared at GC.
+    if (code->is_keyed_store_stub()) KeyedStoreIC::RestoreInlinedVersion(pc());
+  }
 }
 
 
index c7a35e8..bd27af7 100644 (file)
@@ -5749,10 +5749,13 @@ class DeferredReferenceSetKeyedValue: public DeferredCode {
 
   virtual void Generate();
 
+  Label* patch_site() { return &patch_site_; }
+
  private:
   Register value_;
   Register key_;
   Register receiver_;
+  Label patch_site_;
 };
 
 
@@ -5766,6 +5769,15 @@ void DeferredReferenceSetKeyedValue::Generate() {
   // Call the IC stub.
   Handle<Code> ic(Builtins::builtin(Builtins::KeyedStoreIC_Initialize));
   __ call(ic, RelocInfo::CODE_TARGET);
+  // The delta from the start of the map-compare instruction to the
+  // test instruction.  We use masm_-> directly here instead of the
+  // __ macro because the macro sometimes uses macro expansion to turn
+  // into something that can't return a value.  This is encountered
+  // when doing generated code coverage tests.
+  int delta_to_patch_site = masm_->SizeOfCodeGeneratedSince(patch_site());
+  // Here we use masm_-> instead of the __ macro because this is the
+  // instruction that gets patched and coverage code gets in the way.
+  masm_->test(eax, Immediate(-delta_to_patch_site));
   // Restore value (returned from store IC), key and receiver
   // registers.
   if (!value_.is(eax)) __ mov(value_, eax);
@@ -6122,9 +6134,15 @@ void Reference::SetValue(InitState init_state) {
         // is not a dictionary.
         __ mov(tmp.reg(),
                FieldOperand(receiver.reg(), JSObject::kElementsOffset));
+        // Bind the deferred code patch site to be able to locate the
+        // fixed array map comparison.  When debugging, we patch this
+        // comparison to always fail so that we will hit the IC call
+        // in the deferred code which will allow the debugger to
+        // break for fast case stores.
+        __ bind(deferred->patch_site());
         __ cmp(FieldOperand(tmp.reg(), HeapObject::kMapOffset),
-                           Immediate(Factory::hash_table_map()));
-        deferred->Branch(equal);
+               Immediate(Factory::fixed_array_map()));
+        deferred->Branch(not_equal);
 
         // Store the value.
         __ mov(Operand(tmp.reg(),
@@ -6141,6 +6159,11 @@ void Reference::SetValue(InitState init_state) {
         cgen_->frame()->Push(&value);
       } else {
         Result answer = cgen_->frame()->CallKeyedStoreIC();
+        // Make sure that we do not have a test instruction after the
+        // call.  A test instruction after the call is used to
+        // indicate that we have generated an inline version of the
+        // keyed store.
+        __ nop();
         cgen_->frame()->Push(&answer);
       }
       break;
index d7f264d..5da9b2f 100644 (file)
@@ -747,6 +747,21 @@ void KeyedLoadIC::ClearInlinedVersion(Address address) {
 }
 
 
+void KeyedStoreIC::ClearInlinedVersion(Address address) {
+  // Insert null as the elements map to check for.  This will make
+  // sure that the elements fast-case map check fails so that control
+  // flows to the IC instead of the inlined version.
+  PatchInlinedStore(address, Heap::null_value());
+}
+
+
+void KeyedStoreIC::RestoreInlinedVersion(Address address) {
+  // Restore the fast-case elements map check so that the inlined
+  // version can be used again.
+  PatchInlinedStore(address, Heap::fixed_array_map());
+}
+
+
 bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
   // The address of the instruction following the call.
   Address test_instruction_address = address + 4;
@@ -774,7 +789,7 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
 }
 
 
-bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
+static bool PatchInlinedMapCheck(Address address, Object* map) {
   Address test_instruction_address = address + 4;  // 4 = stub address
   // The keyed load has a fast inlined case if the IC call instruction
   // is immediately followed by a test instruction.
@@ -795,6 +810,16 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
 }
 
 
+bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
+  return PatchInlinedMapCheck(address, map);
+}
+
+
+bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) {
+  return PatchInlinedMapCheck(address, map);
+}
+
+
 // Defined in ic.cc.
 Object* KeyedLoadIC_Miss(Arguments args);
 
index bd94fd8..9c96ba2 100644 (file)
--- a/src/ic.h
+++ b/src/ic.h
@@ -356,6 +356,12 @@ class KeyedStoreIC: public IC {
   static void GenerateGeneric(MacroAssembler* masm);
   static void GenerateExtendStorage(MacroAssembler* masm);
 
+  // Clear the inlined version so the IC is always hit.
+  static void ClearInlinedVersion(Address address);
+
+  // Restore the inlined version so the fast case can get hit.
+  static void RestoreInlinedVersion(Address address);
+
  private:
   static void Generate(MacroAssembler* masm, const ExternalReference& f);
 
@@ -378,6 +384,11 @@ class KeyedStoreIC: public IC {
   }
 
   static void Clear(Address address, Code* target);
+
+  // Support for patching the map that is checked in an inlined
+  // version of keyed store.
+  static bool PatchInlinedStore(Address address, Object* map);
+
   friend class IC;
 };
 
index 71a3a9a..858f3a2 100644 (file)
@@ -40,6 +40,14 @@ void KeyedLoadIC::ClearInlinedVersion(Address address) {
   UNIMPLEMENTED();
 }
 
+void KeyedStoreIC::ClearInlinedVersion(Address address) {
+  UNIMPLEMENTED();
+}
+
+void KeyedStoreIC::RestoreInlinedVersion(Address address) {
+  UNIMPLEMENTED();
+}
+
 void KeyedLoadIC::Generate(MacroAssembler* masm,
                            ExternalReference const& f) {
   masm->int3();  // UNIMPLEMENTED.
@@ -58,6 +66,11 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) {
   return false;
 }
 
+bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) {
+  UNIMPLEMENTED();
+  return false;
+}
+
 Object* KeyedLoadStubCompiler::CompileLoadArrayLength(String* name) {
   UNIMPLEMENTED();
   return NULL;
index 92f48e1..7669b43 100644 (file)
@@ -2237,6 +2237,52 @@ TEST(DebugStepKeyedLoadLoop) {
 }
 
 
+// Test of the stepping mechanism for keyed store in a loop.
+TEST(DebugStepKeyedStoreLoop) {
+  v8::HandleScope scope;
+  DebugLocalContext env;
+
+  // Create a function for testing stepping of keyed store. The statement 'y=1'
+  // is there to have more than one breakable statement in the loop, TODO(315).
+  v8::Local<v8::Function> foo = CompileFunction(
+      &env,
+      "function foo(a) {\n"
+      "  var len = a.length;\n"
+      "  for (var i = 0; i < len; i++) {\n"
+      "    y = 1;\n"
+      "    a[i] = 42;\n"
+      "  }\n"
+      "}\n",
+      "foo");
+
+  // Create array [0,1,2,3,4,5,6,7,8,9]
+  v8::Local<v8::Array> a = v8::Array::New(10);
+  for (int i = 0; i < 10; i++) {
+    a->Set(v8::Number::New(i), v8::Number::New(i));
+  }
+
+  // Call function without any break points to ensure inlining is in place.
+  const int kArgc = 1;
+  v8::Handle<v8::Value> args[kArgc] = { a };
+  foo->Call(env->Global(), kArgc, args);
+
+  // Register a debug event listener which steps and counts.
+  v8::Debug::SetDebugEventListener(DebugEventStep);
+
+  // Setup break point and step through the function.
+  SetBreakPoint(foo, 3);
+  step_action = StepNext;
+  break_point_hit_count = 0;
+  foo->Call(env->Global(), kArgc, args);
+
+  // With stepping all break locations are hit.
+  CHECK_EQ(22, break_point_hit_count);
+
+  v8::Debug::SetDebugEventListener(NULL);
+  CheckDebuggerUnloaded();
+}
+
+
 // Test the stepping mechanism with different ICs.
 TEST(DebugStepLinearMixedICs) {
   v8::HandleScope scope;