Simplify LiveDebugVariables by storing MachineOperand copies locations instead

of using a Location class with the same information.

When making a copy of a MachineOperand that was already stored in a
MachineInstr, it is necessary to clear the parent pointer on the copy. Otherwise
the register use-def lists become inconsistent.

Add MachineOperand::clearParent() to do that. An alternative would be a custom
MachineOperand copy constructor that cleared ParentMI. I didn't want to do that
because of the performance impact.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@123109 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/MachineOperand.h

@@ -153,6 +153,16 @@ public:
   MachineInstr *getParent() { return ParentMI; }
   const MachineInstr *getParent() const { return ParentMI; }
 
+  /// clearParent - Reset the parent pointer.
+  ///
+  /// The MachineOperand copy constructor also copies ParentMI, expecting the
+  /// original to be deleted. If a MachineOperand is ever stored outside a
+  /// MachineInstr, the parent pointer must be cleared.
+  ///
+  /// Never call clearParent() on an operand in a MachineInstr.
+  ///
+  void clearParent() { ParentMI = 0; }
+
   void print(raw_ostream &os, const TargetMachine *TM = 0) const;
 
   //===--------------------------------------------------------------------===//

lib/CodeGen/LiveDebugVariables.cpp

@@ -63,105 +63,6 @@ LiveDebugVariables::LiveDebugVariables() : MachineFunctionPass(ID), pImpl(0) {
   initializeLiveDebugVariablesPass(*PassRegistry::getPassRegistry());
 }
 
-/// Location - All the different places a user value can reside.
-/// Note that this includes immediate values that technically aren't locations.
-namespace {
-struct Location {
-  /// kind - What kind of location is this?
-  enum Kind {
-    locUndef = 0,
-    locImm   = 0x80000000,
-    locFPImm
-  };
-  /// Kind - One of the following:
-  /// 1. locUndef
-  /// 2. Register number (physical or virtual), data.SubIdx is the subreg index.
-  /// 3. ~Frame index, data.Offset is the offset.
-  /// 4. locImm, data.ImmVal is the constant integer value.
-  /// 5. locFPImm, data.CFP points to the floating point constant.
-  unsigned Kind;
-
-  /// Data - Extra data about location.
-  union {
-    unsigned SubIdx;          ///< For virtual registers.
-    int64_t Offset;           ///< For frame indices.
-    int64_t ImmVal;           ///< For locImm.
-    const ConstantFP *CFP;    ///< For locFPImm.
-  } Data;
-
-  Location(const MachineOperand &MO) {
-    switch(MO.getType()) {
-    case MachineOperand::MO_Register:
-      Kind = MO.getReg();
-      Data.SubIdx = MO.getSubReg();
-      return;
-    case MachineOperand::MO_Immediate:
-      Kind = locImm;
-      Data.ImmVal = MO.getImm();
-      return;
-    case MachineOperand::MO_FPImmediate:
-      Kind = locFPImm;
-      Data.CFP = MO.getFPImm();
-      return;
-    case MachineOperand::MO_FrameIndex:
-      Kind = ~MO.getIndex();
-      // FIXME: MO_FrameIndex should support an offset.
-      Data.Offset = 0;
-      return;
-    default:
-      Kind = locUndef;
-      return;
-    }
-  }
-
-  /// addOperand - Add this location as a machine operand to MI.
-  MachineInstrBuilder addOperand(MachineInstrBuilder MI) const {
-    switch (Kind) {
-    case locImm:
-      return MI.addImm(Data.ImmVal);
-    case locFPImm:
-      return MI.addFPImm(Data.CFP);
-    default:
-      if (isFrameIndex())
-        return MI.addFrameIndex(getFrameIndex());
-      else
-        return MI.addReg(Kind);  // reg and undef.
-    }
-  }
-
-  bool operator==(const Location &RHS) const {
-    if (Kind != RHS.Kind)
-      return false;
-    switch (Kind) {
-    case locUndef:
-      return true;
-    case locImm:
-      return Data.ImmVal == RHS.Data.ImmVal;
-    case locFPImm:
-      return Data.CFP == RHS.Data.CFP;
-    default:
-      if (isReg())
-        return Data.SubIdx == RHS.Data.SubIdx;
-      else
-         return Data.Offset == RHS.Data.Offset;
-    }
-  }
-
-  /// isUndef - is this the singleton undef?
-  bool isUndef() const { return Kind == locUndef; }
-
-  /// isReg - is this a register location?
-  bool isReg() const { return Kind && Kind < locImm; }
-
-  /// isFrameIndex - is this a frame index location?
-  bool isFrameIndex() const { return Kind > locFPImm; }
-
-  int getFrameIndex() const { return ~Kind; }
-
-  void print(raw_ostream&, const TargetRegisterInfo*);
-};
-}
-
 /// LocMap - Map of where a user value is live, and its location.
 typedef IntervalMap<SlotIndex, unsigned, 4> LocMap;
 
@@ -183,7 +84,7 @@ class UserValue {
   UserValue *next;        ///< Next value in equivalence class, or null.
 
   /// Numbered locations referenced by locmap.
-  SmallVector<Location, 4> locations;
+  SmallVector<MachineOperand, 4> locations;
 
   /// Map of slot indices where this value is live.
   LocMap locInts;
@@ -242,14 +143,16 @@ public:
   }
 
   /// getLocationNo - Return the location number that matches Loc.
-  unsigned getLocationNo(Location Loc) {
+  unsigned getLocationNo(const MachineOperand &LocMO) {
-    if (Loc.isUndef())
+    if (LocMO.isReg() && LocMO.getReg() == 0)
       return ~0u;
-    unsigned n = std::find(locations.begin(), locations.end(), Loc) -
+    for (unsigned i = 0, e = locations.size(); i != e; ++i)
-                 locations.begin();
+      if (LocMO.isIdenticalTo(locations[i]))
-    if (n == locations.size())
+        return i;
-      locations.push_back(Loc);
+    locations.push_back(LocMO);
-    return n;
+    // We are storing a MachineOperand outside a MachineInstr.
+    locations.back().clearParent();
+    return locations.size() - 1;
   }
 
   /// addDef - Add a definition point to this value.
@@ -361,29 +264,6 @@ public:
 };
 } // namespace
 
-void Location::print(raw_ostream &OS, const TargetRegisterInfo *TRI) {
-  switch (Kind) {
-  case locUndef:
-    OS << "undef";
-    return;
-  case locImm:
-    OS << "int:" << Data.ImmVal;
-    return;
-  case locFPImm:
-    OS << "fp:" << Data.CFP->getValueAPF().convertToDouble();
-    return;
-  default:
-    if (isReg()) {
-      OS << PrintReg(Kind, TRI, Data.SubIdx);
-    } else {
-      OS << "fi#" << ~Kind;
-      if (Data.Offset)
-        OS << '+' << Data.Offset;
-    }
-    return;
-  }
-}
-
 void UserValue::print(raw_ostream &OS, const TargetRegisterInfo *TRI) {
   if (const MDString *MDS = dyn_cast<MDString>(variable->getOperand(2)))
     OS << "!\"" << MDS->getString() << "\"\t";
@@ -396,10 +276,8 @@ void UserValue::print(raw_ostream &OS, const TargetRegisterInfo *TRI) {
     else
       OS << I.value();
   }
-  for (unsigned i = 0, e = locations.size(); i != e; ++i) {
+  for (unsigned i = 0, e = locations.size(); i != e; ++i)
-    OS << " Loc" << i << '=';
+    OS << " Loc" << i << '=' << locations[i];
-    locations[i].print(OS, TRI);
-  }
   OS << '\n';
 }
 
@@ -410,17 +288,21 @@ void LDVImpl::print(raw_ostream &OS) {
 }
 
 void UserValue::coalesceLocation(unsigned LocNo) {
-  unsigned KeepLoc = std::find(locations.begin(), locations.begin() + LocNo,
+  unsigned KeepLoc = 0;
-                               locations[LocNo]) - locations.begin();
+  for (unsigned e = locations.size(); KeepLoc != e; ++KeepLoc) {
-  unsigned EraseLoc = LocNo;
+    if (KeepLoc == LocNo)
-  if (KeepLoc == LocNo) {
+      continue;
-    EraseLoc = std::find(locations.begin() + LocNo + 1, locations.end(),
+    if (locations[KeepLoc].isIdenticalTo(locations[LocNo]))
-                         locations[LocNo]) - locations.begin();
+      break;
-    // No matches.
-    if (EraseLoc == locations.size())
-      return;
   }
-  assert(KeepLoc < EraseLoc);
+  // No matches.
+  if (KeepLoc == locations.size())
+    return;
+
+  // Keep the smaller location, erase the larger one.
+  unsigned EraseLoc = LocNo;
+  if (KeepLoc > EraseLoc)
+    std::swap(KeepLoc, EraseLoc);
   locations.erase(locations.begin() + EraseLoc);
 
   // Rewrite values.
@@ -576,11 +458,11 @@ UserValue::computeIntervals(LiveIntervals &LIS, MachineDominatorTree &MDT) {
   for (unsigned i = 0, e = Defs.size(); i != e; ++i) {
     SlotIndex Idx = Defs[i].first;
     unsigned LocNo = Defs[i].second;
-    const Location &Loc = locations[LocNo];
+    const MachineOperand &Loc = locations[LocNo];
 
     // Register locations are constrained to where the register value is live.
-    if (Loc.isReg() && LIS.hasInterval(Loc.Kind)) {
+    if (Loc.isReg() && LIS.hasInterval(Loc.getReg())) {
-      LiveInterval *LI = &LIS.getInterval(Loc.Kind);
+      LiveInterval *LI = &LIS.getInterval(Loc.getReg());
       const VNInfo *VNI = LI->getVNInfoAt(Idx);
       extendDef(Idx, LocNo, LI, VNI, LIS, MDT);
     } else
@@ -639,12 +521,13 @@ renameRegister(unsigned OldReg, unsigned NewReg, unsigned SubIdx,
                const TargetRegisterInfo *TRI) {
   for (unsigned i = locations.size(); i; --i) {
     unsigned LocNo = i - 1;
-    Location &Loc = locations[LocNo];
+    MachineOperand &Loc = locations[LocNo];
-    if (Loc.Kind != OldReg)
+    if (!Loc.isReg() || Loc.getReg() != OldReg)
       continue;
-    Loc.Kind = NewReg;
+    if (TargetRegisterInfo::isPhysicalRegister(NewReg))
-    if (SubIdx && Loc.Data.SubIdx)
+      Loc.substPhysReg(NewReg, *TRI);
-      Loc.Data.SubIdx = TRI->composeSubRegIndices(SubIdx, Loc.Data.SubIdx);
+    else
+      Loc.substVirtReg(NewReg, SubIdx, *TRI);
     coalesceLocation(LocNo);
   }
 }
@@ -676,23 +559,20 @@ UserValue::rewriteLocations(VirtRegMap &VRM, const TargetRegisterInfo &TRI) {
   // Iterate over locations in reverse makes it easier to handle coalescing.
   for (unsigned i = locations.size(); i ; --i) {
     unsigned LocNo = i-1;
-    Location &Loc = locations[LocNo];
+    MachineOperand &Loc = locations[LocNo];
     // Only virtual registers are rewritten.
-    if (!Loc.isReg() || !TargetRegisterInfo::isVirtualRegister(Loc.Kind))
+    if (!Loc.isReg() || !Loc.getReg() ||
+        !TargetRegisterInfo::isVirtualRegister(Loc.getReg()))
       continue;
-    unsigned VirtReg = Loc.Kind;
+    unsigned VirtReg = Loc.getReg();
     if (VRM.isAssignedReg(VirtReg)) {
-      unsigned PhysReg = VRM.getPhys(VirtReg);
+      Loc.substPhysReg(VRM.getPhys(VirtReg), TRI);
-      if (Loc.Data.SubIdx)
-        PhysReg = TRI.getSubReg(PhysReg, Loc.Data.SubIdx);
-      Loc.Kind = PhysReg;
-      Loc.Data.SubIdx = 0;
     } else if (VRM.getStackSlot(VirtReg) != VirtRegMap::NO_STACK_SLOT) {
-      Loc.Kind = ~VRM.getStackSlot(VirtReg);
       // FIXME: Translate SubIdx to a stackslot offset.
-      Loc.Data.Offset = 0;
+      Loc = MachineOperand::CreateFI(VRM.getStackSlot(VirtReg));
     } else {
-      Loc.Kind = Location::locUndef;
+      Loc.setReg(0);
+      Loc.setSubReg(0);
     }
     coalesceLocation(LocNo);
   }
@@ -730,21 +610,20 @@ void UserValue::insertDebugValue(MachineBasicBlock *MBB, SlotIndex Idx,
                                  const TargetInstrInfo &TII) {
   DebugLoc DL;
   MachineBasicBlock::iterator I = findInsertLocation(MBB, Idx, DL, LIS);
-  Location &Loc = locations[LocNo];
+  MachineOperand &Loc = locations[LocNo];
 
   // Frame index locations may require a target callback.
-  if (Loc.isFrameIndex()) {
+  if (Loc.isFI()) {
     MachineInstr *MI = TII.emitFrameIndexDebugValue(*MBB->getParent(),
-                                          Loc.getFrameIndex(),
+                                          Loc.getIndex(), offset, variable, DL);
-                                          offset, variable, DL);
     if (MI) {
       MBB->insert(I, MI);
       return;
     }
   }
   // This is not a frame index, or the target is happy with a standard FI.
-  Loc.addOperand(BuildMI(*MBB, I, DL, TII.get(TargetOpcode::DBG_VALUE)))
+  BuildMI(*MBB, I, DL, TII.get(TargetOpcode::DBG_VALUE))
-    .addImm(offset).addMetadata(variable);
+    .addOperand(Loc).addImm(offset).addMetadata(variable);
 }
 
 void UserValue::insertDebugKill(MachineBasicBlock *MBB, SlotIndex Idx,