Inline the inobject property case for named property loads.

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

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

src/codegen.h

@@ -116,6 +116,9 @@ class DeferredCode: public ZoneObject {
   JumpTarget* enter() { return &enter_; }
   void BindExit() { exit_.Bind(0); }
   void BindExit(Result* result) { exit_.Bind(result, 1); }
+  void BindExit(Result* result0, Result* result1) {
+    exit_.Bind(result0, result1, 2);
+  }
   void BindExit(Result* result0, Result* result1, Result* result2) {
     exit_.Bind(result0, result1, result2, 3);
   }

src/debug.cc

@@ -377,9 +377,7 @@ void BreakLocationIterator::SetDebugBreakAtIC() {
     // is set the patching performed by the runtime system will take place in
     // 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::HasInlinedVersion(pc())) {
-      KeyedLoadIC::ClearInlinedVersion(pc());
-    }
+    if (code->is_keyed_load_stub()) KeyedLoadIC::ClearInlinedVersion(pc());
   }
 }
 

src/ia32/codegen-ia32.cc

@@ -30,6 +30,7 @@
 #include "bootstrapper.h"
 #include "codegen-inl.h"
 #include "debug.h"
+#include "ic-inl.h"
 #include "parser.h"
 #include "register-allocator-inl.h"
 #include "runtime.h"
@@ -3410,7 +3411,10 @@ Result CodeGenerator::LoadFromGlobalSlotCheckExtensions(
                          ? RelocInfo::CODE_TARGET
                          : RelocInfo::CODE_TARGET_CONTEXT;
   Result answer = frame_->CallLoadIC(mode);
-
+  // A test eax instruction following the call signals that the inobject
+  // property case was inlined.  Ensure that there is not a test eax
+  // instruction here.
+  __ nop();
   // Discard the global object. The result is in answer.
   frame_->Drop();
   return answer;
@@ -5233,6 +5237,47 @@ bool CodeGenerator::HasValidEntryRegisters() {
 #endif
 
 
+class DeferredReferenceGetNamedValue: public DeferredCode {
+ public:
+  DeferredReferenceGetNamedValue(CodeGenerator* cgen, Handle<String> name)
+      : DeferredCode(cgen), name_(name) {
+    set_comment("[ DeferredReferenceGetNamedValue");
+  }
+
+  virtual void Generate();
+
+  Label* patch_site() { return &patch_site_; }
+
+ private:
+  Label patch_site_;
+  Handle<String> name_;
+};
+
+
+void DeferredReferenceGetNamedValue::Generate() {
+  CodeGenerator* cgen = generator();
+  Result receiver(cgen);
+  enter()->Bind(&receiver);
+
+  cgen->frame()->Push(&receiver);
+  cgen->frame()->Push(name_);
+  Result answer = cgen->frame()->CallLoadIC(RelocInfo::CODE_TARGET);
+  // The call must be followed by a test eax instruction to indicate
+  // that the inobject property case was inlined.
+  ASSERT(answer.is_register() && answer.reg().is(eax));
+  // Store the delta to the map check instruction here in the test
+  // instruction.
+  int delta_to_patch_site = __ SizeOfCodeGeneratedSince(patch_site());
+  // Here we use masm_-> instead of the double underscore macro because
+  // this is the instruction that gets patched and coverage code gets in
+  // the way.
+  masm_->test(answer.reg(), Immediate(-delta_to_patch_site));
+  __ IncrementCounter(&Counters::named_load_inline_miss, 1);
+  receiver = cgen->frame()->Pop();
+  exit_.Jump(&receiver, &answer);
+}
+
+
 class DeferredReferenceGetKeyedValue: public DeferredCode {
  public:
   DeferredReferenceGetKeyedValue(CodeGenerator* generator, bool is_global)
@@ -5334,16 +5379,63 @@ void Reference::GetValue(TypeofState typeof_state) {
       // thrown below, we must distinguish between the two kinds of
       // loads (typeof expression loads must not throw a reference
       // error).
-      Comment cmnt(masm, "[ Load from named Property");
-      cgen_->frame()->Push(GetName());
-
       Variable* var = expression_->AsVariableProxy()->AsVariable();
-      ASSERT(var == NULL || var->is_global());
-      RelocInfo::Mode mode = (var == NULL)
-                             ? RelocInfo::CODE_TARGET
-                             : RelocInfo::CODE_TARGET_CONTEXT;
-      Result answer = cgen_->frame()->CallLoadIC(mode);
-      cgen_->frame()->Push(&answer);
+      bool is_global = var != NULL;
+      ASSERT(!is_global || var->is_global());
+
+      if (is_global || cgen_->scope()->is_global_scope()) {
+        // Do not inline the inobject property case for loads from the
+        // global object or loads in toplevel code.
+        Comment cmnt(masm, "[ Load from named Property");
+        cgen_->frame()->Push(GetName());
+
+        RelocInfo::Mode mode = is_global
+                               ? RelocInfo::CODE_TARGET_CONTEXT
+                               : RelocInfo::CODE_TARGET;
+        Result answer = cgen_->frame()->CallLoadIC(mode);
+        // A test eax instruction following the call signals that the
+        // inobject property case was inlined.  Ensure that there is not
+        // a test eax instruction here.
+        __ nop();
+        cgen_->frame()->Push(&answer);
+      } else {
+        // Inline the inobject property case.
+        Comment cmnt(masm, "[ Inlined named property load");
+        DeferredReferenceGetNamedValue* deferred =
+            new DeferredReferenceGetNamedValue(cgen_, GetName());
+        Result receiver = cgen_->frame()->Pop();
+        receiver.ToRegister();
+        // Check that the receiver is a heap object.
+        __ test(receiver.reg(), Immediate(kSmiTagMask));
+        deferred->enter()->Branch(zero, &receiver, not_taken);
+
+        // Preallocate the value register to ensure that there is no
+        // spill emitted between the patch site label and the offset in
+        // the load instruction.
+        Result value = cgen_->allocator()->Allocate();
+        ASSERT(value.is_valid());
+        __ bind(deferred->patch_site());
+        // This is the map check instruction that will be patched.
+        // Initially use an invalid map to force a failure.
+        __ cmp(FieldOperand(receiver.reg(), HeapObject::kMapOffset),
+               Immediate(Factory::null_value()));
+        deferred->enter()->Branch(not_equal, &receiver, not_taken);
+
+        // The delta from the patch label to the load offset must be
+        // statically known.
+        ASSERT(masm->SizeOfCodeGeneratedSince(deferred->patch_site()) ==
+               LoadIC::kOffsetToLoadInstruction);
+        // The initial (invalid) offset has to be large enough to force
+        // a 32-bit instruction encoding to allow patching with an
+        // arbitrary offset.  Use kMaxInt (minus kHeapObjectTag).
+        int offset = kMaxInt;
+        __ mov(value.reg(), FieldOperand(receiver.reg(), offset));
+
+        __ IncrementCounter(&Counters::named_load_inline, 1);
+        deferred->BindExit(&receiver, &value);
+        cgen_->frame()->Push(&receiver);
+        cgen_->frame()->Push(&value);
+      }
       break;
     }
 

src/ia32/ic-ia32.cc

@@ -729,36 +729,66 @@ void LoadIC::Generate(MacroAssembler* masm, const ExternalReference& f) {
 static const byte kTestEaxByte = 0xA9;
 
 
-bool KeyedLoadIC::HasInlinedVersion(Address address) {
-  Address test_instruction_address = address + 4;  // 4 = stub address
-  return *test_instruction_address == kTestEaxByte;
+void LoadIC::ClearInlinedVersion(Address address) {
+  // Reset the map check of the inlined inobject property load (if
+  // present) to guarantee failure by holding an invalid map (the null
+  // value).  The offset can be patched to anything.
+  PatchInlinedLoad(address, Heap::null_value(), kMaxInt);
 }
 
 
 void KeyedLoadIC::ClearInlinedVersion(Address address) {
   // Insert null as the map to check for to make sure the map check fails
   // sending control flow to the IC instead of the inlined version.
-  PatchInlinedMapCheck(address, Heap::null_value());
+  PatchInlinedLoad(address, Heap::null_value());
 }
 
 
-void KeyedLoadIC::PatchInlinedMapCheck(Address address, Object* value) {
+bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) {
+  // The address of the instruction following the call.
+  Address test_instruction_address = address + 4;
+  // If the instruction following the call is not a test eax, nothing
+  // was inlined.
+  if (*test_instruction_address != kTestEaxByte) return false;
+
+  Address delta_address = test_instruction_address + 1;
+  // The delta to the start of the map check instruction.
+  int delta = *reinterpret_cast<int*>(delta_address);
+
+  // The map address is the last 4 bytes of the 7-byte
+  // operand-immediate compare instruction, so we add 3 to get the
+  // offset to the last 4 bytes.
+  Address map_address = test_instruction_address + delta + 3;
+  *(reinterpret_cast<Object**>(map_address)) = map;
+
+  // The offset is in the last 4 bytes of a six byte
+  // memory-to-register move instruction, so we add 2 to get the
+  // offset to the last 4 bytes.
+  Address offset_address =
+      test_instruction_address + delta + kOffsetToLoadInstruction + 2;
+  *reinterpret_cast<int*>(offset_address) = offset - kHeapObjectTag;
+  return true;
+}
+
+
+bool KeyedLoadIC::PatchInlinedLoad(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.
-  if (*test_instruction_address == kTestEaxByte) {
-    // Fetch the offset from the test instruction to the map cmp
-    // instruction.  This offset is stored in the last 4 bytes of the
-    // 5 byte test instruction.
-    Address offset_address = test_instruction_address + 1;
-    int offset_value = *(reinterpret_cast<int*>(offset_address));
-    // Compute the map address.  The map address is in the last 4
-    // bytes of the 7-byte operand-immediate compare instruction, so
-    // we add 3 to the offset to get the map address.
-    Address map_address = test_instruction_address + offset_value + 3;
-    // Patch the map check.
-    (*(reinterpret_cast<Object**>(map_address))) = value;
-  }
+  if (*test_instruction_address != kTestEaxByte) return false;
+
+  // Fetch the offset from the test instruction to the map cmp
+  // instruction.  This offset is stored in the last 4 bytes of the 5
+  // byte test instruction.
+  Address delta_address = test_instruction_address + 1;
+  int delta = *reinterpret_cast<int*>(delta_address);
+  // Compute the map address.  The map address is in the last 4 bytes
+  // of the 7-byte operand-immediate compare instruction, so we add 3
+  // to the offset to get the map address.
+  Address map_address = test_instruction_address + delta + 3;
+  // Patch the map check.
+  *(reinterpret_cast<Object**>(map_address)) = map;
+  return true;
 }
 
 

src/ic.cc

@@ -42,7 +42,7 @@ static char TransitionMarkFromState(IC::State state) {
   switch (state) {
     case UNINITIALIZED: return '0';
     case UNINITIALIZED_IN_LOOP: return 'L';
-    case PREMONOMORPHIC: return '0';
+    case PREMONOMORPHIC: return 'P';
     case MONOMORPHIC: return '1';
     case MONOMORPHIC_PROTOTYPE_FAILURE: return '^';
     case MEGAMORPHIC: return 'N';
@@ -244,6 +244,7 @@ void KeyedLoadIC::Clear(Address address, Code* target) {
 
 void LoadIC::Clear(Address address, Code* target) {
   if (target->ic_state() == UNINITIALIZED) return;
+  ClearInlinedVersion(address);
   SetTargetAtAddress(address, initialize_stub());
 }
 
@@ -523,6 +524,31 @@ Object* LoadIC::Load(State state, Handle<Object> object, Handle<String> name) {
     LOG(SuspectReadEvent(*name, *object));
   }
 
+  bool can_be_inlined =
+      FLAG_use_ic &&
+      state == PREMONOMORPHIC &&
+      lookup.IsValid() &&
+      lookup.IsLoaded() &&
+      lookup.IsCacheable() &&
+      lookup.holder() == *object &&
+      lookup.type() == FIELD &&
+      !object->IsAccessCheckNeeded();
+
+  if (can_be_inlined) {
+    Map* map = lookup.holder()->map();
+    // Property's index in the properties array.  If negative we have
+    // an inobject property.
+    int index = lookup.GetFieldIndex() - map->inobject_properties();
+    if (index < 0) {
+      // Index is an offset from the end of the object.
+      int offset = map->instance_size() + (index * kPointerSize);
+      if (PatchInlinedLoad(address(), map, offset)) {
+        set_target(megamorphic_stub());
+        return lookup.holder()->FastPropertyAt(lookup.GetFieldIndex());
+      }
+    }
+  }
+
   // Update inline cache and stub cache.
   if (FLAG_use_ic && lookup.IsLoaded()) {
     UpdateCaches(&lookup, state, object, name);
@@ -734,7 +760,7 @@ Object* KeyedLoadIC::Load(State state,
         !object->IsJSValue() &&
         !JSObject::cast(*object)->HasIndexedInterceptor()) {
       Map* map = JSObject::cast(*object)->map();
-      PatchInlinedMapCheck(address(), map);
+      PatchInlinedLoad(address(), map);
     }
   }
 

src/ic.h

@@ -216,6 +216,11 @@ class LoadIC: public IC {
   static void GenerateStringLength(MacroAssembler* masm);
   static void GenerateFunctionPrototype(MacroAssembler* masm);
 
+  // The offset from the inlined patch site to the start of the
+  // inlined load instruction.  It is 7 bytes (test eax, imm) plus
+  // 6 bytes (jne slow_label).
+  static const int kOffsetToLoadInstruction = 13;
+
  private:
   static void Generate(MacroAssembler* masm, const ExternalReference& f);
 
@@ -238,6 +243,12 @@ class LoadIC: public IC {
   }
 
   static void Clear(Address address, Code* target);
+
+  // Clear the use of the inlined version.
+  static void ClearInlinedVersion(Address address);
+
+  static bool PatchInlinedLoad(Address address, Object* map, int index);
+
   friend class IC;
 };
 
@@ -254,9 +265,6 @@ class KeyedLoadIC: public IC {
   static void GeneratePreMonomorphic(MacroAssembler* masm);
   static void GenerateGeneric(MacroAssembler* masm);
 
-  // Check if this IC corresponds to an inlined version.
-  static bool HasInlinedVersion(Address address);
-
   // Clear the use of the inlined version.
   static void ClearInlinedVersion(Address address);
 
@@ -287,7 +295,7 @@ class KeyedLoadIC: public IC {
 
   // Support for patching the map that is checked in an inlined
   // version of keyed load.
-  static void PatchInlinedMapCheck(Address address, Object* map);
+  static bool PatchInlinedLoad(Address address, Object* map);
 
   friend class IC;
 };

src/v8-counters.h

@@ -116,6 +116,8 @@ namespace v8 { namespace internal {
   SC(keyed_load_interceptor, V8.KeyedLoadInterceptor)               \
   SC(keyed_load_inline, V8.KeyedLoadInline)                         \
   SC(keyed_load_inline_miss, V8.KeyedLoadInlineMiss)                \
+  SC(named_load_inline, V8.NamedLoadInline)                         \
+  SC(named_load_inline_miss, V8.NamedLoadInlineMiss)                \
   SC(keyed_store_field, V8.KeyedStoreField)                         \
   SC(for_in, V8.ForIn)                                              \
   SC(enum_cache_hits, V8.EnumCacheHits)                             \