[llvm] 67720e7 - Revert "[NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap"
Andrea Di Biagio via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 29 05:15:16 PDT 2019
Author: Andrea Di Biagio
Date: 2019-10-29T12:13:23Z
New Revision: 67720e7bf7dfb28750706e3b27d16a1933af9ca0
URL: https://github.com/llvm/llvm-project/commit/67720e7bf7dfb28750706e3b27d16a1933af9ca0
DIFF: https://github.com/llvm/llvm-project/commit/67720e7bf7dfb28750706e3b27d16a1933af9ca0.diff
LOG: 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.
Added:
Modified:
llvm/lib/CodeGen/LiveDebugVariables.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 2dd462fc72b3..253bdbb11d57 100644
--- 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
diff erentiates 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 @@ class UserValue {
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 @@ class LDVImpl {
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 @@ class LDVImpl {
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::
More information about the llvm-commits
mailing list