[turbofan]: Improved source position information

Make sure the initial graph is fully populated with source position information and automatically propagate that information down through newly allocated nodes during reduction passes in the most unobtrusive way that's currently possible.

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

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

diff --git a/src/compiler/graph.cc b/src/compiler/graph.cc
index 1650bdf..1938611 100644 (file)
--- a/src/compiler/graph.cc
+++ b/src/compiler/graph.cc
@@ -22,8 +22,10 @@ Graph::Graph(Zone* zone)
       decorators_(zone) {}
 
 
-void Graph::Decorate(Node* node) {
-  for (auto const decorator : decorators_) decorator->Decorate(node);
+void Graph::Decorate(Node* node, bool incomplete) {
+  for (auto const decorator : decorators_) {
+    decorator->Decorate(node, incomplete);
+  }
 }
 
 
@@ -44,7 +46,7 @@ Node* Graph::NewNode(const Operator* op, int input_count, Node** inputs,
   DCHECK_LE(op->ValueInputCount(), input_count);
   Node* const node =
       Node::New(zone(), NextNodeId(), op, input_count, inputs, incomplete);
-  if (!incomplete) Decorate(node);
+  Decorate(node, incomplete);
   return node;
 }
 

diff --git a/src/compiler/graph.h b/src/compiler/graph.h
index e825b7e..882e549 100644 (file)
--- a/src/compiler/graph.h
+++ b/src/compiler/graph.h
@@ -82,7 +82,7 @@ class Graph : public ZoneObject {
 
   int NodeCount() const { return next_node_id_; }
 
-  void Decorate(Node* node);
+  void Decorate(Node* node, bool incomplete);
   void AddDecorator(GraphDecorator* decorator);
   void RemoveDecorator(GraphDecorator* decorator);
 
@@ -107,7 +107,7 @@ class Graph : public ZoneObject {
 class GraphDecorator : public ZoneObject {
  public:
   virtual ~GraphDecorator() {}
-  virtual void Decorate(Node* node) = 0;
+  virtual void Decorate(Node* node, bool incomplete) = 0;
 };
 
 }  // namespace compiler

diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
index 4d5982a..0153d62 100644 (file)
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -290,11 +290,11 @@ class AstGraphBuilderWithPositions : public AstGraphBuilder {
                                LoopAssignmentAnalysis* loop_assignment,
                                SourcePositionTable* source_positions)
       : AstGraphBuilder(local_zone, info, jsgraph, loop_assignment),
-        source_positions_(source_positions) {}
+        source_positions_(source_positions),
+        start_position_(info->shared_info()->start_position()) {}
 
   bool CreateGraph() {
-    SourcePositionTable::Scope pos(source_positions_,
-                                   SourcePosition::Unknown());
+    SourcePositionTable::Scope pos_scope(source_positions_, start_position_);
     return AstGraphBuilder::CreateGraph();
   }
 
@@ -311,9 +311,45 @@ class AstGraphBuilderWithPositions : public AstGraphBuilder {
 
  private:
   SourcePositionTable* source_positions_;
+  SourcePosition start_position_;
 };
 
 
+namespace {
+
+class SourcePositionWrapper : public Reducer {
+ public:
+  SourcePositionWrapper(Reducer* reducer, SourcePositionTable* table)
+      : reducer_(reducer), table_(table) {}
+  virtual ~SourcePositionWrapper() {}
+
+  virtual Reduction Reduce(Node* node) {
+    SourcePosition pos = table_->GetSourcePosition(node);
+    SourcePositionTable::Scope position(table_, pos);
+    return reducer_->Reduce(node);
+  }
+
+ private:
+  Reducer* reducer_;
+  SourcePositionTable* table_;
+
+  DISALLOW_COPY_AND_ASSIGN(SourcePositionWrapper);
+};
+
+
+static void AddReducer(PipelineData* data, GraphReducer* graph_reducer,
+                       Reducer* reducer) {
+  if (FLAG_turbo_source_positions) {
+    void* buffer = data->graph_zone()->New(sizeof(SourcePositionWrapper));
+    SourcePositionWrapper* wrapper =
+        new (buffer) SourcePositionWrapper(reducer, data->source_positions());
+    graph_reducer->AddReducer(wrapper);
+  } else {
+    graph_reducer->AddReducer(reducer);
+  }
+}
+}  // namespace
+
 class PipelineRunScope {
  public:
   PipelineRunScope(PipelineData* data, const char* phase_name)
@@ -382,7 +418,7 @@ struct ContextSpecializerPhase {
     JSContextSpecializer spec(data->info(), data->jsgraph(),
                               data->context_node());
     GraphReducer graph_reducer(data->graph(), temp_zone);
-    graph_reducer.AddReducer(&spec);
+    AddReducer(data, &graph_reducer, &spec);
     graph_reducer.ReduceGraph();
   }
 };
@@ -435,13 +471,13 @@ struct TypedLoweringPhase {
     SimplifiedOperatorReducer simple_reducer(data->jsgraph());
     CommonOperatorReducer common_reducer;
     GraphReducer graph_reducer(data->graph(), temp_zone);
-    graph_reducer.AddReducer(&vn_reducer);
-    graph_reducer.AddReducer(&builtin_reducer);
-    graph_reducer.AddReducer(&typed_lowering);
-    graph_reducer.AddReducer(&intrinsic_lowering);
-    graph_reducer.AddReducer(&load_elimination);
-    graph_reducer.AddReducer(&simple_reducer);
-    graph_reducer.AddReducer(&common_reducer);
+    AddReducer(data, &graph_reducer, &vn_reducer);
+    AddReducer(data, &graph_reducer, &builtin_reducer);
+    AddReducer(data, &graph_reducer, &typed_lowering);
+    AddReducer(data, &graph_reducer, &intrinsic_lowering);
+    AddReducer(data, &graph_reducer, &load_elimination);
+    AddReducer(data, &graph_reducer, &simple_reducer);
+    AddReducer(data, &graph_reducer, &common_reducer);
     graph_reducer.ReduceGraph();
   }
 };
@@ -453,17 +489,18 @@ struct SimplifiedLoweringPhase {
   void Run(PipelineData* data, Zone* temp_zone) {
     SourcePositionTable::Scope pos(data->source_positions(),
                                    SourcePosition::Unknown());
-    SimplifiedLowering lowering(data->jsgraph(), temp_zone);
+    SimplifiedLowering lowering(data->jsgraph(), temp_zone,
+                                data->source_positions());
     lowering.LowerAllNodes();
     ValueNumberingReducer vn_reducer(temp_zone);
     SimplifiedOperatorReducer simple_reducer(data->jsgraph());
     MachineOperatorReducer machine_reducer(data->jsgraph());
     CommonOperatorReducer common_reducer;
     GraphReducer graph_reducer(data->graph(), temp_zone);
-    graph_reducer.AddReducer(&vn_reducer);
-    graph_reducer.AddReducer(&simple_reducer);
-    graph_reducer.AddReducer(&machine_reducer);
-    graph_reducer.AddReducer(&common_reducer);
+    AddReducer(data, &graph_reducer, &vn_reducer);
+    AddReducer(data, &graph_reducer, &simple_reducer);
+    AddReducer(data, &graph_reducer, &machine_reducer);
+    AddReducer(data, &graph_reducer, &common_reducer);
     graph_reducer.ReduceGraph();
   }
 };
@@ -483,10 +520,10 @@ struct ChangeLoweringPhase {
     CommonOperatorReducer common_reducer;
     GraphReducer graph_reducer(data->graph(), temp_zone);
     graph_reducer.AddReducer(&vn_reducer);
-    graph_reducer.AddReducer(&simple_reducer);
-    graph_reducer.AddReducer(&lowering);
-    graph_reducer.AddReducer(&machine_reducer);
-    graph_reducer.AddReducer(&common_reducer);
+    AddReducer(data, &graph_reducer, &simple_reducer);
+    AddReducer(data, &graph_reducer, &lowering);
+    AddReducer(data, &graph_reducer, &machine_reducer);
+    AddReducer(data, &graph_reducer, &common_reducer);
     graph_reducer.ReduceGraph();
   }
 };
@@ -537,8 +574,8 @@ struct GenericLoweringPhase {
     JSGenericLowering generic(data->info(), data->jsgraph());
     SelectLowering select(data->jsgraph()->graph(), data->jsgraph()->common());
     GraphReducer graph_reducer(data->graph(), temp_zone);
-    graph_reducer.AddReducer(&generic);
-    graph_reducer.AddReducer(&select);
+    AddReducer(data, &graph_reducer, &generic);
+    AddReducer(data, &graph_reducer, &select);
     graph_reducer.ReduceGraph();
   }
 };

diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc
index de33f96..fc3b009 100644 (file)
--- a/src/compiler/simplified-lowering.cc
+++ b/src/compiler/simplified-lowering.cc
@@ -17,6 +17,7 @@
 #include "src/compiler/representation-change.h"
 #include "src/compiler/simplified-lowering.h"
 #include "src/compiler/simplified-operator.h"
+#include "src/compiler/source-position.h"
 #include "src/objects.h"
 
 namespace v8 {
@@ -65,7 +66,8 @@ class RepresentationSelector {
   };
 
   RepresentationSelector(JSGraph* jsgraph, Zone* zone,
-                         RepresentationChanger* changer)
+                         RepresentationChanger* changer,
+                         SourcePositionTable* source_positions)
       : jsgraph_(jsgraph),
         count_(jsgraph->graph()->NodeCount()),
         info_(zone->NewArray<NodeInfo>(count_)),
@@ -73,7 +75,8 @@ class RepresentationSelector {
         replacements_(zone),
         phase_(PROPAGATE),
         changer_(changer),
-        queue_(zone) {
+        queue_(zone),
+        source_positions_(source_positions) {
     memset(info_, 0, sizeof(NodeInfo) * count_);
 
     safe_int_additive_range_ =
@@ -106,7 +109,13 @@ class RepresentationSelector {
       Node* node = *i;
       TRACE((" visit #%d: %s\n", node->id(), node->op()->mnemonic()));
       // Reuse {VisitNode()} so the representation rules are in one place.
-      VisitNode(node, GetUseInfo(node), lowering);
+      if (FLAG_turbo_source_positions) {
+        SourcePositionTable::Scope scope(
+            source_positions_, source_positions_->GetSourcePosition(node));
+        VisitNode(node, GetUseInfo(node), lowering);
+      } else {
+        VisitNode(node, GetUseInfo(node), lowering);
+      }
     }
 
     // Perform the final replacements.
@@ -1071,6 +1080,12 @@ class RepresentationSelector {
   Phase phase_;                     // current phase of algorithm
   RepresentationChanger* changer_;  // for inserting representation changes
   ZoneQueue<Node*> queue_;          // queue for traversing the graph
+  // TODO(danno): RepresentationSelector shouldn't know anything about the
+  // source positions table, but must for now since there currently is no other
+  // way to pass down source position information to nodes created during
+  // lowering. Once this phase becomes a vanilla reducer, it should get source
+  // position information via the SourcePositionWrapper like all other reducers.
+  SourcePositionTable* source_positions_;
   Type* safe_int_additive_range_;
 
   NodeInfo* GetInfo(Node* node) {
@@ -1094,7 +1109,8 @@ Node* SimplifiedLowering::IsTagged(Node* node) {
 void SimplifiedLowering::LowerAllNodes() {
   SimplifiedOperatorBuilder simplified(graph()->zone());
   RepresentationChanger changer(jsgraph(), &simplified, jsgraph()->isolate());
-  RepresentationSelector selector(jsgraph(), zone_, &changer);
+  RepresentationSelector selector(jsgraph(), zone_, &changer,
+                                  source_positions_);
   selector.Run(this);
 }
 

diff --git a/src/compiler/simplified-lowering.h b/src/compiler/simplified-lowering.h
index b21cf21..11506e9 100644 (file)
--- a/src/compiler/simplified-lowering.h
+++ b/src/compiler/simplified-lowering.h
@@ -16,12 +16,13 @@ namespace compiler {
 
 // Forward declarations.
 class RepresentationChanger;
-
+class SourcePositionTable;
 
 class SimplifiedLowering FINAL {
  public:
-  SimplifiedLowering(JSGraph* jsgraph, Zone* zone)
-      : jsgraph_(jsgraph), zone_(zone) {}
+  SimplifiedLowering(JSGraph* jsgraph, Zone* zone,
+                     SourcePositionTable* source_positions)
+      : jsgraph_(jsgraph), zone_(zone), source_positions_(source_positions) {}
   ~SimplifiedLowering() {}
 
   void LowerAllNodes();
@@ -45,6 +46,13 @@ class SimplifiedLowering FINAL {
   JSGraph* const jsgraph_;
   Zone* const zone_;
 
+  // TODO(danno): SimplifiedLowering shouldn't know anything about the source
+  // positions table, but must for now since there currently is no other way to
+  // pass down source position information to nodes created during
+  // lowering. Once this phase becomes a vanilla reducer, it should get source
+  // position information via the SourcePositionWrapper like all other reducers.
+  SourcePositionTable* source_positions_;
+
   Node* SmiTag(Node* node);
   Node* IsTagged(Node* node);
   Node* Untag(Node* node);

diff --git a/src/compiler/source-position.cc b/src/compiler/source-position.cc
index 9e21ae4..50cba0b 100644 (file)
--- a/src/compiler/source-position.cc
+++ b/src/compiler/source-position.cc
@@ -15,7 +15,7 @@ class SourcePositionTable::Decorator FINAL : public GraphDecorator {
   explicit Decorator(SourcePositionTable* source_positions)
       : source_positions_(source_positions) {}
 
-  void Decorate(Node* node) FINAL {
+  void Decorate(Node* node, bool incomplete) FINAL {
     DCHECK(!source_positions_->current_position_.IsInvalid());
     source_positions_->table_.Set(node, source_positions_->current_position_);
   }

diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc
index 5fccdad..3bd0818 100644 (file)
--- a/src/compiler/typer.cc
+++ b/src/compiler/typer.cc
@@ -143,7 +143,7 @@ class LazyTypeCache FINAL : public ZoneObject {
 class Typer::Decorator FINAL : public GraphDecorator {
  public:
   explicit Decorator(Typer* typer) : typer_(typer) {}
-  void Decorate(Node* node) FINAL;
+  void Decorate(Node* node, bool incomplete) FINAL;
 
  private:
   Typer* typer_;
@@ -408,7 +408,8 @@ void Typer::Run() {
 }
 
 
-void Typer::Decorator::Decorate(Node* node) {
+void Typer::Decorator::Decorate(Node* node, bool incomplete) {
+  if (incomplete) return;
   if (node->op()->ValueOutputCount() > 0) {
     // Only eagerly type-decorate nodes with known input types.
     // Other cases will generally require a proper fixpoint iteration with Run.

diff --git a/test/cctest/compiler/test-simplified-lowering.cc b/test/cctest/compiler/test-simplified-lowering.cc
index b48324e..16dfd20 100644 (file)
--- a/test/cctest/compiler/test-simplified-lowering.cc
+++ b/test/cctest/compiler/test-simplified-lowering.cc
@@ -13,6 +13,7 @@
 #include "src/compiler/pipeline.h"
 #include "src/compiler/representation-change.h"
 #include "src/compiler/simplified-lowering.h"
+#include "src/compiler/source-position.h"
 #include "src/compiler/typer.h"
 #include "src/compiler/verifier.h"
 #include "src/execution.h"
@@ -40,11 +41,13 @@ class SimplifiedLoweringTester : public GraphBuilderTester<ReturnType> {
         javascript(this->zone()),
         jsgraph(this->isolate(), this->graph(), this->common(), &javascript,
                 this->machine()),
-        lowering(&jsgraph, this->zone()) {}
+        source_positions(jsgraph.graph()),
+        lowering(&jsgraph, this->zone(), &source_positions) {}
 
   Typer typer;
   JSOperatorBuilder javascript;
   JSGraph jsgraph;
+  SourcePositionTable source_positions;
   SimplifiedLowering lowering;
 
   void LowerAllNodes() {
@@ -699,7 +702,10 @@ class TestingGraph : public HandleAndZoneScope, public GraphAndBuilders {
     CHECK_EQ(expected, node->opcode());
   }
 
-  void Lower() { SimplifiedLowering(&jsgraph, jsgraph.zone()).LowerAllNodes(); }
+  void Lower() {
+    SourcePositionTable table(jsgraph.graph());
+    SimplifiedLowering(&jsgraph, jsgraph.zone(), &table).LowerAllNodes();
+  }
 
   // Inserts the node as the return value of the graph.
   Node* Return(Node* node) {