Track and check representations in load elimination

R=jarin@google.com, tebbi@google.com

Change-Id: Ic64ca132178f189a6d78a73ed18150ae503dd9a9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1617936
Auto-Submit: Georg Schmid <gsps@google.com>
Commit-Queue: Georg Schmid <gsps@google.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61844}

src/compiler/load-elimination.cc

@@ -140,7 +140,7 @@ namespace {
 
 bool IsCompatible(MachineRepresentation r1, MachineRepresentation r2) {
   if (r1 == r2) return true;
-  return IsAnyTagged(r1) && IsAnyTagged(r2);
+  return IsAnyCompressedTagged(r1) && IsAnyCompressedTagged(r2);
 }
 
 }  // namespace
@@ -249,10 +249,11 @@ void LoadElimination::AbstractElements::Print() const {
   }
 }
 
-Node* LoadElimination::AbstractField::Lookup(Node* object) const {
-  for (auto pair : info_for_node_) {
+LoadElimination::FieldInfo const* LoadElimination::AbstractField::Lookup(
+    Node* object) const {
+  for (auto& pair : info_for_node_) {
     if (pair.first->IsDead()) continue;
-    if (MustAlias(object, pair.first)) return pair.second.value;
+    if (MustAlias(object, pair.first)) return &pair.second;
   }
   return nullptr;
 }
@@ -304,9 +305,10 @@ LoadElimination::AbstractField const* LoadElimination::AbstractField::Kill(
 
 void LoadElimination::AbstractField::Print() const {
   for (auto pair : info_for_node_) {
-    PrintF("    #%d:%s -> #%d:%s\n", pair.first->id(),
+    PrintF("    #%d:%s -> #%d:%s [repr=%s]\n", pair.first->id(),
            pair.first->op()->mnemonic(), pair.second.value->id(),
-           pair.second.value->op()->mnemonic());
+           pair.second.value->op()->mnemonic(),
+           MachineReprToString(pair.second.representation));
   }
 }
 
@@ -524,16 +526,16 @@ LoadElimination::AbstractState::KillElement(Node* object, Node* index,
 }
 
 LoadElimination::AbstractState const* LoadElimination::AbstractState::AddField(
-    Node* object, size_t index, Node* value, MaybeHandle<Name> name,
+    Node* object, size_t index, LoadElimination::FieldInfo info,
     PropertyConstness constness, Zone* zone) const {
   AbstractState* that = new (zone) AbstractState(*this);
   AbstractFields& fields = constness == PropertyConstness::kConst
                                ? that->const_fields_
                                : that->fields_;
   if (fields[index]) {
-    fields[index] = fields[index]->Extend(object, value, name, zone);
+    fields[index] = fields[index]->Extend(object, info, zone);
   } else {
-    fields[index] = new (zone) AbstractField(object, value, name, zone);
+    fields[index] = new (zone) AbstractField(object, info, zone);
   }
   return that;
 }
@@ -594,7 +596,7 @@ LoadElimination::AbstractState const* LoadElimination::AbstractState::KillAll(
   return LoadElimination::empty_state();
 }
 
-Node* LoadElimination::AbstractState::LookupField(
+LoadElimination::FieldInfo const* LoadElimination::AbstractState::LookupField(
     Node* object, size_t index, PropertyConstness constness) const {
   AbstractFields const& fields =
       constness == PropertyConstness::kConst ? const_fields_ : fields_;
@@ -733,9 +735,9 @@ Reduction LoadElimination::ReduceEnsureWritableFastElements(Node* node) {
   state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset),
                            MaybeHandle<Name>(), zone());
   // Add the new elements on {object}.
-  state =
-      state->AddField(object, FieldIndexOf(JSObject::kElementsOffset), node,
-                      MaybeHandle<Name>(), PropertyConstness::kMutable, zone());
+  state = state->AddField(object, FieldIndexOf(JSObject::kElementsOffset),
+                          {node, MachineType::RepCompressedTaggedPointer()},
+                          PropertyConstness::kMutable, zone());
   return UpdateState(node, state);
 }
 
@@ -760,9 +762,9 @@ Reduction LoadElimination::ReduceMaybeGrowFastElements(Node* node) {
   state = state->KillField(object, FieldIndexOf(JSObject::kElementsOffset),
                            MaybeHandle<Name>(), zone());
   // Add the new elements on {object}.
-  state =
-      state->AddField(object, FieldIndexOf(JSObject::kElementsOffset), node,
-                      MaybeHandle<Name>(), PropertyConstness::kMutable, zone());
+  state = state->AddField(object, FieldIndexOf(JSObject::kElementsOffset),
+                          {node, MachineType::RepCompressedTaggedPointer()},
+                          PropertyConstness::kMutable, zone());
   return UpdateState(node, state);
 }
 
@@ -851,14 +853,20 @@ Reduction LoadElimination::ReduceLoadField(Node* node,
     int field_index = FieldIndexOf(access);
     if (field_index >= 0) {
       PropertyConstness constness = access.constness;
-      Node* replacement = state->LookupField(object, field_index, constness);
-      if (!replacement && constness == PropertyConstness::kConst) {
-        replacement = state->LookupField(object, field_index,
-                                         PropertyConstness::kMutable);
+      MachineRepresentation representation =
+          access.machine_type.representation();
+      FieldInfo const* lookup_result =
+          state->LookupField(object, field_index, constness);
+      if (!lookup_result && constness == PropertyConstness::kConst) {
+        lookup_result = state->LookupField(object, field_index,
+                                           PropertyConstness::kMutable);
       }
-      if (replacement) {
-        // Make sure we don't resurrect dead {replacement} nodes.
-        if (!replacement->IsDead()) {
+      if (lookup_result) {
+        // Make sure we don't reuse values that were recorded with a different
+        // representation or resurrect dead {replacement} nodes.
+        Node* replacement = lookup_result->value;
+        if (IsCompatible(representation, lookup_result->representation) &&
+            !replacement->IsDead()) {
           // Introduce a TypeGuard if the type of the {replacement} node is not
           // a subtype of the original {node}'s type.
           if (!NodeProperties::GetType(replacement)
@@ -875,8 +883,8 @@ Reduction LoadElimination::ReduceLoadField(Node* node,
           return Replace(replacement);
         }
       }
-      state = state->AddField(object, field_index, node, access.name, constness,
-                              zone());
+      FieldInfo info(node, access.name, representation);
+      state = state->AddField(object, field_index, info, constness, zone());
     }
   }
   Handle<Map> field_map;
@@ -910,34 +918,39 @@ Reduction LoadElimination::ReduceStoreField(Node* node,
     int field_index = FieldIndexOf(access);
     if (field_index >= 0) {
       PropertyConstness constness = access.constness;
-      Node* const old_value =
+      MachineRepresentation representation =
+          access.machine_type.representation();
+      FieldInfo const* lookup_result =
           state->LookupField(object, field_index, constness);
 
-      if (constness == PropertyConstness::kConst && old_value) {
-        // At runtime, we should never see two consecutive const stores, i.e.,
-        //    DCHECK_NULL(old_value)
-        // ought to hold, but we might see such (unreachable) code statically.
-        Node* control = NodeProperties::GetControlInput(node);
-        Node* unreachable =
-            graph()->NewNode(common()->Unreachable(), effect, control);
-        return Replace(unreachable);
-      }
-
-      if (old_value == new_value) {
-        // This store is fully redundant.
-        return Replace(effect);
+      if (lookup_result) {
+        CHECK(lookup_result->name.is_null() ||
+              IsCompatible(representation, lookup_result->representation));
+        if (constness == PropertyConstness::kConst) {
+          // At runtime, we should never see two consecutive const stores, but
+          // we might see such (unreachable) code statically.
+          Node* control = NodeProperties::GetControlInput(node);
+          Node* unreachable =
+              graph()->NewNode(common()->Unreachable(), effect, control);
+          return Replace(unreachable);
+        }
+        if (lookup_result->value == new_value) {
+          // This store is fully redundant.
+          return Replace(effect);
+        }
       }
 
       // Kill all potentially aliasing fields and record the new value.
+      FieldInfo new_info(new_value, access.name, representation);
       state = state->KillField(object, field_index, access.name, zone());
-      state = state->AddField(object, field_index, new_value, access.name,
+      state = state->AddField(object, field_index, new_info,
                               PropertyConstness::kMutable, zone());
       if (constness == PropertyConstness::kConst) {
         // For const stores, we track information in both the const and the
         // mutable world to guard against field accesses that should have
         // been marked const, but were not.
-        state = state->AddField(object, field_index, new_value, access.name,
-                                constness, zone());
+        state =
+            state->AddField(object, field_index, new_info, constness, zone());
       }
     } else {
       // Unsupported StoreField operator.

src/compiler/load-elimination.h

@@ -98,24 +98,42 @@ class V8_EXPORT_PRIVATE LoadElimination final
   // not alias.
   class AliasStateInfo;
 
+  struct FieldInfo {
+    FieldInfo() = default;
+    FieldInfo(Node* value, MachineRepresentation representation)
+        : value(value), name(), representation(representation) {}
+    FieldInfo(Node* value, MaybeHandle<Name> name,
+              MachineRepresentation representation)
+        : value(value), name(name), representation(representation) {}
+
+    bool operator==(const FieldInfo& other) const {
+      return value == other.value && name.address() == other.name.address() &&
+             representation == other.representation;
+    }
+
+    Node* value = nullptr;
+    MaybeHandle<Name> name;
+    MachineRepresentation representation = MachineRepresentation::kNone;
+  };
+
   // Abstract state to approximate the current state of a certain field along
   // the effect paths through the graph.
   class AbstractField final : public ZoneObject {
    public:
     explicit AbstractField(Zone* zone) : info_for_node_(zone) {}
-    AbstractField(Node* object, Node* value, MaybeHandle<Name> name, Zone* zone)
+    AbstractField(Node* object, FieldInfo info, Zone* zone)
         : info_for_node_(zone) {
-      info_for_node_.insert(std::make_pair(object, Field(value, name)));
+      info_for_node_.insert(std::make_pair(object, info));
     }
 
-    AbstractField const* Extend(Node* object, Node* value,
-                                MaybeHandle<Name> name, Zone* zone) const {
+    AbstractField const* Extend(Node* object, FieldInfo info,
+                                Zone* zone) const {
       AbstractField* that = new (zone) AbstractField(zone);
       that->info_for_node_ = this->info_for_node_;
-      that->info_for_node_.insert(std::make_pair(object, Field(value, name)));
+      that->info_for_node_[object] = info;
       return that;
     }
-    Node* Lookup(Node* object) const;
+    FieldInfo const* Lookup(Node* object) const;
     AbstractField const* Kill(const AliasStateInfo& alias_info,
                               MaybeHandle<Name> name, Zone* zone) const;
     bool Equals(AbstractField const* that) const {
@@ -126,7 +144,7 @@ class V8_EXPORT_PRIVATE LoadElimination final
       AbstractField* copy = new (zone) AbstractField(zone);
       for (auto this_it : this->info_for_node_) {
         Node* this_object = this_it.first;
-        Field this_second = this_it.second;
+        FieldInfo this_second = this_it.second;
         if (this_object->IsDead()) continue;
         auto that_it = that->info_for_node_.find(this_object);
         if (that_it != that->info_for_node_.end() &&
@@ -140,19 +158,7 @@ class V8_EXPORT_PRIVATE LoadElimination final
     void Print() const;
 
    private:
-    struct Field {
-      Field() = default;
-      Field(Node* value, MaybeHandle<Name> name) : value(value), name(name) {}
-
-      bool operator==(const Field& other) const {
-        return value == other.value && name.address() == other.name.address();
-      }
-
-      Node* value = nullptr;
-      MaybeHandle<Name> name;
-    };
-
-    ZoneMap<Node*, Field> info_for_node_;
+    ZoneMap<Node*, FieldInfo> info_for_node_;
   };
 
   static size_t const kMaxTrackedFields = 32;
@@ -194,8 +200,7 @@ class V8_EXPORT_PRIVATE LoadElimination final
                                   Zone* zone) const;
     bool LookupMaps(Node* object, ZoneHandleSet<Map>* object_maps) const;
 
-    AbstractState const* AddField(Node* object, size_t index, Node* value,
-                                  MaybeHandle<Name> name,
+    AbstractState const* AddField(Node* object, size_t index, FieldInfo info,
                                   PropertyConstness constness,
                                   Zone* zone) const;
     AbstractState const* KillField(const AliasStateInfo& alias_info,
@@ -206,8 +211,8 @@ class V8_EXPORT_PRIVATE LoadElimination final
     AbstractState const* KillFields(Node* object, MaybeHandle<Name> name,
                                     Zone* zone) const;
     AbstractState const* KillAll(Zone* zone) const;
-    Node* LookupField(Node* object, size_t index,
-                      PropertyConstness constness) const;
+    FieldInfo const* LookupField(Node* object, size_t index,
+                                 PropertyConstness constness) const;
 
     AbstractState const* AddElement(Node* object, Node* index, Node* value,
                                     MachineRepresentation representation,