[turbofan] Clean up cached nodes in JSGraph.

R=mstarzinger@chromium.org
BUG=

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

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

src/compiler/control-reducer.cc

@@ -100,7 +100,7 @@ class ControlReducerImpl {
     // Gather all nodes backwards-reachable from end (through inputs).
     ReachabilityMarker marked(graph());
     NodeVector nodes(zone_);
-    AddNodesReachableFromEnd(marked, nodes);
+    AddNodesReachableFromRoots(marked, nodes);
 
     // Walk forward through control nodes, looking for back edges to nodes
     // that are not connected to end. Those are non-terminating loops (NTLs).
@@ -158,7 +158,6 @@ class ControlReducerImpl {
     }
 
     // Trim references from dead nodes to live nodes first.
-    jsgraph_->GetCachedNodes(&nodes);
     TrimNodes(marked, nodes);
 
     // Any control nodes not reachable from start are dead, even loops.
@@ -251,13 +250,14 @@ class ControlReducerImpl {
     return ret;
   }
 
-  void AddNodesReachableFromEnd(ReachabilityMarker& marked, NodeVector& nodes) {
+  void AddNodesReachableFromRoots(ReachabilityMarker& marked,
+                                  NodeVector& nodes) {
+    jsgraph_->GetCachedNodes(&nodes);  // Consider cached nodes roots.
     Node* end = graph()->end();
     marked.SetReachableFromEnd(end);
-    if (!end->IsDead()) {
+    if (!end->IsDead()) nodes.push_back(end);  // Consider end to be a root.
-      nodes.push_back(end);
+    for (Node* node : nodes) marked.SetReachableFromEnd(node);
-      AddBackwardsReachableNodes(marked, nodes, nodes.size() - 1);
+    AddBackwardsReachableNodes(marked, nodes, 0);
-    }
   }
 
   void AddBackwardsReachableNodes(ReachabilityMarker& marked, NodeVector& nodes,
@@ -276,10 +276,8 @@ class ControlReducerImpl {
     // Gather all nodes backwards-reachable from end through inputs.
     ReachabilityMarker marked(graph());
     NodeVector nodes(zone_);
-    AddNodesReachableFromEnd(marked, nodes);
-
-    // Process cached nodes in the JSGraph too.
     jsgraph_->GetCachedNodes(&nodes);
+    AddNodesReachableFromRoots(marked, nodes);
     TrimNodes(marked, nodes);
   }
 

src/compiler/js-graph.cc

@@ -17,77 +17,60 @@ Node* JSGraph::ImmovableHeapConstant(Handle<HeapObject> object) {
 }
 
 
+#define CACHED(name, expr) \
+  cached_nodes_[name] ? cached_nodes_[name] : (cached_nodes_[name] = (expr))
+
+
 Node* JSGraph::CEntryStubConstant(int result_size) {
   if (result_size == 1) {
-    if (!c_entry_stub_constant_.is_set()) {
+    return CACHED(kCEntryStubConstant,
-      c_entry_stub_constant_.set(
+                  ImmovableHeapConstant(CEntryStub(isolate(), 1).GetCode()));
-          ImmovableHeapConstant(CEntryStub(isolate(), 1).GetCode()));
-    }
-    return c_entry_stub_constant_.get();
   }
-
   return ImmovableHeapConstant(CEntryStub(isolate(), result_size).GetCode());
 }
 
 
 Node* JSGraph::UndefinedConstant() {
-  if (!undefined_constant_.is_set()) {
+  return CACHED(kUndefinedConstant,
-    undefined_constant_.set(
+                ImmovableHeapConstant(factory()->undefined_value()));
-        ImmovableHeapConstant(factory()->undefined_value()));
-  }
-  return undefined_constant_.get();
 }
 
 
 Node* JSGraph::TheHoleConstant() {
-  if (!the_hole_constant_.is_set()) {
+  return CACHED(kTheHoleConstant,
-    the_hole_constant_.set(ImmovableHeapConstant(factory()->the_hole_value()));
+                ImmovableHeapConstant(factory()->the_hole_value()));
-  }
-  return the_hole_constant_.get();
 }
 
 
 Node* JSGraph::TrueConstant() {
-  if (!true_constant_.is_set()) {
+  return CACHED(kTrueConstant, ImmovableHeapConstant(factory()->true_value()));
-    true_constant_.set(ImmovableHeapConstant(factory()->true_value()));
-  }
-  return true_constant_.get();
 }
 
 
 Node* JSGraph::FalseConstant() {
-  if (!false_constant_.is_set()) {
+  return CACHED(kFalseConstant,
-    false_constant_.set(ImmovableHeapConstant(factory()->false_value()));
+                ImmovableHeapConstant(factory()->false_value()));
-  }
-  return false_constant_.get();
 }
 
 
 Node* JSGraph::NullConstant() {
-  if (!null_constant_.is_set()) {
+  return CACHED(kNullConstant, ImmovableHeapConstant(factory()->null_value()));
-    null_constant_.set(ImmovableHeapConstant(factory()->null_value()));
-  }
-  return null_constant_.get();
 }
 
 
 Node* JSGraph::ZeroConstant() {
-  if (!zero_constant_.is_set()) zero_constant_.set(NumberConstant(0.0));
+  return CACHED(kZeroConstant, NumberConstant(0.0));
-  return zero_constant_.get();
 }
 
 
 Node* JSGraph::OneConstant() {
-  if (!one_constant_.is_set()) one_constant_.set(NumberConstant(1.0));
+  return CACHED(kOneConstant, NumberConstant(1.0));
-  return one_constant_.get();
 }
 
 
 Node* JSGraph::NaNConstant() {
-  if (!nan_constant_.is_set()) {
+  return CACHED(kNaNConstant,
-    nan_constant_.set(NumberConstant(std::numeric_limits<double>::quiet_NaN()));
+                NumberConstant(std::numeric_limits<double>::quiet_NaN()));
-  }
-  return nan_constant_.get();
 }
 
 
@@ -201,35 +184,27 @@ Node* JSGraph::ExternalConstant(ExternalReference reference) {
 
 
 Node* JSGraph::EmptyFrameState() {
-  if (!empty_frame_state_.is_set()) {
+  if (cached_nodes_[kEmptyFrameState] == nullptr) {
     Node* values = graph()->NewNode(common()->StateValues(0));
     Node* state_node = graph()->NewNode(
         common()->FrameState(JS_FRAME, BailoutId::None(),
                              OutputFrameStateCombine::Ignore()),
         values, values, values, NoContextConstant(), UndefinedConstant());
-    empty_frame_state_.set(state_node);
+    cached_nodes_[kEmptyFrameState] = state_node;
   }
-  return empty_frame_state_.get();
+  return cached_nodes_[kEmptyFrameState];
 }
 
 
 Node* JSGraph::DeadControl() {
-  if (!dead_control_.is_set()) {
+  return CACHED(kDeadControl, graph()->NewNode(common()->Dead()));
-    Node* dead_node = graph()->NewNode(common()->Dead());
-    dead_control_.set(dead_node);
-  }
-  return dead_control_.get();
 }
 
 
 void JSGraph::GetCachedNodes(NodeVector* nodes) {
   cache_.GetCachedNodes(nodes);
-  SetOncePointer<Node>* ptrs[] = {
+  for (size_t i = 0; i < arraysize(cached_nodes_); i++) {
-      &c_entry_stub_constant_, &undefined_constant_, &the_hole_constant_,
+    if (cached_nodes_[i]) nodes->push_back(cached_nodes_[i]);
-      &true_constant_,         &false_constant_,     &null_constant_,
-      &zero_constant_,         &one_constant_,       &nan_constant_};
-  for (size_t i = 0; i < arraysize(ptrs); i++) {
-    if (ptrs[i]->is_set()) nodes->push_back(ptrs[i]->get());
   }
 }
 

src/compiler/js-graph.h

@@ -30,7 +30,9 @@ class JSGraph : public ZoneObject {
         common_(common),
         javascript_(javascript),
         machine_(machine),
-        cache_(zone()) {}
+        cache_(zone()) {
+    for (int i = 0; i < kNumCachedNodes; i++) cached_nodes_[i] = nullptr;
+  }
 
   // Canonicalized global constants.
   Node* CEntryStubConstant(int result_size);
@@ -128,26 +130,28 @@ class JSGraph : public ZoneObject {
   void GetCachedNodes(NodeVector* nodes);
 
  private:
+  enum CachedNode {
+    kCEntryStubConstant,
+    kUndefinedConstant,
+    kTheHoleConstant,
+    kTrueConstant,
+    kFalseConstant,
+    kNullConstant,
+    kZeroConstant,
+    kOneConstant,
+    kNaNConstant,
+    kEmptyFrameState,
+    kDeadControl,
+    kNumCachedNodes  // Must remain last.
+  };
+
   Isolate* isolate_;
   Graph* graph_;
   CommonOperatorBuilder* common_;
   JSOperatorBuilder* javascript_;
   MachineOperatorBuilder* machine_;
-
-  // TODO(titzer): make this into a simple array.
-  SetOncePointer<Node> c_entry_stub_constant_;
-  SetOncePointer<Node> undefined_constant_;
-  SetOncePointer<Node> the_hole_constant_;
-  SetOncePointer<Node> true_constant_;
-  SetOncePointer<Node> false_constant_;
-  SetOncePointer<Node> null_constant_;
-  SetOncePointer<Node> zero_constant_;
-  SetOncePointer<Node> one_constant_;
-  SetOncePointer<Node> nan_constant_;
-  SetOncePointer<Node> empty_frame_state_;
-  SetOncePointer<Node> dead_control_;
-
   CommonNodeCache cache_;
+  Node* cached_nodes_[kNumCachedNodes];
 
   Node* ImmovableHeapConstant(Handle<HeapObject> value);
   Node* NumberConstant(double value);

test/cctest/compiler/test-control-reducer.cc

@@ -400,6 +400,24 @@ TEST(Trim_constants) {
 }
 
 
+TEST(Trim_EmptyFrameState1) {
+  ControlReducerTester T;
+
+  Node* node = T.jsgraph.EmptyFrameState();
+  T.Trim();
+
+  for (Node* input : node->inputs()) {
+    CHECK_NOT_NULL(input);
+  }
+}
+
+
+TEST(Trim_EmptyFrameState2) {
+  ControlReducerTester T;
+  CheckTrimConstant(&T, T.jsgraph.EmptyFrameState());
+}
+
+
 TEST(CReducePhi1) {
   ControlReducerTester R;