[Interpreter] Log source positions for bytecode arrays.

Add support to log source position offsets to the profiler. As part of
this change PositionsRecorder is split into two, with the subset needed
by log.cc moved into log.h and the remainder kept in assembler.h as
AssemblerPositionsRecorder. The interpreter's source position table
builder is updated to log positions when the profiler is active.

BUG=v8:4766
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#34416}

src/arm/assembler-arm.h

@@ -1390,7 +1390,9 @@ class Assembler : public AssemblerBase {
   // Emits the address of the code stub's first instruction.
   void emit_code_stub_address(Code* stub);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   // Read/patch instructions
   Instr instr_at(int pos) { return *reinterpret_cast<Instr*>(buffer_ + pos); }
@@ -1637,8 +1639,8 @@ class Assembler : public AssemblerBase {
   friend class RelocInfo;
   friend class CodePatcher;
   friend class BlockConstPoolScope;
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
   friend class EnsureSpace;
 };
 

src/arm64/assembler-arm64.h

@@ -922,7 +922,9 @@ class Assembler : public AssemblerBase {
   }
 
   // Debugging ----------------------------------------------------------------
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
   void RecordComment(const char* msg);
 
   // Record a deoptimization reason that can be used by a log or cpu profiler.
@@ -2135,8 +2137,8 @@ class Assembler : public AssemblerBase {
   void DeleteUnresolvedBranchInfoForLabelTraverse(Label* label);
 
  private:
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
   friend class EnsureSpace;
   friend class ConstPool;
 };

src/assembler.cc

@@ -1648,8 +1648,7 @@ std::ostream& operator<<(std::ostream& os, ExternalReference reference) {
   return os;
 }
 
-
-void PositionsRecorder::RecordPosition(int pos) {
+void AssemblerPositionsRecorder::RecordPosition(int pos) {
   DCHECK(pos != RelocInfo::kNoPosition);
   DCHECK(pos >= 0);
   state_.current_position = pos;
@@ -1659,8 +1658,7 @@ void PositionsRecorder::RecordPosition(int pos) {
                                                  pos));
 }
 
-
-void PositionsRecorder::RecordStatementPosition(int pos) {
+void AssemblerPositionsRecorder::RecordStatementPosition(int pos) {
   DCHECK(pos != RelocInfo::kNoPosition);
   DCHECK(pos >= 0);
   state_.current_statement_position = pos;
@@ -1671,8 +1669,7 @@ void PositionsRecorder::RecordStatementPosition(int pos) {
                      pos));
 }
 
-
-bool PositionsRecorder::WriteRecordedPositions() {
+bool AssemblerPositionsRecorder::WriteRecordedPositions() {
   bool written = false;
 
   // Write the statement position if it is different from what was written last

src/assembler.h

@@ -38,6 +38,7 @@
 #include "src/allocation.h"
 #include "src/builtins.h"
 #include "src/isolate.h"
+#include "src/log.h"
 #include "src/runtime/runtime.h"
 
 namespace v8 {
@@ -1085,23 +1086,11 @@ struct PositionState {
   int written_statement_position;
 };
 
-
-class PositionsRecorder BASE_EMBEDDED {
+class AssemblerPositionsRecorder : public PositionsRecorder {
  public:
-  explicit PositionsRecorder(Assembler* assembler)
-      : assembler_(assembler) {
-    jit_handler_data_ = NULL;
-  }
-
-  void AttachJITHandlerData(void* user_data) {
-    jit_handler_data_ = user_data;
-  }
+  explicit AssemblerPositionsRecorder(Assembler* assembler)
+      : assembler_(assembler) {}
 
-  void* DetachJITHandlerData() {
-    void* old_data = jit_handler_data_;
-    jit_handler_data_ = NULL;
-    return old_data;
-  }
   // Set current position to pos.
   void RecordPosition(int pos);
 
@@ -1121,11 +1110,7 @@ class PositionsRecorder BASE_EMBEDDED {
   Assembler* assembler_;
   PositionState state_;
 
-  // Currently jit_handler_data_ is used to store JITHandler-specific data
-  // over the lifetime of a PositionsRecorder
-  void* jit_handler_data_;
-
-  DISALLOW_COPY_AND_ASSIGN(PositionsRecorder);
+  DISALLOW_COPY_AND_ASSIGN(AssemblerPositionsRecorder);
 };
 
 

src/ia32/assembler-ia32.h

@@ -1435,7 +1435,9 @@ class Assembler : public AssemblerBase {
 
   static bool IsNop(Address addr);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   int relocation_writer_size() {
     return (buffer_ + buffer_size_) - reloc_info_writer.pos();
@@ -1542,8 +1544,8 @@ class Assembler : public AssemblerBase {
   // code generation
   RelocInfoWriter reloc_info_writer;
 
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
 };
 
 

src/interpreter/bytecode-array-builder.cc

@@ -91,6 +91,8 @@ BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone,
   return_position_ =
       literal ? std::max(literal->start_position(), literal->end_position() - 1)
               : RelocInfo::kNoPosition;
+  LOG_CODE_EVENT(isolate_, CodeStartLinePosInfoRecordEvent(
+                               source_position_table_builder()));
 }
 
 BytecodeArrayBuilder::~BytecodeArrayBuilder() { DCHECK_EQ(0, unbound_jumps_); }
@@ -130,13 +132,18 @@ Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() {
   Handle<FixedArray> handler_table = handler_table_builder()->ToHandlerTable();
   Handle<ByteArray> source_position_table =
       source_position_table_builder()->ToSourcePositionTable();
-  Handle<BytecodeArray> output = isolate_->factory()->NewBytecodeArray(
+  Handle<BytecodeArray> bytecode_array = isolate_->factory()->NewBytecodeArray(
       bytecode_size, &bytecodes_.front(), frame_size, parameter_count(),
       constant_pool);
-  output->set_handler_table(*handler_table);
-  output->set_source_position_table(*source_position_table);
+  bytecode_array->set_handler_table(*handler_table);
+  bytecode_array->set_source_position_table(*source_position_table);
+
+  void* line_info = source_position_table_builder()->DetachJITHandlerData();
+  LOG_CODE_EVENT(isolate_, CodeEndLinePosInfoRecordEvent(
+                               AbstractCode::cast(*bytecode_array), line_info));
+
   bytecode_generated_ = true;
-  return output;
+  return bytecode_array;
 }
 
 

src/interpreter/source-position-table.cc

@@ -117,12 +117,20 @@ void DecodeEntry(ByteArray* bytes, int* index, PositionTableEntry* entry) {
 
 void SourcePositionTableBuilder::AddStatementPosition(size_t bytecode_offset,
                                                       int source_position) {
-  AddEntry({static_cast<int>(bytecode_offset), source_position, true});
+  int offset = static_cast<int>(bytecode_offset);
+  AddEntry({offset, source_position, true});
+  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddStatementPositionEvent(
+                               jit_handler_data_, offset, source_position));
+  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddPositionEvent(
+                               jit_handler_data_, offset, source_position));
 }
 
 void SourcePositionTableBuilder::AddExpressionPosition(size_t bytecode_offset,
                                                        int source_position) {
-  AddEntry({static_cast<int>(bytecode_offset), source_position, false});
+  int offset = static_cast<int>(bytecode_offset);
+  AddEntry({offset, source_position, false});
+  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddPositionEvent(
+                               jit_handler_data_, offset, source_position));
 }
 
 void SourcePositionTableBuilder::AddEntry(const PositionTableEntry& entry) {

src/interpreter/source-position-table.h

@@ -8,6 +8,7 @@
 #include "src/assert-scope.h"
 #include "src/checks.h"
 #include "src/handles.h"
+#include "src/log.h"
 #include "src/zone-containers.h"
 
 namespace v8 {
@@ -33,7 +34,7 @@ struct PositionTableEntry {
   bool is_statement;
 };
 
-class SourcePositionTableBuilder {
+class SourcePositionTableBuilder : public PositionsRecorder {
  public:
   explicit SourcePositionTableBuilder(Isolate* isolate, Zone* zone)
       : isolate_(isolate),

src/log.h

@@ -441,6 +441,26 @@ class TimerEventScope {
   Isolate* isolate_;
 };
 
+class PositionsRecorder BASE_EMBEDDED {
+ public:
+  PositionsRecorder() { jit_handler_data_ = NULL; }
+
+  void AttachJITHandlerData(void* user_data) { jit_handler_data_ = user_data; }
+
+  void* DetachJITHandlerData() {
+    void* old_data = jit_handler_data_;
+    jit_handler_data_ = NULL;
+    return old_data;
+  }
+
+ protected:
+  // Currently jit_handler_data_ is used to store JITHandler-specific data
+  // over the lifetime of a PositionsRecorder
+  void* jit_handler_data_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PositionsRecorder);
+};
 
 class CodeEventListener {
  public:

src/mips/assembler-mips.h

@@ -1048,7 +1048,9 @@ class Assembler : public AssemblerBase {
   void dp(uintptr_t data) { dd(data); }
   void dd(Label* label);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   // Postpone the generation of the trampoline pool for the specified number of
   // instructions.
@@ -1441,8 +1443,8 @@ class Assembler : public AssemblerBase {
   friend class CodePatcher;
   friend class BlockTrampolinePoolScope;
 
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
   friend class EnsureSpace;
 };
 

src/mips64/assembler-mips64.h

@@ -1107,7 +1107,9 @@ class Assembler : public AssemblerBase {
   void dp(uintptr_t data) { dq(data); }
   void dd(Label* label);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   // Postpone the generation of the trampoline pool for the specified number of
   // instructions.
@@ -1490,8 +1492,8 @@ class Assembler : public AssemblerBase {
   friend class CodePatcher;
   friend class BlockTrampolinePoolScope;
 
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
   friend class EnsureSpace;
 };
 

src/ppc/assembler-ppc.h

@@ -1216,7 +1216,9 @@ class Assembler : public AssemblerBase {
   void dq(uint64_t data);
   void dp(uintptr_t data);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   // Read/patch instructions
   Instr instr_at(int pos) { return *reinterpret_cast<Instr*>(buffer_ + pos); }
@@ -1463,8 +1465,8 @@ class Assembler : public AssemblerBase {
   friend class RelocInfo;
   friend class CodePatcher;
   friend class BlockTrampolinePoolScope;
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
   friend class EnsureSpace;
 };
 

src/profiler/cpu-profiler.cc

@@ -287,8 +287,15 @@ void CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag,
         }
       }
     } else {
-      DCHECK(abstract_code->IsBytecodeArray());
-      // TODO(rmcilroy): source position tracking for bytecode arrays.
+      BytecodeArray* bytecode = abstract_code->GetBytecodeArray();
+      line_table = new JITLineInfoTable();
+      interpreter::SourcePositionTableIterator it(
+          bytecode->source_position_table());
+      for (; !it.done(); it.Advance()) {
+        int line_number = script->GetLineNumber(it.source_position()) + 1;
+        int pc_offset = it.bytecode_offset() + BytecodeArray::kHeaderSize;
+        line_table->SetPosition(pc_offset, line_number);
+      }
     }
   }
   rec->entry = profiles_->NewCodeEntry(

src/x64/assembler-x64.h

@@ -1695,7 +1695,9 @@ class Assembler : public AssemblerBase {
   void dp(uintptr_t data) { dq(data); }
   void dq(Label* label);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   // 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
@@ -2178,8 +2180,8 @@ class Assembler : public AssemblerBase {
 
   List< Handle<Code> > code_targets_;
 
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
 };
 
 

src/x87/assembler-x87.h

@@ -961,7 +961,9 @@ class Assembler : public AssemblerBase {
 
   static bool IsNop(Address addr);
 
-  PositionsRecorder* positions_recorder() { return &positions_recorder_; }
+  AssemblerPositionsRecorder* positions_recorder() {
+    return &positions_recorder_;
+  }
 
   int relocation_writer_size() {
     return (buffer_ + buffer_size_) - reloc_info_writer.pos();
@@ -1048,8 +1050,8 @@ class Assembler : public AssemblerBase {
   // code generation
   RelocInfoWriter reloc_info_writer;
 
-  PositionsRecorder positions_recorder_;
-  friend class PositionsRecorder;
+  AssemblerPositionsRecorder positions_recorder_;
+  friend class AssemblerPositionsRecorder;
 };
 
 

test/cctest/cctest.status

@@ -130,7 +130,6 @@
   'test-cpu-profiler/DeoptAtFirstLevelInlinedSource': [PASS, NO_VARIANTS],
   'test-cpu-profiler/DeoptAtSecondLevelInlinedSource': [PASS, NO_VARIANTS],
   'test-cpu-profiler/DeoptUntrackedFunction': [PASS, NO_VARIANTS],
-  'test-cpu-profiler/TickLines': [PASS, NO_VARIANTS],
 
   ############################################################################
   # Slow tests.
@@ -544,6 +543,10 @@
   'test-run-jsexceptions/ThrowMessagePosition': [FAIL],
   'test-api/TryCatchMixedNesting': [FAIL],
 
+  # TODO(rmcilroy,4766): Requires BytecodeGraphBuilder to track source position
+  # on nodes (behind --turbo_source_positions flag).
+  'test-cpu-profiler/TickLinesOptimized': [FAIL],
+
   # TODO(rmcilroy,4680): Test assert errors.
   'test-feedback-vector/VectorCallICStates': [FAIL],
   'test-compiler/FeedbackVectorPreservedAcrossRecompiles': [FAIL],

test/cctest/test-cpu-profiler.cc

@@ -983,11 +983,11 @@ TEST(BoundFunctionCall) {
   profile->Delete();
 }
 
-
 // This tests checks distribution of the samples through the source lines.
-TEST(TickLines) {
+static void TickLines(bool optimize) {
   CcTest::InitializeVM();
   LocalContext env;
+  i::FLAG_allow_natives_syntax = true;
   i::FLAG_turbo_source_positions = true;
   i::Isolate* isolate = CcTest::i_isolate();
   i::Factory* factory = isolate->factory();
@@ -996,6 +996,8 @@ TEST(TickLines) {
   i::EmbeddedVector<char, 512> script;
 
   const char* func_name = "func";
+  const char* opt_func =
+      optimize ? "%OptimizeFunctionOnNextCall" : "%NeverOptimizeFunction";
   i::SNPrintF(script,
               "function %s() {\n"
               "  var n = 0;\n"
@@ -1005,8 +1007,9 @@ TEST(TickLines) {
               "    n += m * m * m;\n"
               "  }\n"
               "}\n"
+              "%s(%s);\n"
               "%s();\n",
-              func_name, func_name);
+              func_name, opt_func, func_name, func_name);
 
   CompileRun(script.start());
 
@@ -1014,14 +1017,7 @@ TEST(TickLines) {
       v8::Utils::OpenHandle(*GetFunction(env.local(), func_name)));
   CHECK(func->shared());
   CHECK(func->shared()->abstract_code());
-  i::AbstractCode* code = NULL;
-  if (func->abstract_code()->kind() == i::AbstractCode::OPTIMIZED_FUNCTION) {
-    code = func->abstract_code();
-  } else {
-    CHECK(func->shared()->abstract_code() == func->abstract_code() ||
-          !i::FLAG_crankshaft);
-    code = func->shared()->abstract_code();
-  }
+  i::AbstractCode* code = func->abstract_code();
   CHECK(code);
   i::Address code_address = code->instruction_start();
   CHECK(code_address);
@@ -1082,6 +1078,10 @@ TEST(TickLines) {
   CHECK_EQ(hit_count, value);
 }
 
+TEST(TickLinesBaseline) { TickLines(false); }
+
+TEST(TickLinesOptimized) { TickLines(true); }
+
 static const char* call_function_test_source =
     "%NeverOptimizeFunction(bar);\n"
     "%NeverOptimizeFunction(start);\n"