Improvements in positions handling in optimizing compiler.

- When building binary arithmetic op and comparison restore source position of the operation itself before building operation itself after it was changed by building operands. This ensures that position recorded for operation points to the operation token instead of pointing to the rightmost operand;

- Add support for recording operands' positions and use these positions when inserting HChange instructions;

- When generating hydrogen.cfg emit H-instruction position as BCI (previously 0 was emitted), additionally on every lithium instruction emit annotation pointing to corresponding hydrogen-instruction. This allows to easily reach from deopt_id to lithium instruction and from it to hydrogen instruction and source position.

BUG=
R=danno@chromium.org

Review URL: https://codereview.chromium.org/49203002

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

diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 9bd0b90..1167212 100644 (file)
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -642,6 +642,7 @@ class HValue : public ZoneObject {
   virtual ~HValue() {}
 
   virtual int position() const { return RelocInfo::kNoPosition; }
+  virtual int operand_position(int index) const { return position(); }
 
   HBasicBlock* block() const { return block_; }
   void SetBlock(HBasicBlock* block);
@@ -1105,6 +1106,102 @@ class HValue : public ZoneObject {
   }
 
 
+// A helper class to represent per-operand position information attached to
+// the HInstruction in the compact form. Uses tagging to distinguish between
+// case when only instruction's position is available and case when operands'
+// positions are also available.
+// In the first case it contains intruction's position as a tagged value.
+// In the second case it points to an array which contains instruction's
+// position and operands' positions.
+// TODO(vegorov): what we really want to track here is a combination of
+// source position and a script id because cross script inlining can easily
+// result in optimized functions composed of several scripts.
+class HPositionInfo {
+ public:
+  explicit HPositionInfo(int pos) : data_(TagPosition(pos)) { }
+
+  int position() const {
+    if (has_operand_positions()) {
+      return operand_positions()[kInstructionPosIndex];
+    }
+    return UntagPosition(data_);
+  }
+
+  void set_position(int pos) {
+    if (has_operand_positions()) {
+      operand_positions()[kInstructionPosIndex] = pos;
+    } else {
+      data_ = TagPosition(pos);
+    }
+  }
+
+  void ensure_storage_for_operand_positions(Zone* zone, int operand_count) {
+    if (has_operand_positions()) {
+      return;
+    }
+
+    const int length = kFirstOperandPosIndex + operand_count;
+    intptr_t* positions =
+        zone->NewArray<intptr_t>(length);
+    for (int i = 0; i < length; i++) {
+      positions[i] = RelocInfo::kNoPosition;
+    }
+
+    const int pos = position();
+    data_ = reinterpret_cast<intptr_t>(positions);
+    set_position(pos);
+
+    ASSERT(has_operand_positions());
+  }
+
+  int operand_position(int idx) const {
+    if (!has_operand_positions()) {
+      return position();
+    }
+    return static_cast<int>(*operand_position_slot(idx));
+  }
+
+  void set_operand_position(int idx, int pos) {
+    *operand_position_slot(idx) = pos;
+  }
+
+ private:
+  static const intptr_t kInstructionPosIndex = 0;
+  static const intptr_t kFirstOperandPosIndex = 1;
+
+  intptr_t* operand_position_slot(int idx) const {
+    ASSERT(has_operand_positions());
+    return &(operand_positions()[kFirstOperandPosIndex + idx]);
+  }
+
+  bool has_operand_positions() const {
+    return !IsTaggedPosition(data_);
+  }
+
+  intptr_t* operand_positions() const {
+    ASSERT(has_operand_positions());
+    return reinterpret_cast<intptr_t*>(data_);
+  }
+
+  static const intptr_t kPositionTag = 1;
+  static const intptr_t kPositionShift = 1;
+  static bool IsTaggedPosition(intptr_t val) {
+    return (val & kPositionTag) != 0;
+  }
+  static intptr_t UntagPosition(intptr_t val) {
+    ASSERT(IsTaggedPosition(val));
+    return val >> kPositionShift;
+  }
+  static intptr_t TagPosition(intptr_t val) {
+    const intptr_t result = (val << kPositionShift) | kPositionTag;
+    ASSERT(UntagPosition(result) == val);
+    return result;
+  }
+
+  intptr_t data_;
+};
+
+
 class HInstruction : public HValue {
  public:
   HInstruction* next() const { return next_; }
@@ -1119,12 +1216,26 @@ class HInstruction : public HValue {
   void InsertAfter(HInstruction* previous);
 
   // The position is a write-once variable.
-  virtual int position() const V8_OVERRIDE { return position_; }
-  bool has_position() const { return position_ != RelocInfo::kNoPosition; }
+  virtual int position() const V8_OVERRIDE {
+    return position_.position();
+  }
+  bool has_position() const {
+    return position_.position() != RelocInfo::kNoPosition;
+  }
   void set_position(int position) {
     ASSERT(!has_position());
     ASSERT(position != RelocInfo::kNoPosition);
-    position_ = position;
+    position_.set_position(position);
+  }
+
+  virtual int operand_position(int index) const V8_OVERRIDE {
+    const int pos = position_.operand_position(index);
+    return (pos != RelocInfo::kNoPosition) ? pos : position();
+  }
+  void set_operand_position(Zone* zone, int index, int pos) {
+    ASSERT(0 <= index && index < OperandCount());
+    position_.ensure_storage_for_operand_positions(zone, OperandCount());
+    position_.set_operand_position(index, pos);
   }
 
   bool CanTruncateToInt32() const { return CheckFlag(kTruncatingToInt32); }
@@ -1160,7 +1271,7 @@ class HInstruction : public HValue {
 
   HInstruction* next_;
   HInstruction* previous_;
-  int position_;
+  HPositionInfo position_;
 
   friend class HBasicBlock;
 };
@@ -3688,6 +3799,11 @@ class HBinaryOperation : public HTemplateInstruction<3> {
     return representation();
   }
 
+  void SetOperandPositions(Zone* zone, int left_pos, int right_pos) {
+    set_operand_position(zone, 1, left_pos);
+    set_operand_position(zone, 2, right_pos);
+  }
+
   DECLARE_ABSTRACT_INSTRUCTION(BinaryOperation)
 
  private:
@@ -4121,6 +4237,11 @@ class HCompareNumericAndBranch : public HTemplateControlInstruction<2, 2> {
   }
   virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE;
 
+  void SetOperandPositions(Zone* zone, int left_pos, int right_pos) {
+    set_operand_position(zone, 0, left_pos);
+    set_operand_position(zone, 1, right_pos);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(CompareNumericAndBranch)
 
  private:

diff --git a/src/hydrogen-representation-changes.cc b/src/hydrogen-representation-changes.cc
index d0c9b58..07fc8be 100644 (file)
--- a/src/hydrogen-representation-changes.cc
+++ b/src/hydrogen-representation-changes.cc
@@ -61,8 +61,8 @@ void HRepresentationChangesPhase::InsertRepresentationChangeForUse(
   if (new_value == NULL) {
     new_value = new(graph()->zone()) HChange(
         value, to, is_truncating_to_smi, is_truncating_to_int);
-    if (use_value->position() != RelocInfo::kNoPosition) {
-      new_value->set_position(use_value->position());
+    if (use_value->operand_position(use_index) != RelocInfo::kNoPosition) {
+      new_value->set_position(use_value->operand_position(use_index));
     } else {
       ASSERT(!FLAG_emit_opt_code_positions || !graph()->info()->IsOptimizing());
     }

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index b47ec80..72f276c 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -8627,10 +8627,14 @@ void HOptimizedGraphBuilder::VisitLogicalExpression(BinaryOperation* expr) {
 void HOptimizedGraphBuilder::VisitArithmeticExpression(BinaryOperation* expr) {
   CHECK_ALIVE(VisitForValue(expr->left()));
   CHECK_ALIVE(VisitForValue(expr->right()));
-  if (!FLAG_emit_opt_code_positions) SetSourcePosition(expr->position());
+  SetSourcePosition(expr->position());
   HValue* right = Pop();
   HValue* left = Pop();
   HInstruction* instr = BuildBinaryOperation(expr, left, right);
+  if (FLAG_emit_opt_code_positions && instr->IsBinaryOperation()) {
+    HBinaryOperation::cast(instr)->SetOperandPositions(
+        zone(), expr->left()->position(), expr->right()->position());
+  }
   return ast_context()->ReturnInstruction(instr, expr->id());
 }
 
@@ -8639,7 +8643,7 @@ void HOptimizedGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr,
                                                         Expression* sub_expr,
                                                         Handle<String> check) {
   CHECK_ALIVE(VisitForTypeOf(sub_expr));
-  if (!FLAG_emit_opt_code_positions) SetSourcePosition(expr->position());
+  SetSourcePosition(expr->position());
   HValue* value = Pop();
   HTypeofIsAndBranch* instr = New<HTypeofIsAndBranch>(value, check);
   return ast_context()->ReturnControl(instr, expr->id());
@@ -8701,6 +8705,8 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
   CHECK_ALIVE(VisitForValue(expr->left()));
   CHECK_ALIVE(VisitForValue(expr->right()));
 
+  if (FLAG_emit_opt_code_positions) SetSourcePosition(expr->position());
+
   HValue* right = Pop();
   HValue* left = Pop();
   Token::Value op = expr->op();
@@ -8779,6 +8785,10 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
           AddCheckMap(right, map);
           HCompareObjectEqAndBranch* result =
               New<HCompareObjectEqAndBranch>(left, right);
+          if (FLAG_emit_opt_code_positions) {
+            result->set_operand_position(zone(), 0, expr->left()->position());
+            result->set_operand_position(zone(), 1, expr->right()->position());
+          }
           return ast_context()->ReturnControl(result, expr->id());
         } else {
           BuildCheckHeapObject(left);
@@ -8820,6 +8830,11 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
       HCompareNumericAndBranch* result =
           New<HCompareNumericAndBranch>(left, right, op);
       result->set_observed_input_representation(left_rep, right_rep);
+      if (FLAG_emit_opt_code_positions) {
+        result->SetOperandPositions(zone(),
+                                    expr->left()->position(),
+                                    expr->right()->position());
+      }
       return ast_context()->ReturnControl(result, expr->id());
     }
   }
@@ -10136,7 +10151,8 @@ void HTracer::Trace(const char* name, HGraph* graph, LChunk* chunk) {
       Tag HIR_tag(this, "HIR");
       for (HInstructionIterator it(current); !it.Done(); it.Advance()) {
         HInstruction* instruction = it.Current();
-        int bci = 0;
+        int bci = FLAG_emit_opt_code_positions && instruction->has_position() ?
+            instruction->position() : 0;
         int uses = instruction->UseCount();
         PrintIndent();
         trace_.Add("%d %d ", bci, uses);
@@ -10161,6 +10177,9 @@ void HTracer::Trace(const char* name, HGraph* graph, LChunk* chunk) {
             trace_.Add("%d ",
                        LifetimePosition::FromInstructionIndex(i).Value());
             linstr->PrintTo(&trace_);
+            trace_.Add(" [hir:");
+            linstr->hydrogen_value()->PrintNameTo(&trace_);
+            trace_.Add("]");
             trace_.Add(" <|@\n");
           }
         }