[turbofan] Improve the store-store elimination.

src/compiler/store-store-elimination.cc

Index: src/compiler/store-store-elimination.cc
diff --git a/src/compiler/store-store-elimination.cc b/src/compiler/store-store-elimination.cc
index 3b7e3e053e018e1ee194cafcb02845d528857051..ec00994f1fcd88a43e38cb0d2d550c77dfaa81b5 100644
--- a/src/compiler/store-store-elimination.cc
+++ b/src/compiler/store-store-elimination.cc
@@ -13,152 +13,132 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-#define TRACE(fmt, ...)                                              \
-  do {                                                               \
-    if (FLAG_trace_store_elimination) {                              \
-      PrintF("StoreStoreElimination::ReduceEligibleNode: " fmt "\n", \
-             ##__VA_ARGS__);                                         \
-    }                                                                \
+#define TRACE1(fmt, ...)                                              \
+  do {                                                                \
+    if (FLAG_trace_store_elimination) {                               \
+      PrintF("StoreStoreElimination::Run: " fmt "\n", ##__VA_ARGS__); \
+    }                                                                 \
   } while (false)
 
-// A simple store-store elimination. When the effect chain contains the
-// following sequence,
-//
-// - StoreField[[+off_1]](x1, y1)
-// - StoreField[[+off_2]](x2, y2)
-// - StoreField[[+off_3]](x3, y3)
-//   ...
-// - StoreField[[+off_n]](xn, yn)
-//
-// where the xes are the objects and the ys are the values to be stored, then
-// we are going to say that a store is superfluous if the same offset of the
-// same object will be stored to in the future. If off_i == off_j and xi == xj
-// and i < j, then we optimize the i'th StoreField away.
-//
-// This optimization should be initiated on the last StoreField in such a
-// sequence.
-//
-// The algorithm works by walking the effect chain from the last StoreField
-// upwards. While walking, we maintain a map {futureStore} from offsets to
-// nodes; initially it is empty. As we walk the effect chain upwards, if
-// futureStore[off] = n, then any store to node {n} with offset {off} is
-// guaranteed to be useless because we do a tagged-width[2] store to that
-// offset of that object in the near future anyway. For example, for this
-// effect chain
-//
-// 71: StoreField(60, 0)
-// 72: StoreField(65, 8)
-// 73: StoreField(63, 8)
-// 74: StoreField(65, 16)
-// 75: StoreField(62, 8)
-//
-// just before we get to 72, we will have futureStore = {8: 63, 16: 65}.
+#define TRACE2(level, fmt, ...)                                         \
+  do {                                                                  \
+    if (FLAG_trace_store_elimination &&                                 \
+        FLAG_trace_store_elimination_level >= (level)) {                \
+      PrintF("StoreStoreFinder: [%d] " fmt "\n", level, ##__VA_ARGS__); \
+    }                                                                   \
+  } while (false)

[Jarin, 2016/07/20 11:43:33]

This feels a bit over-engineered - normally, you would be the only one staring
at the diagnostic output; one general TRACE (no levels) should be enough.

[bgeron, 2016/07/20 16:27:16]

Because trace level 1 is ~37% as big as trace level 5. In level 1 it's very easy
to spot how often we revisit a chain, and it's easy to see what nodes observe
everything / observe nothing / are observable/unobservable StoreFields. I agree
that there currently are a lot of TRACE2s left though; should I try to cut them
down?

+
+// CHECK_EXTRA is like CHECK, but has two or more arguments: a boolean
+// expression, a format string, and any number of extra arguments. The boolean
+// expression will be evaluated at runtime. If it evaluates to false, then an
+// error message will be shown containing the condition, as well as the extra
+// info formatted like with printf.
+#define CHECK_EXTRA(condition, fmt, ...)                                 \
+  do {                                                                   \
+    if (V8_UNLIKELY(!(condition))) {                                     \
+      V8_Fatal(__FILE__, __LINE__, "Check failed: %s. Extra info: " fmt, \
+               #condition, ##__VA_ARGS__);                               \
+    }                                                                    \
+  } while (0)
+
+#ifdef DEBUG
+#define DCHECK_EXTRA(condition, fmt, ...) \
+  CHECK_EXTRA(condition, fmt, ##__VA_ARGS__)
+#else
+#define DCHECK_EXTRA(condition, fmt, ...) ((void)0)
+#endif
+
+// Store-store elimination.
 //
-// Here is the complete process.
+// The aim of this optimization is to detect the following pattern in the
+// effect graph:
 //
-// - We are at the end of a sequence of consecutive StoreFields.
-// - We start out with futureStore = empty.
-// - We then walk the effect chain upwards to find the next StoreField [1].
+// - StoreField[+24, kRepTagged](263, ...)
 //
-//   1. If the offset is not a key of {futureStore} yet, we put it in.
-//   2. If the offset is a key of {futureStore}, but futureStore[offset] is a
-//      different node, we overwrite futureStore[offset] with the current node.
-//   3. If the offset is a key of {futureStore} and futureStore[offset] equals
-//      this node, we eliminate this StoreField.
+//   ... lots of nodes from which the field at offset 24 of the object
+//       returned by node #263 cannot be observed ...
 //
-//   As long as the current effect input points to a node with a single effect
-//   output, and as long as its opcode is StoreField, we keep traversing
-//   upwards.
+// - StoreField[+24, kRepTagged](263, ...)
 //
+// In such situations, the earlier StoreField cannot be observed, and can be
+// eliminated. This optimization should work for any offset and input node, of
+// course.
 //
+// The optimization also works across splits. It currently does not work for
+// loops, because we tend to put a stack check in loops, and like deopts,
+// stack checks can observe anything.
 //
-//       footnotes:
+// The implementation consists of two phases. The first phase
+// (StoreStoreFinder) analyzes the graph, and at every point determines what
+// fields are guaranteed not to be observed in the future. The second phase
+// then eliminates all StoreField nodes that are known to be unobservable.
 //
-// [1] We make sure that we only traverse the linear part, that is, the part
-//   where every node has exactly one incoming and one outgoing effect edge.
-//   Also, we only keep walking upwards as long as we keep finding consecutive
-//   StoreFields on the same node.
+// Per node, we store a data structure called the UnobservablesSet, which is a
+// set of pairs of nodes and offsets. For instance, a Return always has an
+// empty UnobservablesSet, as does a deoptimization, throw, or stack check.
+// The UnobservablesSet of a StoreField consists of the node and the offset
+// that it stores to, as well as the UnobservablesSet of the next node in the
+// effect chain. When there are multiple succeeding nodes in the effect graph,
+// then the intersection of their UnobservablesSet is taken, and then the
+// offset of that StoreField is added.
 //
-// [2] This optimization is sound only in certain cases. Specifically, the
-//   presence of a future store to {off} by itself does not automatically mean
-//   that earlier stores to {off} are superfluous: a future narrow store does
-//   not obviate an earlier wide store. However, future stores of a certain
-//   size do obviate stores to the same offset of lesser or equal size.
+// To allow for cycles in the effect graph, the implementation does a fixpoint
+// computation. Unfortunately, we compute a least fixpoint not a greatest
+// fixpoint. In practical terms this means that if we have a loop without any
+// deopt or stack check, and a StoreField inside and after the loop, then we
+// will not detect that the StoreField inside the loop is unobservable. This
+// means that theoretically this implementation is slightly suboptimal.
+// However, in practice we always put a stack check inside a loop, so the
+// implementation should be optimal in practice.
 //
-//   It turns out that we are most interested in stores of "tagged" size,
-//   which is 8 bytes on 64-bit archs and 4 bit on 32-bit archs. In
-//   {futureStore}, we record future writes that are of at least this size.
-//   The three cases are actually a bit more subtle.
+// To implement this fixpoint, we use a special value
+// UnobservablesSet::Undetermined(), which is functionally empty. When a
+// node's UnobservablesSet is undetermined, then that means that we have no
+// knowledge about what fields are unobservable at that point. In contrast,
+// when a node's UnobservablesSet is determined but empty, and the node is not
+// marked for revisiting, then the node's effect input is determined and up-
+// to-date. Initially, the UnobservablesSet of all nodes is set to
+// undetermined.
 //
-//   1. If the offset is not a key of {futureStore} and the StoreField is of
-//      "tagged" size or wider, then we put it in.
-//   2. If the offset is present in {futureStore} but the value is different,
-//      then we overwrite the value if the current StoreField is of "tagged"
-//      size or wider.
-//   3. If the offset is present and the value matches the node, and the
-//      current StoreField is AT MOST of "tagged" size, then we eliminate this
-//      StoreField.
+// We apply some sharing to save memory. The class UnobservablesSet is only a
+// pointer wide, and a copy does not use any heap (or temp_zone) memory. Most
+// changes to an UnobservablesSet allocate in the temp_zone.

[Jarin, 2016/07/20 11:43:33]

This blurb should go into the UnobservablesSet class.

[bgeron, 2016/07/20 16:27:16]

Done.

+
+// An ideal store-store elimination would keep for every object a set of byte
+// ranges into that object which are unobservable. We skimp on this, and only
+// store a set of offsets. If offset {off} is in the set, then that means that
+// bytes {off} up to but excluding {off+taggedSize} are unobservable, where
+// {taggedSize} is the size in bytes of a tagged value. We don't record that
+// writes smaller than taggedSize are unobservable, and we don't optimize away
+// writes larger than taggedSize.
 //
-//   Examples of stores that we do not detect as superfluous: 2-byte stores
-//   followed by 2-byte stores to the same offset; 16-byte stores followed by
-//   16-byte stores to the same offset. On ia32, we do not detect consecutive
-//   float64 stores as superfluous, and on x86 we do not detect consecutive
-//   int32 stores as superfluous.
-
-// At a late stage, we realized that this code is more complicated than it
-// needs to be: if we store a set of pairs (offset, node), the code simplifies
-// to 3 cases instead of 6. We could even store a map from nodes to sets of
-// bytes.
-
-StoreStoreElimination::StoreStoreElimination(JSGraph* js_graph, Zone* temp_zone)
-    : jsgraph_(js_graph), temp_zone_(temp_zone) {}
-
-StoreStoreElimination::~StoreStoreElimination() {}
-
-void StoreStoreElimination::Run() {
-  // The store-store elimination performs work on chains of certain types of
-  // nodes. The elimination must be invoked on the lowest node in such a
-  // chain; we have a helper function IsEligibleNode that returns true
-  // precisely on the lowest node in such a chain.
-  //
-  // Because the elimination removes nodes from the graph, even remove nodes
-  // that the elimination was not invoked on, we cannot use a normal
-  // AdvancedReducer but we manually find which nodes to invoke the
-  // elimination on. Then in a next step, we invoke the elimination for each
-  // node that was eligible.
-
-  NodeVector eligible(temp_zone());  // loops over all nodes
-  AllNodes all(temp_zone(), jsgraph()->graph());
-
-  for (Node* node : all.live) {
-    if (IsEligibleNode(node)) {
-      eligible.push_back(node);
-    }
-  }
+// The result should be that this store-store elimination is fast enough, and
+// also optimizes away most superfluous stores.
 
-  for (Node* node : eligible) {
-    ReduceEligibleNode(node);
-  }
-}
+// Assumption: every byte of a JS object is only ever accessed through one
+// offset. For instance, byte 15 of a given object may be accessed using a
+// two-byte read at offset 14, or a four-byte read at offset 12, but never
+// both in the same program.
 
-namespace {
+// This implementation needs all dead nodes removed from the graph, and the
+// graph should be trimmed.
 
-// 16 bits was chosen fairly arbitrarily; it seems enough now. 8 bits is too
-// few.
-typedef uint16_t Offset;
+namespace {
 
 // To safely cast an offset from a FieldAccess, which has a wider range
 // (namely int).
-Offset ToOffset(int offset) {
-  CHECK(0 <= offset && offset < (1 << 8 * sizeof(Offset)));
-  return (Offset)offset;
+StoreOffset to_offset(int offset) {

[Jarin, 2016/07/20 11:43:33]

Function names should be CamelCase. (Here and below.)

[bgeron, 2016/07/20 16:27:16]

Yeah, you're right. I was thinking of the "simple function" exception, but that
doesn't really apply here any more. Fixed, with the exception of rep_size_of.

+  CHECK(0 <= offset && offset < (1 << 8 * sizeof(StoreOffset)));
+  return (StoreOffset)offset;
 }
 
-Offset ToOffset(const FieldAccess& access) { return ToOffset(access.offset); }
+StoreOffset to_offset(const FieldAccess& access) {
+  return to_offset(access.offset);
+}
 
 // If node has a single effect use, return that node. If node has no or
 // multiple effect uses, return nullptr.
-Node* SingleEffectUse(Node* node) {
+Node* single_effect_use(Node* node) {
   Node* last_use = nullptr;
   for (Edge edge : node->use_edges()) {
     if (!NodeProperties::IsEffectEdge(edge)) {
@@ -174,27 +154,17 @@ Node* SingleEffectUse(Node* node) {
   return last_use;
 }
 
-// Return true if node is the last consecutive StoreField node in a linear
-// part of the effect chain.
-bool IsEndOfStoreFieldChain(Node* node) {
-  Node* next_on_chain = SingleEffectUse(node);
-  return (next_on_chain == nullptr ||
-          next_on_chain->op()->opcode() != IrOpcode::kStoreField);
-}
-
-// The argument must be a StoreField node. If there is a node before it in the
-// effect chain, and if this part of the effect chain is linear (no other
-// effect uses of that previous node), then return that previous node.
-// Otherwise, return nullptr.
-//
-// The returned node need not be a StoreField.
-Node* PreviousEffectBeforeStoreField(Node* node) {
-  DCHECK_EQ(node->op()->opcode(), IrOpcode::kStoreField);
-  DCHECK_EQ(node->op()->EffectInputCount(), 1);
-
-  Node* previous = NodeProperties::GetEffectInput(node);
-  if (previous != nullptr && node == SingleEffectUse(previous)) {
-    return previous;
+// If there is a node before {node} in the effect chain, and if this part of
+// the effect chain is linear (no other effect uses of that previous node),
+// then return that previous node. Otherwise, return nullptr.
+Node* previous_effect_in_chain(Node* node) {
+  if (node->op()->EffectInputCount() == 1) {
+    Node* previous = NodeProperties::GetEffectInput(node);
+    if (previous != nullptr && node == single_effect_use(previous)) {
+      return previous;
+    } else {
+      return nullptr;
+    }
   } else {
     return nullptr;
   }
@@ -207,113 +177,457 @@ size_t rep_size_of(FieldAccess access) {
   return rep_size_of(access.machine_type.representation());
 }
 
-bool AtMostTagged(FieldAccess access) {
+bool at_most_tagged(FieldAccess access) {
   return rep_size_of(access) <= rep_size_of(MachineRepresentation::kTagged);
 }
 
-bool AtLeastTagged(FieldAccess access) {
+bool at_least_tagged(FieldAccess access) {
   return rep_size_of(access) >= rep_size_of(MachineRepresentation::kTagged);
 }
 
+int effect_use_count(Node* node) {
+  int uses = 0;
+  for (const Edge edge : node->use_edges()) {
+    if (NodeProperties::IsEffectEdge(edge)) {
+      uses++;
+    }
+  }
+  return uses;
+}
+
 }  // namespace
 
-bool StoreStoreElimination::IsEligibleNode(Node* node) {
-  return (node->op()->opcode() == IrOpcode::kStoreField) &&
-         IsEndOfStoreFieldChain(node);
+void StoreStoreElimination::Run(JSGraph* js_graph, Zone* temp_zone) {
+  // Find superfluous nodes
+  GraphReducer graph_reducer(temp_zone, js_graph->graph(), js_graph->Dead());
+  StoreStoreFinder finder(&graph_reducer, js_graph, temp_zone);
+  graph_reducer.AddReducer(&finder);
+  graph_reducer.ReduceGraph();
+
+  // Remove superfluous nodes
+  TRACE1("Eliminating %d nodes", (int)finder.to_remove_const().size());
+  for (Node* node : finder.to_remove_const()) {
+    TRACE1("  Eliminating node: #%d:%s", node->id(), node->op()->mnemonic());
+    Node* previous_effect = NodeProperties::GetEffectInput(node);
+    NodeProperties::ReplaceUses(node, nullptr, previous_effect, nullptr,
+                                nullptr);
+    node->Kill();
+  }
 }
 
-void StoreStoreElimination::ReduceEligibleNode(Node* node) {
-  DCHECK(IsEligibleNode(node));
+bool StoreStoreFinder::IsEligibleNode(Node* node) {
+  DCHECK_LE(node->op()->EffectOutputCount(), 1);
 
-  // if (FLAG_trace_store_elimination) {
-  //   PrintF("** StoreStoreElimination::ReduceEligibleNode: activated:
-  //   #%d\n",
-  //          node->id());
-  // }
+  TRACE2(5, "%d e-inputs, %d e-outputs, %d e-uses for node %d:%s",
+         node->op()->EffectInputCount(), node->op()->EffectOutputCount(),
+         effect_use_count(node), node->id(), node->op()->mnemonic());
 
-  TRACE("activated: #%d", node->id());
+  bool isEffectful = (node->op()->EffectInputCount() >= 1);
+  bool endsEffectChain =
+      (effect_use_count(node) == 1)
+          ? (single_effect_use(node)->op()->EffectInputCount() >= 2)
+          : true;
+  return isEffectful && endsEffectChain;
+}
 
-  // Initialize empty futureStore.
-  ZoneMap<Offset, Node*> futureStore(temp_zone());
+// Recompute unobservables-set for a node. Will also mark superfluous nodes
+// as to be removed.
+
+UnobservablesSet StoreStoreFinder::RecomputeSet(Node* node,
+                                                UnobservablesSet uses) {
+  // Usually, we decide using the operator properties that an operator
+  // observes everything or observes nothing (see CanObserveAnything,
+  // CanObserveNothing), but there are some opcodes we treat specially.
+  switch (node->op()->opcode()) {
+    case IrOpcode::kStoreField: {
+      Node* stored_to = node->InputAt(0);
+      FieldAccess access = OpParameter<FieldAccess>(node->op());
+      StoreOffset offset = to_offset(access);
+
+      StoreObservation observation = {stored_to->id(), offset};
+      bool presentInSet = uses.Contains(observation);
+
+      if (presentInSet && at_most_tagged(access)) {
+        TRACE2(1, "  #%d is StoreField[+%d,%s](#%d), unobservable", node->id(),
+               offset,
+               MachineReprToString(access.machine_type.representation()),
+               stored_to->id());
+        to_remove().insert(node);
+        return uses;
+      } else if (presentInSet && !at_most_tagged(access)) {
+        TRACE2(1,
+               "  #%d is StoreField[+%d,%s](#%d), repeated in future but too "
+               "big to optimize away",
+               node->id(), offset,
+               MachineReprToString(access.machine_type.representation()),
+               stored_to->id());
+        return uses;
+      } else if (!presentInSet && at_least_tagged(access)) {
+        TRACE2(1,
+               "  #%d is StoreField[+%d,%s](#%d), observable, recording in set",
+               node->id(), offset,
+               MachineReprToString(access.machine_type.representation()),
+               stored_to->id());
+        return uses.Add(observation, temp_zone());
+      } else if (!presentInSet && !at_least_tagged(access)) {
+        TRACE2(1,
+               "  #%d is StoreField[+%d,%s](#%d), observable but too small to "
+               "record",
+               node->id(), offset,
+               MachineReprToString(access.machine_type.representation()),
+               stored_to->id());
+        return uses;
+      } else {
+        UNREACHABLE();
+      }
+      break;
+    }
+    case IrOpcode::kLoadField: {
+      Node* loaded_from = node->InputAt(0);
+      FieldAccess access = OpParameter<FieldAccess>(node->op());
+      StoreOffset offset = to_offset(access);
+
+      TRACE2(1,
+             "  #%d is LoadField[+%d,%s](#%d), removing all offsets [+%d] from "
+             "set",
+             node->id(), offset,
+             MachineReprToString(access.machine_type.representation()),
+             loaded_from->id(), offset);
+
+      return uses.RemoveSameOffset(offset, temp_zone());
+      break;
+    }
+    default:
+      if (CanObserveNothing(node)) {
+        TRACE2(1, "  #%d:%s can observe nothing, set stays unchanged",
+               node->id(), node->op()->mnemonic());
+        return uses;
+      } else if (CanObserveAnything(node)) {
+        TRACE2(1, "  #%d:%s can observe everything, recording empty set",
+               node->id(), node->op()->mnemonic());
+        return UnobservablesSet::DeterminedEmpty(temp_zone());
+      } else {
+        // It is safe to turn this check off in the future, but it is better
+        // to list opcodes in CanObserveNothing, in CanObserveAnything, or if
+        // you don't know, to add another case inside this DCHECK_EXTRA.
+        DCHECK_EXTRA(node->op()->opcode() == IrOpcode::kCall, "%s",
+                     node->op()->mnemonic());
+        TRACE2(1,
+               "  cannot determine unobservables-set for #%d:%s; "
+               "conservatively recording empty set",
+               node->id(), node->op()->mnemonic());
+        return UnobservablesSet::DeterminedEmpty(temp_zone());
+      }
+  }
+  UNREACHABLE();
+  return UnobservablesSet::Undetermined();
+}
+
+bool StoreStoreFinder::CanObserveNothing(Node* node) {
+  Operator::Properties mask =
+      Operator::kNoRead | Operator::kNoDeopt | Operator::kNoThrow;
+
+  return (node->op()->properties() & mask) == mask ||

[Jarin, 2016/07/20 11:43:33]

node->op()->HasProperty(Operator::kNoRead | Operator::kNoDeopt |
Operator::kNoThrow) ...

[bgeron, 2016/07/20 16:27:16]

That doesn't compile, because Operator::kNoRead | ... is of type
Operator::Properties, not Operator::Property.

+         node->opcode() == IrOpcode::kAllocate ||
+         node->opcode() == IrOpcode::kCheckedLoad ||
+         node->opcode() == IrOpcode::kLoadElement;
+}
+
+bool StoreStoreFinder::CanObserveAnything(Node* node) {
+  const Operator* op = node->op();
+  auto opcode = op->opcode();
+  if (opcode == IrOpcode::kLoad) {
+    return true;
+  }
+  return !op->HasProperty(Operator::kNoThrow) ||
+         !op->HasProperty(Operator::kNoDeopt);
+}
+
+// Initialize unobservable_ with js_graph->graph->NodeCount() empty sets.
+StoreStoreFinder::StoreStoreFinder(Editor* editor, JSGraph* js_graph,
+                                   Zone* temp_zone)
+    : AdvancedReducer(editor),
+      jsgraph_(js_graph),
+      temp_zone_(temp_zone),
+      unobservable_(js_graph->graph()->NodeCount(),
+                    UnobservablesSet::Undetermined(), temp_zone),
+      to_remove_(temp_zone) {}
+
+StoreStoreFinder::~StoreStoreFinder() {}
+
+Reduction StoreStoreFinder::Reduce(Node* node) {
+  if (IsEligibleNode(node)) {
+    return ReduceEligibleNode(node);
+  } else {
+    return NoChange();
+  }
+}
+
+Reduction StoreStoreFinder::ReduceEligibleNode(Node* node) {
+  TRACE2(1, "Found eligible node: %4d:%s", node->id(), node->op()->mnemonic());
 
-  Node* current_node = node;
+  Node* cur = node;
+  TRACE2(1, "  Recomputing unobservable-use-intersection for #%d:%s", cur->id(),
+         cur->op()->mnemonic());
+  UnobservablesSet after_set = RecomputeUseIntersection(cur);
+  bool cur_set_changed;
 
   do {
-    FieldAccess access = OpParameter<FieldAccess>(current_node->op());
-    Offset offset = ToOffset(access);
-    Node* object_input = current_node->InputAt(0);
-
-    Node* previous = PreviousEffectBeforeStoreField(current_node);
-
-    // Find the map entry.
-    ZoneMap<Offset, Node*>::iterator find_result = futureStore.find(offset);
-
-    bool present = find_result != futureStore.end();
-    Node* value = present ? find_result->second : nullptr;
-
-    if (present && value == object_input && AtMostTagged(access)) {
-      // Key was present, and the value equalled object_input. This means
-      // that soon after in the effect chain, we will do a StoreField to the
-      // same object with the same offset, therefore current_node can be
-      // optimized away. Also, the future StoreField is at least as big as this
-      // one.
-      //
-      // We don't need to update futureStore.
-
-      Node* previous_effect = NodeProperties::GetEffectInput(current_node);
-
-      NodeProperties::ReplaceUses(current_node, nullptr, previous_effect,
-                                  nullptr, nullptr);
-      current_node->Kill();
-      TRACE("#%d[[+%d,%s]](#%d) -- at most tagged size, eliminated",
-            current_node->id(), offset,
-            MachineReprToString(access.machine_type.representation()),
-            object_input->id());
-    } else if (present && value == object_input && !AtMostTagged(access)) {
-      TRACE("#%d[[+%d,%s]](#%d) -- too wide, not eliminated",
-            current_node->id(), offset,
-            MachineReprToString(access.machine_type.representation()),
-            object_input->id());
-    } else if (present && value != object_input && AtLeastTagged(access)) {
-      // Key was present, and the value did not equal object_input. This means
-      // that there is a StoreField to this offset in the future, but the
-      // object instance comes from a different Node. We pessimistically
-      // assume that we cannot optimize current_node away. However, we will
-      // guess that the current StoreField is more relevant than the future
-      // one, record the current StoreField in futureStore instead, and
-      // continue ascending up the chain.
-      find_result->second = object_input;
-      TRACE("#%d[[+%d,%s]](#%d) -- wide enough, diff object, updated in map",
-            current_node->id(), offset,
-            MachineReprToString(access.machine_type.representation()),
-            object_input->id());
-    } else if (!present && AtLeastTagged(access)) {
-      // Key was not present. This means that there is no matching
-      // StoreField to this offset in the future, so we cannot optimize
-      // current_node away. However, we will record the current StoreField
-      // in futureStore, and continue ascending up the chain.
-      futureStore.insert(std::make_pair(offset, object_input));
-      TRACE(
-          "#%d[[+%d,%s]](#%d) -- wide enough, key not present, inserted in map",
-          current_node->id(), offset,
-          MachineReprToString(access.machine_type.representation()),
-          object_input->id());
-    } else if (!AtLeastTagged(access)) {
-      TRACE("#%d[[+%d,%s]](#%d) -- too narrow to record", current_node->id(),
-            offset, MachineReprToString(access.machine_type.representation()),
-            object_input->id());
+    UnobservablesSet before_set = RecomputeSet(cur, after_set);
+
+    DCHECK_NOT_NULL(before_set.set());
+    TRACE2(2, "  %d StoreObservations in new set",
+           (int)before_set.set()->size());
+
+    UnobservablesSet* stored_for_node = &unobservable().at(cur->id());
+
+    cur_set_changed = (*stored_for_node != before_set);
+
+    if (!stored_for_node->IsUndetermined() && !cur_set_changed) {
+      // We will not be able to update the part of this chain above any more.
+      // Exit.
+      TRACE2(1, "+ No change: stabilized. Stopping this chain.");
+      break;
+    } else if (stored_for_node->IsUndetermined() && !cur_set_changed) {
+      DCHECK(before_set.IsEmpty());
+      TRACE2(2, "  Still empty set, but marking determined. Walking up.");
     } else {
-      UNREACHABLE();
+      TRACE2(2, "  Growing unreachable-set and walking up.");
     }
 
-    // Regardless of whether we eliminated node {current}, we want to
-    // continue walking up the effect chain.
+    // Overwrite vector in-place.
+    *stored_for_node = before_set;
+
+    Node* previous = previous_effect_in_chain(cur);
+    if (previous == nullptr && cur_set_changed) {
+      TRACE2(1,
+             "- Reached top of chain; marking effect inputs for revisiting.");
+      for (int i = 0; i < cur->op()->EffectInputCount(); i++) {
+        Node* input = NodeProperties::GetEffectInput(cur, i);
+        if (!CanObserveAnything(input)) {
+          Revisit(input);
+        }
+      }
+
+      cur = nullptr;
+    } else if (previous == nullptr && !cur_set_changed) {
+      TRACE2(1, "+ Reached top of chain and stabilized.");
+      cur = nullptr;
+    } else {
+      // Update variables for next loop iteration
+      cur = previous;
+      DCHECK(effect_use_count(previous) == 1);
+      after_set = before_set;
+      if (FLAG_turbo_verify_store_elimination) {
+        DCHECK(after_set == RecomputeUseIntersection(cur));
+      }
+      DCHECK_NOT_NULL(cur);
+    }
+  } while (cur != nullptr);
+
+  return NoChange();
+}
+
+// Compute the intersection of the UnobservablesSets of all effect uses and
+// return it. This function only works if {node} has an effect use.
+//
+// The result UnobservablesSet will always be determined.
+UnobservablesSet StoreStoreFinder::RecomputeUseIntersection(Node* node) {
+  TRACE2(4, "    recomputing use intersections of #%d:%s", node->id(),
+         node->op()->mnemonic());
+
+  // {first} == true indicates that we haven't looked at any elements yet.
+  // {first} == false indicates that cur_set is the intersection of at least one
+  // thing.
 
-    current_node = previous;
-  } while (current_node != nullptr &&
-           current_node->op()->opcode() == IrOpcode::kStoreField);
+  bool first = true;
+  UnobservablesSet cur_set = UnobservablesSet::Undetermined();  // irrelevant
+
+  for (Edge edge : node->use_edges()) {
+    // Skip non-effect edges
+    if (!NodeProperties::IsEffectEdge(edge)) {
+      continue;
+    }
+
+    Node* use = edge.from();
+    TRACE2(4, "      found use %d", use->id());
+    UnobservablesSet new_set = unobservable().at(use->id());
+    // Include new_set in the intersection.
+    if (first) {
+      // Intersection of a one-element set is that one element
+      first = false;
+      cur_set = new_set;
+    } else {
+      // Take the intersection of cur_set and new_set.
+      cur_set = cur_set.Intersect(new_set, temp_zone());
+    }
+
+    if (FLAG_trace_store_elimination) {
+      // Serialise the UnobservablesSet.
+      std::ostringstream os;
+      os << "intersected with " << new_set << ", current intersection is "
+         << cur_set;
+      std::string msg = os.str();
+      TRACE2(4, "        %s", msg.c_str());
+    }
+  }
+
+  if (first) {
+    // There were no effect uses.
+    auto opcode = node->op()->opcode();
+    // List of opcodes that may end this effect chain. The opcodes are not
+    // important to the soundness of this optimization; this serves as a
+    // general sanity check. Add opcodes to this list as it suits you.
+    //
+    // Everything is observable after these opcodes; return the empty set.
+    DCHECK_EXTRA(
+        opcode == IrOpcode::kReturn || opcode == IrOpcode::kTerminate ||
+            opcode == IrOpcode::kDeoptimize || opcode == IrOpcode::kThrow,
+        "for #%d:%s", node->id(), node->op()->mnemonic());
+    USE(opcode);  // silence warning about unused variable
+
+    TRACE2(3, "    ..no effect uses, so unobservables-set = []");
+    return UnobservablesSet::DeterminedEmpty(temp_zone());
+  } else {
+    if (cur_set.IsUndetermined()) {
+      cur_set = UnobservablesSet::DeterminedEmpty(temp_zone());
+    }
+
+    if (FLAG_trace_store_elimination) {
+      // Serialise the UnobservablesSet.
+      std::ostringstream os;
+      os << cur_set;
+      std::string msg = os.str();
+      TRACE2(2, "    ..use-intersection: %s", msg.c_str());
+    }
+
+    return cur_set;
+  }
+}
+
+UnobservablesSet UnobservablesSet::Undetermined() { return UnobservablesSet(); }
+
+UnobservablesSet::UnobservablesSet() : set_(nullptr) {}
+
+UnobservablesSet UnobservablesSet::DeterminedEmpty(Zone* zone) {
+  // Create a new empty UnobservablesSet. This allocates in the zone, and
+  // can probably be optimized to use a global singleton.
+  ZoneSet<StoreObservation>* empty_set =
+      new (zone->New(sizeof(ZoneSet<StoreObservation>)))
+          ZoneSet<StoreObservation>(zone);
+  return UnobservablesSet(empty_set);
+}
+
+// Computes the intersection of two UnobservablesSets. May return
+// UnobservablesSet::Undetermined() instead of an empty UnobservablesSet for
+// speed.
+UnobservablesSet UnobservablesSet::Intersect(UnobservablesSet other,
+                                             Zone* zone) const {
+  if (set() == nullptr || other.set() == nullptr) {
+    return Undetermined();
+  } else if (other.set() == nullptr) {
+    return *this;
+  } else {
+    ZoneSet<StoreObservation>* intersection =
+        new (zone->New(sizeof(ZoneSet<StoreObservation>)))
+            ZoneSet<StoreObservation>(zone);
+    // Put the intersection of set() and other.set() in intersection.
+    set_intersection(set()->begin(), set()->end(), other.set()->begin(),
+                     other.set()->end(),
+                     std::inserter(*intersection, intersection->end()));
+    TRACE2(3, "intersected; result:");
+    for (StoreObservation obs : *intersection) {
+      std::ostringstream os;
+      os << obs;
+      std::string msg = os.str();
+      TRACE2(3, "- %s", msg.c_str());
+    }
+    return UnobservablesSet(intersection);
+  }
+}
+
+UnobservablesSet UnobservablesSet::Add(StoreObservation obs, Zone* zone) const {
+  bool present = (set()->find(obs) != set()->end());
+  if (present) {
+    return *this;
+  } else {
+    // Make a new empty set.
+    ZoneSet<StoreObservation>* new_set =
+        new (zone->New(sizeof(ZoneSet<StoreObservation>)))
+            ZoneSet<StoreObservation>(zone);
+    // Copy the old elements over.
+    *new_set = *set();
+    // Add the new element.
+    bool inserted = new_set->insert(obs).second;
+    DCHECK(inserted);
+    USE(inserted);  // silence warning about unused variable
+
+    return UnobservablesSet(new_set);
+  }
+}
+
+UnobservablesSet UnobservablesSet::RemoveSameOffset(StoreOffset offset,
+                                                    Zone* zone) const {
+  // Make a new empty set.
+  ZoneSet<StoreObservation>* new_set =
+      new (zone->New(sizeof(ZoneSet<StoreObservation>)))
+          ZoneSet<StoreObservation>(zone);
+  // Copy all elements over that have a different offset.
+  for (auto obs : *set()) {
+    if (obs.offset_ != offset) {
+      new_set->insert(obs);
+    }
+  }
+
+  return UnobservablesSet(new_set);
+}
+
+// Used for debugging.
+std::ostream& operator<<(std::ostream& os, UnobservablesSet set) {
+  if (set.set() == nullptr) {
+    os << "(undetermined)";
+  } else {
+    os << "[";
+    bool first = true;
+    for (StoreObservation obs : *set.set()) {
+      if (!first) {
+        os << ",";
+      } else {
+        first = false;
+      }
+      os << obs;
+    }
+    os << "]";
+  }
+  return os;
+}
+
+bool UnobservablesSet::operator==(const UnobservablesSet& other) const {
+  if (IsUndetermined() || other.IsUndetermined()) {
+    TRACE2(4, "    (either is undetermined)");
+    return IsEmpty() && other.IsEmpty();
+  } else {
+    TRACE2(4, "    (neither is undetermined)");
+    // Both pointers guaranteed not to be nullptrs.
+    return *set() == *other.set();
+  }
+}
+
+bool UnobservablesSet::operator!=(const UnobservablesSet& other) const {
+  return !(*this == other);
+}
+
+bool operator==(StoreObservation a, StoreObservation b) {
+  return (a.id_ == b.id_) && (a.offset_ == b.offset_);
+}
+
+bool operator<(StoreObservation a, StoreObservation b) {
+  return (a.id_ < b.id_) || (a.id_ == b.id_ && a.offset_ < b.offset_);
+}
 
-  TRACE("finished");
+std::ostream& operator<<(std::ostream& os, StoreObservation obs) {
+  os << "#" << obs.id_ << "[+" << obs.offset_ << "]";
+  return os;
 }
 
 }  // namespace compiler