Revert "[NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap"

This reverts commit 8af5ada09319e5a021d57a1a03715b2fd022e415.

As Bjorn pointed out in D68816, the iteration over `UserVals` may not be safe.

Reverting on behalf of Orlando.

diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 2dd462f..253bdbb 100644 (file)
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -142,51 +142,22 @@ namespace {
 
 class LDVImpl;
 
-/// A UserValue is uniquely identified by the source variable it refers to
-/// (Variable), the expression describing how to get the value (Expression) and
-/// the specific usage (InlinedAt). InlinedAt differentiates both between
-/// inline and non-inline functions, and multiple inlined instances in the same
-/// scope. FIXME: The only part of the Expression which matters for UserValue
-/// identification is the fragment part.
-class UserValueIdentity {
-private:
-  /// The debug info variable we are part of.
-  const DILocalVariable *Variable;
-  /// Any complex address expression.
-  const DIExpression *Expression;
-  /// Function usage identification.
-  const DILocation *InlinedAt;
-
-public:
-  UserValueIdentity(const DILocalVariable *Var, const DIExpression *Expr,
-                    const DILocation *IA)
-      : Variable(Var), Expression(Expr), InlinedAt(IA) {}
-
-  bool match(const DILocalVariable *Var, const DIExpression *Expr,
-             const DILocation *IA) const {
-    // FIXME: The fragment should be part of the identity, but not
-    // other things in the expression like stack values.
-    return Var == Variable && Expr == Expression && IA == InlinedAt;
-  }
-
-  bool match(const UserValueIdentity &Other) const {
-    return match(Other.Variable, Other.Expression, Other.InlinedAt);
-  }
-
-  unsigned hash_value() const {
-    return hash_combine(Variable, Expression, InlinedAt);
-  }
-};
-
 /// A user value is a part of a debug info user variable.
 ///
 /// A DBG_VALUE instruction notes that (a sub-register of) a virtual register
 /// holds part of a user variable. The part is identified by a byte offset.
+///
+/// UserValues are grouped into equivalence classes for easier searching. Two
+/// user values are related if they refer to the same variable, or if they are
+/// held by the same virtual register. The equivalence class is the transitive
+/// closure of that relation.
 class UserValue {
   const DILocalVariable *Variable; ///< The debug info variable we are part of.
   const DIExpression *Expression; ///< Any complex address expression.
   DebugLoc dl;            ///< The debug location for the variable. This is
                           ///< used by dwarf writer to find lexical scope.
+  UserValue *leader;      ///< Equivalence class leader.
+  UserValue *next = nullptr; ///< Next value in equivalence class, or null.
 
   /// Numbered locations referenced by locmap.
   SmallVector<MachineOperand, 4> locations;
@@ -207,15 +178,49 @@ class UserValue {
                      LiveIntervals &LIS);
 
 public:
-  UserValue(const UserValue &) = delete;
-
   /// Create a new UserValue.
   UserValue(const DILocalVariable *var, const DIExpression *expr, DebugLoc L,
             LocMap::Allocator &alloc)
-      : Variable(var), Expression(expr), dl(std::move(L)), locInts(alloc) {}
+      : Variable(var), Expression(expr), dl(std::move(L)), leader(this),
+        locInts(alloc) {}
+
+  /// Get the leader of this value's equivalence class.
+  UserValue *getLeader() {
+    UserValue *l = leader;
+    while (l != l->leader)
+      l = l->leader;
+    return leader = l;
+  }
 
-  UserValueIdentity getId() {
-    return UserValueIdentity(Variable, Expression, dl->getInlinedAt());
+  /// Return the next UserValue in the equivalence class.
+  UserValue *getNext() const { return next; }
+
+  /// Does this UserValue match the parameters?
+  bool match(const DILocalVariable *Var, const DIExpression *Expr,
+             const DILocation *IA) const {
+    // FIXME: The fragment should be part of the equivalence class, but not
+    // other things in the expression like stack values.
+    return Var == Variable && Expr == Expression && dl->getInlinedAt() == IA;
+  }
+
+  /// Merge equivalence classes.
+  static UserValue *merge(UserValue *L1, UserValue *L2) {
+    L2 = L2->getLeader();
+    if (!L1)
+      return L2;
+    L1 = L1->getLeader();
+    if (L1 == L2)
+      return L1;
+    // Splice L2 before L1's members.
+    UserValue *End = L2;
+    while (End->next) {
+      End->leader = L1;
+      End = End->next;
+    }
+    End->leader = L1;
+    End->next = L1->next;
+    L1->next = L2;
+    return L1;
   }
 
   /// Return the location number that matches Loc.
@@ -327,29 +332,7 @@ public:
 
   void print(raw_ostream &, const TargetRegisterInfo *);
 };
-} // namespace
-
-namespace llvm {
-template <> struct DenseMapInfo<UserValueIdentity> {
-  static UserValueIdentity getEmptyKey() {
-    auto Key = DenseMapInfo<DILocalVariable *>::getEmptyKey();
-    return UserValueIdentity(Key, nullptr, nullptr);
-  }
-  static UserValueIdentity getTombstoneKey() {
-    auto Key = DenseMapInfo<DILocalVariable *>::getTombstoneKey();
-    return UserValueIdentity(Key, nullptr, nullptr);
-  }
-  static unsigned getHashValue(const UserValueIdentity &Val) {
-    return Val.hash_value();
-  }
-  static bool isEqual(const UserValueIdentity &LHS,
-                      const UserValueIdentity &RHS) {
-    return LHS.match(RHS);
-  }
-};
-} // namespace llvm
 
-namespace {
 /// A user label is a part of a debug info user label.
 class UserLabel {
   const DILabel *Label; ///< The debug info label we are part of.
@@ -401,20 +384,20 @@ class LDVImpl {
   /// All allocated UserLabel instances.
   SmallVector<std::unique_ptr<UserLabel>, 2> userLabels;
 
-  /// Map virtual register to UserValues which use it.
-  using VRMap = DenseMap<unsigned, SmallVector<UserValue *, 4>>;
-  VRMap VirtRegToUserVals;
+  /// Map virtual register to eq class leader.
+  using VRMap = DenseMap<unsigned, UserValue *>;
+  VRMap virtRegToEqClass;
 
-  /// Map unique UserValue identity to UserValue.
-  using UVMap = DenseMap<UserValueIdentity, UserValue *>;
-  UVMap UserVarMap;
+  /// Map user variable to eq class leader.
+  using UVMap = DenseMap<const DILocalVariable *, UserValue *>;
+  UVMap userVarMap;
 
   /// Find or create a UserValue.
   UserValue *getUserValue(const DILocalVariable *Var, const DIExpression *Expr,
                           const DebugLoc &DL);
 
-  /// Find the UserValues for VirtReg or null.
-  SmallVectorImpl<UserValue *> *lookupVirtReg(unsigned VirtReg);
+  /// Find the EC leader for VirtReg or null.
+  UserValue *lookupVirtReg(unsigned VirtReg);
 
   /// Add DBG_VALUE instruction to our maps.
   ///
@@ -454,8 +437,8 @@ public:
     MF = nullptr;
     userValues.clear();
     userLabels.clear();
-    VirtRegToUserVals.clear();
-    UserVarMap.clear();
+    virtRegToEqClass.clear();
+    userVarMap.clear();
     // Make sure we call emitDebugValues if the machine function was modified.
     assert((!ModifiedMF || EmitDone) &&
            "Dbg values are not emitted in LDV");
@@ -463,8 +446,8 @@ public:
     ModifiedMF = false;
   }
 
-  /// Map virtual register to a UserValue.
-  void mapVirtReg(unsigned VirtReg, UserValue *UV);
+  /// Map virtual register to an equivalence class.
+  void mapVirtReg(unsigned VirtReg, UserValue *EC);
 
   /// Replace all references to OldReg with NewRegs.
   void splitRegister(unsigned OldReg, ArrayRef<unsigned> NewRegs);
@@ -572,27 +555,31 @@ void UserValue::mapVirtRegs(LDVImpl *LDV) {
 
 UserValue *LDVImpl::getUserValue(const DILocalVariable *Var,
                                  const DIExpression *Expr, const DebugLoc &DL) {
-  auto Ident = UserValueIdentity(Var, Expr, DL->getInlinedAt());
-  UserValue *&UVEntry = UserVarMap[Ident];
-
-  if (UVEntry)
-    return UVEntry;
+  UserValue *&Leader = userVarMap[Var];
+  if (Leader) {
+    UserValue *UV = Leader->getLeader();
+    Leader = UV;
+    for (; UV; UV = UV->getNext())
+      if (UV->match(Var, Expr, DL->getInlinedAt()))
+        return UV;
+  }
 
-  userValues.push_back(std::make_unique<UserValue>(Var, Expr, DL, allocator));
-  return UVEntry = userValues.back().get();
+  userValues.push_back(
+      std::make_unique<UserValue>(Var, Expr, DL, allocator));
+  UserValue *UV = userValues.back().get();
+  Leader = UserValue::merge(Leader, UV);
+  return UV;
 }
 
-void LDVImpl::mapVirtReg(unsigned VirtReg, UserValue *UV) {
+void LDVImpl::mapVirtReg(unsigned VirtReg, UserValue *EC) {
   assert(Register::isVirtualRegister(VirtReg) && "Only map VirtRegs");
-  assert(UserVarMap.find(UV->getId()) != UserVarMap.end() &&
-         "UserValue should exist in UserVarMap");
-  VirtRegToUserVals[VirtReg].push_back(UV);
+  UserValue *&Leader = virtRegToEqClass[VirtReg];
+  Leader = UserValue::merge(Leader, EC);
 }
 
-SmallVectorImpl<UserValue *> *LDVImpl::lookupVirtReg(unsigned VirtReg) {
-  VRMap::iterator Itr = VirtRegToUserVals.find(VirtReg);
-  if (Itr != VirtRegToUserVals.end())
-    return &Itr->getSecond();
+UserValue *LDVImpl::lookupVirtReg(unsigned VirtReg) {
+  if (UserValue *UV = virtRegToEqClass.lookup(VirtReg))
+    return UV->getLeader();
   return nullptr;
 }
 
@@ -1129,18 +1116,16 @@ UserValue::splitRegister(unsigned OldReg, ArrayRef<unsigned> NewRegs,
 
 void LDVImpl::splitRegister(unsigned OldReg, ArrayRef<unsigned> NewRegs) {
   bool DidChange = false;
-  if (auto *UserVals = lookupVirtReg(OldReg))
-    for (auto *UV : *UserVals)
-      DidChange |= UV->splitRegister(OldReg, NewRegs, *LIS);
+  for (UserValue *UV = lookupVirtReg(OldReg); UV; UV = UV->getNext())
+    DidChange |= UV->splitRegister(OldReg, NewRegs, *LIS);
 
   if (!DidChange)
     return;
 
   // Map all of the new virtual registers.
-  if (auto *UserVals = lookupVirtReg(OldReg))
-    for (auto *UV : *UserVals)
-      for (unsigned i = 0; i != NewRegs.size(); ++i)
-        mapVirtReg(NewRegs[i], UV);
+  UserValue *UV = lookupVirtReg(OldReg);
+  for (unsigned i = 0; i != NewRegs.size(); ++i)
+    mapVirtReg(NewRegs[i], UV);
 }
 
 void LiveDebugVariables::