Revision: 7516
Author:   [email protected]
Date:     Wed Apr  6 07:23:27 2011
Log:      Always iterate outgoing arguments as a part of caller frame.

Change caller_sp() to always point to the place after outgoing arguments.

Change deoptimizer to use absolute stack slot addresses for deferred HeapNumber's materialization.

(This is reapplication of r7504 with fix for mozilla testsuite failures).

Review URL: http://codereview.chromium.org/6677164
http://code.google.com/p/v8/source/detail?r=7516

Modified:
 /branches/bleeding_edge/src/deoptimizer.cc
 /branches/bleeding_edge/src/deoptimizer.h
 /branches/bleeding_edge/src/frames-inl.h
 /branches/bleeding_edge/src/frames.cc
 /branches/bleeding_edge/src/frames.h
 /branches/bleeding_edge/src/runtime.cc

=======================================
--- /branches/bleeding_edge/src/deoptimizer.cc  Fri Apr  1 07:46:30 2011
+++ /branches/bleeding_edge/src/deoptimizer.cc  Wed Apr  6 07:23:27 2011
@@ -218,8 +218,7 @@
       fp_to_sp_delta_(fp_to_sp_delta),
       output_count_(0),
       output_(NULL),
-      integer32_values_(NULL),
-      double_values_(NULL) {
+      deferred_heap_numbers_(0) {
   if (FLAG_trace_deopt && type != OSR) {
     PrintF("**** DEOPT: ");
     function->PrintName();
@@ -258,8 +257,6 @@

 Deoptimizer::~Deoptimizer() {
   ASSERT(input_ == NULL && output_ == NULL);
-  delete[] integer32_values_;
-  delete[] double_values_;
 }


@@ -390,13 +387,8 @@
   int count = iterator.Next();
   ASSERT(output_ == NULL);
   output_ = new FrameDescription*[count];
-  // Per-frame lists of untagged and unboxed int32 and double values.
-  integer32_values_ = new List<ValueDescriptionInteger32>[count];
-  double_values_ = new List<ValueDescriptionDouble>[count];
   for (int i = 0; i < count; ++i) {
     output_[i] = NULL;
-    integer32_values_[i].Initialize(0);
-    double_values_[i].Initialize(0);
   }
   output_count_ = count;

@@ -424,38 +416,20 @@
 }


-void Deoptimizer::InsertHeapNumberValues(int index, JavaScriptFrame* frame) {
-  // We need to adjust the stack index by one for the top-most frame.
-  int extra_slot_count = (index == output_count() - 1) ? 1 : 0;
-  List<ValueDescriptionInteger32>* ints = &integer32_values_[index];
-  for (int i = 0; i < ints->length(); i++) {
-    ValueDescriptionInteger32 value = ints->at(i);
-    double val = static_cast<double>(value.int32_value());
- InsertHeapNumberValue(frame, value.stack_index(), val, extra_slot_count);
-  }
-
-  // Iterate over double values and convert them to a heap number.
-  List<ValueDescriptionDouble>* doubles = &double_values_[index];
-  for (int i = 0; i < doubles->length(); ++i) {
-    ValueDescriptionDouble value = doubles->at(i);
-    InsertHeapNumberValue(frame, value.stack_index(), value.double_value(),
-                          extra_slot_count);
+void Deoptimizer::MaterializeHeapNumbers() {
+  for (int i = 0; i < deferred_heap_numbers_.length(); i++) {
+    HeapNumberMaterializationDescriptor d = deferred_heap_numbers_[i];
+    Handle<Object> num = isolate_->factory()->NewNumber(d.value());
+    if (FLAG_trace_deopt) {
+      PrintF("Materializing a new heap number %p [%e] in slot %p\n",
+             reinterpret_cast<void*>(*num),
+             d.value(),
+             d.slot_address());
+    }
+
+    Memory::Object_at(d.slot_address()) = *num;
   }
 }
-
-
-void Deoptimizer::InsertHeapNumberValue(JavaScriptFrame* frame,
-                                        int stack_index,
-                                        double val,
-                                        int extra_slot_count) {
-  // Add one to the TOS index to take the 'state' pushed before jumping
-  // to the stub that calls Runtime::NotifyDeoptimized into account.
-  int tos_index = stack_index + extra_slot_count;
-  int index = (frame->ComputeExpressionsCount() - 1) - tos_index;
-  if (FLAG_trace_deopt) PrintF("Allocating a new heap number: %e\n", val);
-  Handle<Object> num = isolate_->factory()->NewNumber(val);
-  frame->SetExpression(index, *num);
-}


 void Deoptimizer::DoTranslateCommand(TranslationIterator* iterator,
@@ -500,7 +474,6 @@
       int input_reg = iterator->Next();
       intptr_t value = input_->GetRegister(input_reg);
       bool is_smi = Smi::IsValid(value);
-      unsigned output_index = output_offset / kPointerSize;
       if (FLAG_trace_deopt) {
         PrintF(
" 0x%08" V8PRIxPTR ": [top + %d] <- %" V8PRIdPTR " ; %s (%s)\n",
@@ -517,9 +490,8 @@
       } else {
         // We save the untagged value on the side and store a GC-safe
         // temporary placeholder in the frame.
-        AddInteger32Value(frame_index,
-                          output_index,
-                          static_cast<int32_t>(value));
+        AddDoubleValue(output_[frame_index]->GetTop() + output_offset,
+                       static_cast<double>(static_cast<int32_t>(value)));
         output_[frame_index]->SetFrameSlot(output_offset, kPlaceholder);
       }
       return;
@@ -528,7 +500,6 @@
     case Translation::DOUBLE_REGISTER: {
       int input_reg = iterator->Next();
       double value = input_->GetDoubleRegister(input_reg);
-      unsigned output_index = output_offset / kPointerSize;
       if (FLAG_trace_deopt) {
         PrintF("    0x%08" V8PRIxPTR ": [top + %d] <- %e ; %s\n",
                output_[frame_index]->GetTop() + output_offset,
@@ -538,7 +509,7 @@
       }
       // We save the untagged value on the side and store a GC-safe
       // temporary placeholder in the frame.
-      AddDoubleValue(frame_index, output_index, value);
+ AddDoubleValue(output_[frame_index]->GetTop() + output_offset, value);
       output_[frame_index]->SetFrameSlot(output_offset, kPlaceholder);
       return;
     }
@@ -566,7 +537,6 @@
           input_->GetOffsetFromSlotIndex(this, input_slot_index);
       intptr_t value = input_->GetFrameSlot(input_offset);
       bool is_smi = Smi::IsValid(value);
-      unsigned output_index = output_offset / kPointerSize;
       if (FLAG_trace_deopt) {
         PrintF("    0x%08" V8PRIxPTR ": ",
                output_[frame_index]->GetTop() + output_offset);
@@ -583,9 +553,8 @@
       } else {
         // We save the untagged value on the side and store a GC-safe
         // temporary placeholder in the frame.
-        AddInteger32Value(frame_index,
-                          output_index,
-                          static_cast<int32_t>(value));
+        AddDoubleValue(output_[frame_index]->GetTop() + output_offset,
+                       static_cast<double>(static_cast<int32_t>(value)));
         output_[frame_index]->SetFrameSlot(output_offset, kPlaceholder);
       }
       return;
@@ -596,7 +565,6 @@
       unsigned input_offset =
           input_->GetOffsetFromSlotIndex(this, input_slot_index);
       double value = input_->GetDoubleFrameSlot(input_offset);
-      unsigned output_index = output_offset / kPointerSize;
       if (FLAG_trace_deopt) {
         PrintF("    0x%08" V8PRIxPTR ": [top + %d] <- %e ; [esp + %d]\n",
                output_[frame_index]->GetTop() + output_offset,
@@ -606,7 +574,7 @@
       }
       // We save the untagged value on the side and store a GC-safe
       // temporary placeholder in the frame.
-      AddDoubleValue(frame_index, output_index, value);
+ AddDoubleValue(output_[frame_index]->GetTop() + output_offset, value);
       output_[frame_index]->SetFrameSlot(output_offset, kPlaceholder);
       return;
     }
@@ -910,19 +878,11 @@
 }


-void Deoptimizer::AddInteger32Value(int frame_index,
-                                    int slot_index,
-                                    int32_t value) {
-  ValueDescriptionInteger32 value_desc(slot_index, value);
-  integer32_values_[frame_index].Add(value_desc);
-}
-
-
-void Deoptimizer::AddDoubleValue(int frame_index,
-                                 int slot_index,
+void Deoptimizer::AddDoubleValue(intptr_t slot_address,
                                  double value) {
-  ValueDescriptionDouble value_desc(slot_index, value);
-  double_values_[frame_index].Add(value_desc);
+  HeapNumberMaterializationDescriptor value_desc(
+      reinterpret_cast<Address>(slot_address), value);
+  deferred_heap_numbers_.Add(value_desc);
 }


=======================================
--- /branches/bleeding_edge/src/deoptimizer.h   Fri Apr  1 04:41:36 2011
+++ /branches/bleeding_edge/src/deoptimizer.h   Wed Apr  6 07:23:27 2011
@@ -42,41 +42,20 @@
 class DeoptimizingCodeListNode;


-class ValueDescription BASE_EMBEDDED {
+class HeapNumberMaterializationDescriptor BASE_EMBEDDED {
  public:
-  explicit ValueDescription(int index) : stack_index_(index) { }
-  int stack_index() const { return stack_index_; }
-
- private:
-  // Offset relative to the top of the stack.
-  int stack_index_;
-};
-
-
-class ValueDescriptionInteger32: public ValueDescription {
- public:
-  ValueDescriptionInteger32(int index, int32_t value)
-      : ValueDescription(index), int32_value_(value) { }
-  int32_t int32_value() const { return int32_value_; }
+  HeapNumberMaterializationDescriptor(Address slot_address, double val)
+      : slot_address_(slot_address), val_(val) { }
+
+  Address slot_address() const { return slot_address_; }
+  double value() const { return val_; }

  private:
-  // Raw value.
-  int32_t int32_value_;
+  Address slot_address_;
+  double val_;
 };


-class ValueDescriptionDouble: public ValueDescription {
- public:
-  ValueDescriptionDouble(int index, double value)
-      : ValueDescription(index), double_value_(value) { }
-  double double_value() const { return double_value_; }
-
- private:
-  // Raw value.
-  double double_value_;
-};
-
-
 class OptimizedFunctionVisitor BASE_EMBEDDED {
  public:
   virtual ~OptimizedFunctionVisitor() {}
@@ -190,7 +169,7 @@

   ~Deoptimizer();

-  void InsertHeapNumberValues(int index, JavaScriptFrame* frame);
+  void MaterializeHeapNumbers();

   static void ComputeOutputFrames(Deoptimizer* deoptimizer);

@@ -277,13 +256,7 @@

   Object* ComputeLiteral(int index) const;

-  void InsertHeapNumberValue(JavaScriptFrame* frame,
-                             int stack_index,
-                             double val,
-                             int extra_slot_count);
-
-  void AddInteger32Value(int frame_index, int slot_index, int32_t value);
-  void AddDoubleValue(int frame_index, int slot_index, double value);
+  void AddDoubleValue(intptr_t slot_address, double value);

   static LargeObjectChunk* CreateCode(BailoutType type);
   static void GenerateDeoptimizationEntries(
@@ -310,8 +283,7 @@
   // Array of output frame descriptions.
   FrameDescription** output_;

-  List<ValueDescriptionInteger32>* integer32_values_;
-  List<ValueDescriptionDouble>* double_values_;
+  List<HeapNumberMaterializationDescriptor> deferred_heap_numbers_;

   static int table_entry_size_;

=======================================
--- /branches/bleeding_edge/src/frames-inl.h    Tue Apr  5 11:46:06 2011
+++ /branches/bleeding_edge/src/frames-inl.h    Wed Apr  6 07:23:27 2011
@@ -146,17 +146,28 @@
       Memory::Object_at(fp + StandardFrameConstants::kMarkerOffset);
   return marker == Smi::FromInt(CONSTRUCT);
 }
+
+
+Address JavaScriptFrame::GetParameterSlot(int index) const {
+  int param_count = ComputeParametersCount();
+  ASSERT(-1 <= index && index < param_count);
+  int parameter_offset = (param_count - index - 1) * kPointerSize;
+  return caller_sp() + parameter_offset;
+}
+
+
+Object* JavaScriptFrame::GetParameter(int index) const {
+  return Memory::Object_at(GetParameterSlot(index));
+}


 inline Object* JavaScriptFrame::receiver() const {
-  const int offset = JavaScriptFrameConstants::kReceiverOffset;
-  return Memory::Object_at(caller_sp() + offset);
+  return GetParameter(-1);
 }


 inline void JavaScriptFrame::set_receiver(Object* value) {
-  const int offset = JavaScriptFrameConstants::kReceiverOffset;
-  Memory::Object_at(caller_sp() + offset) = value;
+  Memory::Object_at(GetParameterSlot(-1)) = value;
 }


=======================================
--- /branches/bleeding_edge/src/frames.cc       Tue Apr  5 11:46:06 2011
+++ /branches/bleeding_edge/src/frames.cc       Wed Apr  6 07:23:27 2011
@@ -579,9 +579,7 @@
       isolate(), pc(), &safepoint_entry, &stack_slots);
   unsigned slot_space = stack_slots * kPointerSize;

-  // Visit the outgoing parameters. This is usually dealt with by the
-  // callee, but while GC'ing we artificially lower the number of
-  // arguments to zero and let the caller deal with it.
+  // Visit the outgoing parameters.
   Object** parameters_base = &Memory::Object_at(sp());
   Object** parameters_limit = &Memory::Object_at(
       fp() + JavaScriptFrameConstants::kFunctionOffset - slot_space);
@@ -635,21 +633,6 @@

   // Visit the return address in the callee and incoming arguments.
   IteratePc(v, pc_address(), code);
-  IterateArguments(v);
-}
-
-
-Object* JavaScriptFrame::GetParameter(int index) const {
-  ASSERT(index >= 0 && index < ComputeParametersCount());
-  const int offset = JavaScriptFrameConstants::kParam0Offset;
-  return Memory::Object_at(caller_sp() + offset - (index * kPointerSize));
-}
-
-
-int JavaScriptFrame::ComputeParametersCount() const {
-  Address base  = caller_sp() + JavaScriptFrameConstants::kReceiverOffset;
-  Address limit = fp() + JavaScriptFrameConstants::kLastParameterOffset;
-  return static_cast<int>((base - limit) / kPointerSize);
 }


@@ -667,29 +650,19 @@
   JSFunction* function = JSFunction::cast(this->function());
   return function->unchecked_code();
 }
+
+
+int JavaScriptFrame::GetNumberOfIncomingArguments() const {
+  ASSERT(!SafeStackFrameIterator::is_active(isolate()) &&
+         isolate()->heap()->gc_state() == Heap::NOT_IN_GC);
+
+  JSFunction* function = JSFunction::cast(this->function());
+  return function->shared()->formal_parameter_count();
+}


 Address JavaScriptFrame::GetCallerStackPointer() const {
-  int arguments;
-  if (SafeStackFrameIterator::is_active(isolate()) ||
-      isolate()->heap()->gc_state() != Heap::NOT_IN_GC) {
-    // If the we are currently iterating the safe stack the
-    // arguments for frames are traversed as if they were
-    // expression stack elements of the calling frame. The reason for
-    // this rather strange decision is that we cannot access the
-    // function during mark-compact GCs when objects may have been marked.
-    // In fact accessing heap objects (like function->shared() below)
-    // at all during GC is problematic.
-    arguments = 0;
-  } else {
-    // Compute the number of arguments by getting the number of formal
-    // parameters of the function. We must remember to take the
-    // receiver into account (+1).
-    JSFunction* function = JSFunction::cast(this->function());
-    arguments = function->shared()->formal_parameter_count() + 1;
-  }
-  const int offset = StandardFrameConstants::kCallerSPOffset;
-  return fp() + offset + (arguments * kPointerSize);
+  return fp() + StandardFrameConstants::kCallerSPOffset;
 }


@@ -867,9 +840,7 @@


 Address ArgumentsAdaptorFrame::GetCallerStackPointer() const {
-  const int arguments = Smi::cast(GetExpression(0))->value();
-  const int offset = StandardFrameConstants::kCallerSPOffset;
-  return fp() + offset + (arguments + 1) * kPointerSize;
+  return fp() + StandardFrameConstants::kCallerSPOffset;
 }


@@ -1109,17 +1080,6 @@
 void JavaScriptFrame::Iterate(ObjectVisitor* v) const {
   IterateExpressions(v);
   IteratePc(v, pc_address(), LookupCode());
-  IterateArguments(v);
-}
-
-
-void JavaScriptFrame::IterateArguments(ObjectVisitor* v) const {
-  // Traverse callee-saved registers, receiver, and parameters.
-  const int kBaseOffset = JavaScriptFrameConstants::kLastParameterOffset;
-  const int kLimitOffset = JavaScriptFrameConstants::kReceiverOffset;
-  Object** base = &Memory::Object_at(fp() + kBaseOffset);
-  Object** limit = &Memory::Object_at(caller_sp() + kLimitOffset) + 1;
-  v->VisitPointers(base, limit);
 }


=======================================
--- /branches/bleeding_edge/src/frames.h        Tue Apr  5 11:46:06 2011
+++ /branches/bleeding_edge/src/frames.h        Wed Apr  6 07:23:27 2011
@@ -463,8 +463,11 @@
   inline void set_receiver(Object* value);

   // Access the parameters.
-  Object* GetParameter(int index) const;
-  int ComputeParametersCount() const;
+  inline Address GetParameterSlot(int index) const;
+  inline Object* GetParameter(int index) const;
+  inline int ComputeParametersCount() const {
+    return GetNumberOfIncomingArguments();
+  }

   // Check if this frame is a constructor frame invoked through 'new'.
   bool IsConstructor() const;
@@ -502,6 +505,8 @@

   virtual Address GetCallerStackPointer() const;

+  virtual int GetNumberOfIncomingArguments() const;
+
   // Garbage collection support. Iterates over incoming arguments,
   // receiver, and any callee-saved registers.
   void IterateArguments(ObjectVisitor* v) const;
@@ -561,6 +566,10 @@
  protected:
   explicit ArgumentsAdaptorFrame(StackFrameIterator* iterator)
       : JavaScriptFrame(iterator) { }
+
+  virtual int GetNumberOfIncomingArguments() const {
+    return Smi::cast(GetExpression(0))->value();
+  }

   virtual Address GetCallerStackPointer() const;

=======================================
--- /branches/bleeding_edge/src/runtime.cc      Tue Apr  5 02:21:02 2011
+++ /branches/bleeding_edge/src/runtime.cc      Wed Apr  6 07:23:27 2011
@@ -7323,14 +7323,13 @@
   ASSERT(isolate->heap()->IsAllocationAllowed());
   int frames = deoptimizer->output_count();

+  deoptimizer->MaterializeHeapNumbers();
+  delete deoptimizer;
+
   JavaScriptFrameIterator it(isolate);
   JavaScriptFrame* frame = NULL;
-  for (int i = 0; i < frames; i++) {
-    if (i != 0) it.Advance();
-    frame = it.frame();
-    deoptimizer->InsertHeapNumberValues(frames - i - 1, frame);
-  }
-  delete deoptimizer;
+  for (int i = 0; i < frames - 1; i++) it.Advance();
+  frame = it.frame();

   RUNTIME_ASSERT(frame->function()->IsJSFunction());
Handle<JSFunction> function(JSFunction::cast(frame->function()), isolate);

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to