[DebugInfo][NFC] Fixup the UserValue methods to use FragmentInfo

[Orlando]

Summary

Fixup the UserValue methods to use FragmentInfo instead of DIExpression because
the DIExpression is only ever used to get the to get the FragmentInfo. The
DIExpression is meaningless in the UserValue class because each definition point
added to a UserValue may have a unique DIExpression.

[aprantl]

llvm/lib/CodeGen/LiveDebugVariables.cpp

163

Personally, I'd prefer Fragment or FragmentInfo for readability

[Orlando]

Address comments:

+ UserValue FragInfo -> Fragment

[aprantl]

llvm/lib/CodeGen/LiveDebugVariables.cpp

211–212

matches?

[Orlando]

Thanks for looking at these patches. When they are all ready to land would it be better to land them independently or all together as one?

llvm/lib/CodeGen/LiveDebugVariables.cpp

211–212

It hadn't crossed my mind to rename this. The UserLabel class has a match() method too. Would you like me to update both?

[aprantl]

Thanks for looking at these patches. When they are all ready to land would it be better to land them independently or all together as one?

I would land them as individual commits, but you can roll them into one push. Doin separate pushes makes it easier to debug bot breakage though.

llvm/lib/CodeGen/LiveDebugVariables.cpp

211–212

Sure, why not.

[Orlando]

+ Rebase

[Orlando]

Address review comments:
+ match() -> matches()

llvm/lib/CodeGen/LiveDebugVariables.cpp

 /// 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.
-  // FIXME: This is only used to get the FragmentInfo that describes the part
-  // of the variable we are a part of. We should just store the FragmentInfo.
-  const DIExpression *Expression; ///< Any complex address expression.
+  /// The part of the variable we describe.
+  const Optional<DIExpression::FragmentInfo> Fragment;
   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;
 
class UserValue {
 
   /// Replace OldLocNo ranges with NewRegs ranges where NewRegs
   /// is live. Returns true if any changes were made.
   bool splitLocation(unsigned OldLocNo, ArrayRef<unsigned> NewRegs,
                      LiveIntervals &LIS);
 
 public:
   /// Create a new UserValue.
-  UserValue(const DILocalVariable *var, const DIExpression *expr, DebugLoc L,
+  UserValue(const DILocalVariable *var,
+            Optional<DIExpression::FragmentInfo> Fragment, DebugLoc L,
             LocMap::Allocator &alloc)
-      : Variable(var), Expression(expr), dl(std::move(L)), leader(this),
+      : Variable(var), Fragment(Fragment), 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;
   }
 
   /// 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,
+  bool match(const DILocalVariable *Var,
+             Optional<DIExpression::FragmentInfo> OtherFragment,
              const DILocation *IA) const {
     // FIXME: Handle partially overlapping fragments.
     // A DBG_VALUE with a fragment which overlaps a previous DBG_VALUE fragment
     // for the same variable terminates the interval opened by the first.
     // getUserValue() uses match() to filter DBG_VALUEs into interval maps to
     // represent these intervals.
     // Given two _partially_ overlapping fragments match() will always return
     // false. The DBG_VALUEs will be filtered into separate interval maps and
     // therefore we do not faithfully represent the original intervals.
     // See D70121#1849741 for a more detailed explanation and further
     // discussion.
-    return Var == Variable &&
-           Expr->getFragmentInfo() == Expression->getFragmentInfo() &&
+    return Var == Variable && OtherFragment == Fragment &&
            dl->getInlinedAt() == IA;
   }
 
   /// Merge equivalence classes.
   static UserValue *merge(UserValue *L1, UserValue *L2) {
     L2 = L2->getLeader();
     if (!L1)
       return L2;
class LDVImpl {
   using VRMap = DenseMap<unsigned, UserValue *>;
   VRMap virtRegToEqClass;
 
   /// 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,
+  UserValue *getUserValue(const DILocalVariable *Var,
+                          Optional<DIExpression::FragmentInfo> Fragment,
                           const DebugLoc &DL);
 
   /// Find the EC leader for VirtReg or null.
   UserValue *lookupVirtReg(unsigned VirtReg);
 
   /// Add DBG_VALUE instruction to our maps.
   ///
   /// \param MI DBG_VALUE instruction

 void UserValue::mapVirtRegs(LDVImpl *LDV) {
   for (unsigned i = 0, e = locations.size(); i != e; ++i)
     if (locations[i].isReg() &&
         Register::isVirtualRegister(locations[i].getReg()))
       LDV->mapVirtReg(locations[i].getReg(), this);
 }
 
 UserValue *LDVImpl::getUserValue(const DILocalVariable *Var,
-                                 const DIExpression *Expr, const DebugLoc &DL) {
+                                 Optional<DIExpression::FragmentInfo> Fragment,
+                                 const DebugLoc &DL) {
   UserValue *&Leader = userVarMap[Var];
   if (Leader) {
     UserValue *UV = Leader->getLeader();
     Leader = UV;
     for (; UV; UV = UV->getNext())
-      if (UV->match(Var, Expr, DL->getInlinedAt()))
+      if (UV->match(Var, Fragment, DL->getInlinedAt()))
         return UV;
   }
 
   userValues.push_back(
-      std::make_unique<UserValue>(Var, Expr, DL, allocator));
+      std::make_unique<UserValue>(Var, Fragment, DL, allocator));
   UserValue *UV = userValues.back().get();
   Leader = UserValue::merge(Leader, UV);
   return UV;
 }
 
 void LDVImpl::mapVirtReg(unsigned VirtReg, UserValue *EC) {
   assert(Register::isVirtualRegister(VirtReg) && "Only map VirtRegs");
   UserValue *&Leader = virtRegToEqClass[VirtReg];
bool LDVImpl::handleDebugValue(MachineInstr &MI, SlotIndex Idx) {
   }
 
   // Get or create the UserValue for (variable,offset) here.
   bool IsIndirect = MI.getOperand(1).isImm();
   if (IsIndirect)
     assert(MI.getOperand(1).getImm() == 0 && "DBG_VALUE with nonzero offset");
   const DILocalVariable *Var = MI.getDebugVariable();
   const DIExpression *Expr = MI.getDebugExpression();
-  UserValue *UV =
-      getUserValue(Var, Expr, MI.getDebugLoc());
+  UserValue *UV = getUserValue(Var, Expr->getFragmentInfo(), MI.getDebugLoc());
   if (!Discard)
     UV->addDef(Idx, MI.getOperand(0), IsIndirect, *Expr);
   else {
     MachineOperand MO = MachineOperand::CreateReg(0U, false);
     MO.setIsDebug();
     UV->addDef(Idx, MO, false, *Expr);
   }
   return true;