Defer the writing of the source position data to the relocation information

until a possible debug break location is reached. Currently this is call sites
with calls to code objects and JS return. Source position information in the
code therefore no longer refers to the "first" instruction generated for a
given source position (which was not the case defered code anyway) but to the
first break location after that source position was passed (again defered code
always start with source position information). This doesn't make a difference
for the debugger as it will always be stopped only at debug break locations.
However, this makes the life of the peep-hole optimizer much easier as many
oportunities for posh/pop eliminations where previosly blocked by relocation
information already written to the code object.

Two types of source positions are still collected. Statement positions indicate
the position of the start of the statement leading to this code and (plain)
positions indicate other places typically call sites to help indicate current
position in backtraces. The two different types of positions are also used to
distinguish between step next and step in.

Runs all the tests (including debugger tests) as before.

Moved the checking for the FLAG_debug_info to one place.

I will do the same changes to the ARM codegenerator in a seperate changelist.

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

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

src/assembler-arm.h

@@ -645,9 +645,7 @@ class Assembler : public Malloced {
 
   int pc_offset() const { return pc_ - buffer_; }
   int last_position() const { return last_position_; }
-  bool last_position_is_statement() const {
-    return last_position_is_statement_;
-  }
+  int last_statement_position() const { return last_statement_position_; }
 
  protected:
   int buffer_space() const { return reloc_info_writer.pos() - pc_; }
@@ -747,8 +745,8 @@ class Assembler : public Malloced {
   int last_bound_pos_;
 
   // source position information
+  int last_statement_position_;
   int last_position_;
-  bool last_position_is_statement_;
 
   // Code emission
   inline void CheckBuffer();

src/assembler-ia32.cc

@@ -317,7 +317,7 @@ Assembler::Assembler(void* buffer, int buffer_size) {
   last_pc_ = NULL;
   last_bound_pos_ = 0;
   last_position_ = kNoPosition;
-  last_position_is_statement_ = false;
+  last_statement_position_ = kNoPosition;
 }
 
 
@@ -1306,6 +1306,7 @@ void Assembler::call(const Operand& adr) {
 
 
 void Assembler::call(Handle<Code> code,  RelocMode rmode) {
+  WriteRecordedPositions();
   EnsureSpace ensure_space(this);
   last_pc_ = pc_;
   ASSERT(is_code_target(rmode));
@@ -1844,6 +1845,7 @@ void Assembler::Print() {
 
 
 void Assembler::RecordJSReturn() {
+  WriteRecordedPositions();
   EnsureSpace ensure_space(this);
   RecordRelocInfo(js_return);
 }
@@ -1859,23 +1861,30 @@ void Assembler::RecordComment(const char* msg) {
 
 void Assembler::RecordPosition(int pos) {
   if (pos == kNoPosition) return;
-  ASSERT(position >= 0);
-  if (pos == last_position_) return;
-  EnsureSpace ensure_space(this);
-  RecordRelocInfo(position, pos);
+  ASSERT(pos >= 0);
   last_position_ = pos;
-  last_position_is_statement_ = false;
 }
 
 
 void Assembler::RecordStatementPosition(int pos) {
   if (pos == kNoPosition) return;
-  ASSERT(position >= 0);
-  if (pos == last_position_) return;
-  EnsureSpace ensure_space(this);
-  RecordRelocInfo(statement_position, pos);
-  last_position_ = pos;
-  last_position_is_statement_ = true;
+  ASSERT(pos >= 0);
+  last_statement_position_ = pos;
+}
+
+
+void Assembler::WriteRecordedPositions() {
+  if (last_statement_position_ != kNoPosition) {
+    EnsureSpace ensure_space(this);
+    RecordRelocInfo(statement_position, last_statement_position_);
+  }
+  if ((last_position_ != kNoPosition) &&
+      (last_position_ != last_statement_position_)) {
+    EnsureSpace ensure_space(this);
+    RecordRelocInfo(position, last_position_);
+  }
+  last_statement_position_ = kNoPosition;
+  last_position_ = kNoPosition;
 }
 
 

src/assembler-ia32.h

@@ -673,12 +673,11 @@ class Assembler : public Malloced {
 
   void RecordPosition(int pos);
   void RecordStatementPosition(int pos);
+  void WriteRecordedPositions();
 
   int pc_offset() const  { return pc_ - buffer_; }
+  int last_statement_position() const  { return last_statement_position_; }
   int last_position() const  { return last_position_; }
-  bool last_position_is_statement() const  {
-    return last_position_is_statement_;
-  }
 
   // Check if there is less than kGap bytes available in the buffer.
   // If this is the case, we need to grow the buffer before emitting
@@ -725,7 +724,7 @@ class Assembler : public Malloced {
 
   // source position information
   int last_position_;
-  bool last_position_is_statement_;
+  int last_statement_position_;
 
   byte* addr_at(int pos)  { return buffer_ + pos; }
   byte byte_at(int pos)  { return buffer_[pos]; }

src/codegen-ia32.cc

@@ -2656,7 +2656,7 @@ void Ia32CodeGenerator::CheckStack() {
 
 void Ia32CodeGenerator::VisitBlock(Block* node) {
   Comment cmnt(masm_, "[ Block");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   node->set_break_stack_height(break_stack_height_);
   VisitStatements(node->statements());
   __ bind(node->break_target());
@@ -2734,7 +2734,7 @@ void Ia32CodeGenerator::VisitDeclaration(Declaration* node) {
 
 void Ia32CodeGenerator::VisitExpressionStatement(ExpressionStatement* node) {
   Comment cmnt(masm_, "[ ExpressionStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   Expression* expression = node->expression();
   expression->MarkAsStatement();
   Load(expression);
@@ -2755,7 +2755,7 @@ void Ia32CodeGenerator::VisitIfStatement(IfStatement* node) {
   bool has_then_stm = node->HasThenStatement();
   bool has_else_stm = node->HasElseStatement();
 
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   Label exit;
   if (has_then_stm && has_else_stm) {
     Label then;
@@ -2819,7 +2819,7 @@ void Ia32CodeGenerator::CleanStack(int num_bytes) {
 
 void Ia32CodeGenerator::VisitContinueStatement(ContinueStatement* node) {
   Comment cmnt(masm_, "[ ContinueStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   CleanStack(break_stack_height_ - node->target()->break_stack_height());
   __ jmp(node->target()->continue_target());
 }
@@ -2827,7 +2827,7 @@ void Ia32CodeGenerator::VisitContinueStatement(ContinueStatement* node) {
 
 void Ia32CodeGenerator::VisitBreakStatement(BreakStatement* node) {
   Comment cmnt(masm_, "[ BreakStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   CleanStack(break_stack_height_ - node->target()->break_stack_height());
   __ jmp(node->target()->break_target());
 }
@@ -2835,7 +2835,7 @@ void Ia32CodeGenerator::VisitBreakStatement(BreakStatement* node) {
 
 void Ia32CodeGenerator::VisitReturnStatement(ReturnStatement* node) {
   Comment cmnt(masm_, "[ ReturnStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   Load(node->expression());
 
   // Move the function result into eax
@@ -2873,7 +2873,7 @@ void Ia32CodeGenerator::VisitReturnStatement(ReturnStatement* node) {
 
 void Ia32CodeGenerator::VisitWithEnterStatement(WithEnterStatement* node) {
   Comment cmnt(masm_, "[ WithEnterStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   Load(node->expression());
   __ CallRuntime(Runtime::kPushContext, 1);
 
@@ -2902,7 +2902,7 @@ void Ia32CodeGenerator::VisitWithExitStatement(WithExitStatement* node) {
 
 void Ia32CodeGenerator::VisitSwitchStatement(SwitchStatement* node) {
   Comment cmnt(masm_, "[ SwitchStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   node->set_break_stack_height(break_stack_height_);
 
   Load(node->tag());
@@ -2966,7 +2966,7 @@ void Ia32CodeGenerator::VisitSwitchStatement(SwitchStatement* node) {
 
 void Ia32CodeGenerator::VisitLoopStatement(LoopStatement* node) {
   Comment cmnt(masm_, "[ LoopStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   node->set_break_stack_height(break_stack_height_);
 
   // simple condition analysis
@@ -3007,7 +3007,8 @@ void Ia32CodeGenerator::VisitLoopStatement(LoopStatement* node) {
     // Record source position of the statement as this code which is after the
     // code for the body actually belongs to the loop statement and not the
     // body.
-    if (FLAG_debug_info) __ RecordPosition(node->statement_pos());
+    RecordStatementPosition(node);
+    __ RecordPosition(node->statement_pos());
     ASSERT(node->type() == LoopStatement::FOR_LOOP);
     Visit(node->next());
   }
@@ -3034,7 +3035,7 @@ void Ia32CodeGenerator::VisitLoopStatement(LoopStatement* node) {
 
 void Ia32CodeGenerator::VisitForInStatement(ForInStatement* node) {
   Comment cmnt(masm_, "[ ForInStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
 
   // We keep stuff on the stack while the body is executing.
   // Record it, so that a break/continue crossing this statement
@@ -3446,7 +3447,7 @@ void Ia32CodeGenerator::VisitTryFinally(TryFinally* node) {
 
 void Ia32CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) {
   Comment cmnt(masm_, "[ DebuggerStatement");
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   __ CallRuntime(Runtime::kDebugBreak, 1);
   __ push(eax);
 }
@@ -3809,7 +3810,7 @@ bool Ia32CodeGenerator::IsInlineSmi(Literal* literal) {
 void Ia32CodeGenerator::VisitAssignment(Assignment* node) {
   Comment cmnt(masm_, "[ Assignment");
 
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
   Reference target(this, node->target());
   if (target.is_illegal()) return;
 
@@ -3877,7 +3878,7 @@ void Ia32CodeGenerator::VisitCall(Call* node) {
 
   ZoneList<Expression*>* args = node->arguments();
 
-  if (FLAG_debug_info) RecordStatementPosition(node);
+  RecordStatementPosition(node);
 
   // Check if the function is a variable or a property.
   Expression* function = node->expression();

src/codegen.cc

@@ -37,8 +37,8 @@ namespace v8 { namespace internal {
 DeferredCode::DeferredCode(CodeGenerator* generator)
   : masm_(generator->masm()),
     generator_(generator),
-    position_(masm_->last_position()),
-    position_is_statement_(masm_->last_position_is_statement()) {
+    statement_position_(masm_->last_statement_position()),
+    position_(masm_->last_position()) {
   generator->AddDeferred(this);
 #ifdef DEBUG
   comment_ = "";
@@ -51,9 +51,10 @@ void CodeGenerator::ProcessDeferred() {
     DeferredCode* code = deferred_.RemoveLast();
     MacroAssembler* masm = code->masm();
     // Record position of deferred code stub.
-    if (code->position_is_statement()) {
-      masm->RecordStatementPosition(code->position());
-    } else {
+    if (code->statement_position() != kNoPosition) {
+      masm->RecordStatementPosition(code->statement_position());
+    }
+    if (code->position() != kNoPosition) {
       masm->RecordPosition(code->position());
     }
     // Bind labels and generate the code.

src/codegen.h

@@ -63,8 +63,8 @@ class DeferredCode: public ZoneObject {
   Label* enter() { return &enter_; }
   Label* exit() { return &exit_; }
 
+  int statement_position() const { return statement_position_; }
   int position() const { return position_; }
-  bool position_is_statement() const { return position_is_statement_; }
 
 #ifdef DEBUG
   void set_comment(const char* comment) { comment_ = comment; }
@@ -84,8 +84,8 @@ class DeferredCode: public ZoneObject {
   CodeGenerator* const generator_;
   Label enter_;
   Label exit_;
+  int statement_position_;
   int position_;
-  bool position_is_statement_;
 #ifdef DEBUG
   const char* comment_;
 #endif

src/debug.cc

@@ -94,14 +94,16 @@ void BreakLocationIterator::Next() {
     first = false;
     if (RinfoDone()) return;
 
-    // Update the current source position each time a source position is
-    // passed.
+    // Whenever a statement position or (plain) position is passed update the
+    // current value of these.
     if (is_position(rmode())) {
-      position_ = rinfo()->data() - debug_info_->shared()->start_position();
       if (is_statement_position(rmode())) {
         statement_position_ =
             rinfo()->data() - debug_info_->shared()->start_position();
       }
+      // Always update the position as we don't want that to be before the
+      // statement position.
+      position_ = rinfo()->data() - debug_info_->shared()->start_position();
       ASSERT(position_ >= 0);
       ASSERT(statement_position_ >= 0);
     }
@@ -978,7 +980,7 @@ bool Debug::StepNextContinue(BreakLocationIterator* break_location_iterator,
     int current_statement_position =
         break_location_iterator->code()->SourceStatementPosition(frame->pc());
     return thread_local_.last_fp_ == frame->fp() &&
-           thread_local_.last_statement_position_ == current_statement_position;
+        thread_local_.last_statement_position_ == current_statement_position;
   }
 
   // No step next action - don't continue.

src/objects.cc

@@ -4133,9 +4133,19 @@ int Code::SourcePosition(Address pc) {
   // source.
   RelocIterator it(this, RelocInfo::kPositionMask);
   while (!it.done()) {
-    if (it.rinfo()->pc() < pc && (pc - it.rinfo()->pc()) < distance) {
-      position = it.rinfo()->data();
-      distance = pc - it.rinfo()->pc();
+    // Only look at positions after the current pc.
+    if (it.rinfo()->pc() < pc) {
+      // Get position and distance.
+      int dist = pc - it.rinfo()->pc();
+      int pos = it.rinfo()->data();
+      // If this position is closer than the current candidate or if it has the
+      // same distance as the current candidate and the position is higher then
+      // this position is the new candidate.
+      if ((dist < distance) ||
+          (dist == distance && pos > position)) {
+        position = pos;
+        distance = dist;
+      }
     }
     it.next();
   }