Reapply "[DebugInfo] Add DWARF emission for DBG_VALUE_LIST"

This reverts commit 429c6ec.

Author: SLTozer

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -918,9 +918,6 @@ static bool emitDebugValueComment(const MachineInstr *MI, AsmPrinter &AP) {
   OS << V->getName();
   OS << " <- ";
 
-  // The second operand is only an offset if it's an immediate.
-  bool MemLoc = MI->isIndirectDebugValue();
-  auto Offset = StackOffset::getFixed(MemLoc ? MI->getOperand(1).getImm() : 0);
   const DIExpression *Expr = MI->getDebugExpression();
   if (Expr->getNumElements()) {
     OS << '[';
@@ -934,56 +931,71 @@ static bool emitDebugValueComment(const MachineInstr *MI, AsmPrinter &AP) {
   }
 
   // Register or immediate value. Register 0 means undef.
-  if (MI->getDebugOperand(0).isFPImm()) {
-    APFloat APF = APFloat(MI->getDebugOperand(0).getFPImm()->getValueAPF());
-    if (MI->getDebugOperand(0).getFPImm()->getType()->isFloatTy()) {
-      OS << (double)APF.convertToFloat();
-    } else if (MI->getDebugOperand(0).getFPImm()->getType()->isDoubleTy()) {
-      OS << APF.convertToDouble();
-    } else {
-      // There is no good way to print long double.  Convert a copy to
-      // double.  Ah well, it's only a comment.
-      bool ignored;
-      APF.convert(APFloat::IEEEdouble(), APFloat::rmNearestTiesToEven,
-                  &ignored);
-      OS << "(long double) " << APF.convertToDouble();
+  for (const MachineOperand &Op : MI->debug_operands()) {
+    if (&Op != MI->debug_operands().begin())
+      OS << ", ";
+    switch (Op.getType()) {
+    case MachineOperand::MO_FPImmediate: {
+      APFloat APF = APFloat(Op.getFPImm()->getValueAPF());
+      if (Op.getFPImm()->getType()->isFloatTy()) {
+        OS << (double)APF.convertToFloat();
+      } else if (Op.getFPImm()->getType()->isDoubleTy()) {
+        OS << APF.convertToDouble();
+      } else {
+        // There is no good way to print long double.  Convert a copy to
+        // double.  Ah well, it's only a comment.
+        bool ignored;
+        APF.convert(APFloat::IEEEdouble(), APFloat::rmNearestTiesToEven,
+                    &ignored);
+        OS << "(long double) " << APF.convertToDouble();
+      }
+      break;
     }
-  } else if (MI->getDebugOperand(0).isImm()) {
-    OS << MI->getDebugOperand(0).getImm();
-  } else if (MI->getDebugOperand(0).isCImm()) {
-    MI->getDebugOperand(0).getCImm()->getValue().print(OS, false /*isSigned*/);
-  } else if (MI->getDebugOperand(0).isTargetIndex()) {
-    auto Op = MI->getDebugOperand(0);
-    OS << "!target-index(" << Op.getIndex() << "," << Op.getOffset() << ")";
-    // NOTE: Want this comment at start of line, don't emit with AddComment.
-    AP.OutStreamer->emitRawComment(OS.str());
-    return true;
-  } else {
-    Register Reg;
-    if (MI->getDebugOperand(0).isReg()) {
-      Reg = MI->getDebugOperand(0).getReg();
-    } else {
-      assert(MI->getDebugOperand(0).isFI() && "Unknown operand type");
-      const TargetFrameLowering *TFI = AP.MF->getSubtarget().getFrameLowering();
-      Offset += TFI->getFrameIndexReference(
-          *AP.MF, MI->getDebugOperand(0).getIndex(), Reg);
-      MemLoc = true;
+    case MachineOperand::MO_Immediate: {
+      OS << Op.getImm();
+      break;
+    }
+    case MachineOperand::MO_CImmediate: {
+      Op.getCImm()->getValue().print(OS, false /*isSigned*/);
+      break;
     }
-    if (Reg == 0) {
-      // Suppress offset, it is not meaningful here.
-      OS << "undef";
+    case MachineOperand::MO_TargetIndex: {
+      OS << "!target-index(" << Op.getIndex() << "," << Op.getOffset() << ")";
       // NOTE: Want this comment at start of line, don't emit with AddComment.
       AP.OutStreamer->emitRawComment(OS.str());
-      return true;
+      break;
+    }
+    case MachineOperand::MO_Register:
+    case MachineOperand::MO_FrameIndex: {
+      Register Reg;
+      Optional<StackOffset> Offset;
+      if (Op.isReg()) {
+        Reg = Op.getReg();
+      } else {
+        const TargetFrameLowering *TFI =
+            AP.MF->getSubtarget().getFrameLowering();
+        Offset = TFI->getFrameIndexReference(*AP.MF, Op.getIndex(), Reg);
+      }
+      if (!Reg) {
+        // Suppress offset, it is not meaningful here.
+        OS << "undef";
+        break;
+      }
+      // The second operand is only an offset if it's an immediate.
+      if (MI->isIndirectDebugValue())
+        Offset = StackOffset::getFixed(MI->getDebugOffset().getImm());
+      if (Offset)
+        OS << '[';
+      OS << printReg(Reg, AP.MF->getSubtarget().getRegisterInfo());
+      if (Offset)
+        OS << '+' << Offset->getFixed() << ']';
+      break;
+    }
+    default:
+      llvm_unreachable("Unknown operand type");
     }
-    if (MemLoc)
-      OS << '[';
-    OS << printReg(Reg, AP.MF->getSubtarget().getRegisterInfo());
   }
 
-  if (MemLoc)
-    OS << '+' << Offset.getFixed() << ']';
-
   // NOTE: Want this comment at start of line, don't emit with AddComment.
   AP.OutStreamer->emitRawComment(OS.str());
   return true;

llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

@@ -37,22 +37,6 @@ namespace {
 using EntryIndex = DbgValueHistoryMap::EntryIndex;
 }
 
-// If @MI is a DBG_VALUE with debug value described by a
-// defined register, returns the number of this register.
-// In the other case, returns 0.
-static Register isDescribedByReg(const MachineInstr &MI) {
-  assert(MI.isDebugValue());
-  assert(MI.getNumOperands() == 4);
-  // If the location of variable is an entry value (DW_OP_LLVM_entry_value)
-  // do not consider it as a register location.
-  if (MI.getDebugExpression()->isEntryValue())
-    return 0;
-  // If location of variable is described using a register (directly or
-  // indirectly), this register is always a first operand.
-  return MI.getDebugOperand(0).isReg() ? MI.getDebugOperand(0).getReg()
-                                       : Register();
-}
-
 void InstructionOrdering::initialize(const MachineFunction &MF) {
   // We give meta instructions the same ordinal as the preceding instruction
   // because this class is written for the task of comparing positions of
@@ -333,24 +317,44 @@ static void addRegDescribedVar(RegDescribedVarsMap &RegVars, unsigned RegNo,
 }
 
 /// Create a clobbering entry and end all open debug value entries
-/// for \p Var that are described by \p RegNo using that entry.
+/// for \p Var that are described by \p RegNo using that entry. Inserts into \p
+/// FellowRegisters the set of Registers that were also used to describe \p Var
+/// alongside \p RegNo.
 static void clobberRegEntries(InlinedEntity Var, unsigned RegNo,
                               const MachineInstr &ClobberingInstr,
                               DbgValueEntriesMap &LiveEntries,
-                              DbgValueHistoryMap &HistMap) {
+                              DbgValueHistoryMap &HistMap,
+                              SmallVectorImpl<Register> &FellowRegisters) {
   EntryIndex ClobberIndex = HistMap.startClobber(Var, ClobberingInstr);
-
   // Close all entries whose values are described by the register.
   SmallVector<EntryIndex, 4> IndicesToErase;
+  // If a given register appears in a live DBG_VALUE_LIST for Var alongside the
+  // clobbered register, and never appears in a live DBG_VALUE* for Var without
+  // the clobbered register, then it is no longer linked to the variable.
+  SmallSet<Register, 4> MaybeRemovedRegisters;
+  SmallSet<Register, 4> KeepRegisters;
   for (auto Index : LiveEntries[Var]) {
     auto &Entry = HistMap.getEntry(Var, Index);
     assert(Entry.isDbgValue() && "Not a DBG_VALUE in LiveEntries");
-    if (isDescribedByReg(*Entry.getInstr()) == RegNo) {
+    if (Entry.getInstr()->isDebugEntryValue())
+      continue;
+    if (Entry.getInstr()->hasDebugOperandForReg(RegNo)) {
       IndicesToErase.push_back(Index);
       Entry.endEntry(ClobberIndex);
+      for (auto &MO : Entry.getInstr()->debug_operands())
+        if (MO.isReg() && MO.getReg() && MO.getReg() != RegNo)
+          MaybeRemovedRegisters.insert(MO.getReg());
+    } else {
+      for (auto &MO : Entry.getInstr()->debug_operands())
+        if (MO.isReg() && MO.getReg())
+          KeepRegisters.insert(MO.getReg());
     }
   }
 
+  for (Register Reg : MaybeRemovedRegisters)
+    if (!KeepRegisters.contains(Reg))
+      FellowRegisters.push_back(Reg);
+
   // Drop all entries that have ended.
   for (auto Index : IndicesToErase)
     LiveEntries[Var].erase(Index);
@@ -378,17 +382,24 @@ static void handleNewDebugValue(InlinedEntity Var, const MachineInstr &DV,
         IndicesToErase.push_back(Index);
         Entry.endEntry(NewIndex);
       }
-      if (Register Reg = isDescribedByReg(DV))
-        TrackedRegs[Reg] |= !Overlaps;
+      if (!DV.isDebugEntryValue())
+        for (const MachineOperand &Op : DV.debug_operands())
+          if (Op.isReg() && Op.getReg())
+            TrackedRegs[Op.getReg()] |= !Overlaps;
     }
 
     // If the new debug value is described by a register, add tracking of
     // that register if it is not already tracked.
-    if (Register NewReg = isDescribedByReg(DV)) {
-      if (!TrackedRegs.count(NewReg))
-        addRegDescribedVar(RegVars, NewReg, Var);
-      LiveEntries[Var].insert(NewIndex);
-      TrackedRegs[NewReg] = true;
+    if (!DV.isDebugEntryValue()) {
+      for (const MachineOperand &Op : DV.debug_operands()) {
+        if (Op.isReg() && Op.getReg()) {
+          Register NewReg = Op.getReg();
+          if (!TrackedRegs.count(NewReg))
+            addRegDescribedVar(RegVars, NewReg, Var);
+          LiveEntries[Var].insert(NewIndex);
+          TrackedRegs[NewReg] = true;
+        }
+      }
     }
 
     // Drop tracking of registers that are no longer used.
@@ -411,9 +422,16 @@ static void clobberRegisterUses(RegDescribedVarsMap &RegVars,
                                 DbgValueEntriesMap &LiveEntries,
                                 const MachineInstr &ClobberingInstr) {
   // Iterate over all variables described by this register and add this
-  // instruction to their history, clobbering it.
-  for (const auto &Var : I->second)
-    clobberRegEntries(Var, I->first, ClobberingInstr, LiveEntries, HistMap);
+  // instruction to their history, clobbering it. All registers that also
+  // describe the clobbered variables (i.e. in variadic debug values) will have
+  // those Variables removed from their DescribedVars.
+  for (const auto &Var : I->second) {
+    SmallVector<Register, 4> FellowRegisters;
+    clobberRegEntries(Var, I->first, ClobberingInstr, LiveEntries, HistMap,
+                      FellowRegisters);
+    for (Register RegNo : FellowRegisters)
+      dropRegDescribedVar(RegVars, RegNo, Var);
+  }
   RegVars.erase(I);
 }
 

llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

@@ -35,7 +35,8 @@ Optional<DbgVariableLocation>
 DbgVariableLocation::extractFromMachineInstruction(
     const MachineInstr &Instruction) {
   DbgVariableLocation Location;
-  if (!Instruction.isDebugValue())
+  // Variables calculated from multiple locations can't be represented here.
+  if (Instruction.getNumDebugOperands() != 1)
     return None;
   if (!Instruction.getDebugOperand(0).isReg())
     return None;
@@ -46,6 +47,15 @@ DbgVariableLocation::extractFromMachineInstruction(
   int64_t Offset = 0;
   const DIExpression *DIExpr = Instruction.getDebugExpression();
   auto Op = DIExpr->expr_op_begin();
+  // We can handle a DBG_VALUE_LIST iff it has exactly one location operand that
+  // appears exactly once at the start of the expression.
+  if (Instruction.isDebugValueList()) {
+    if (Instruction.getNumDebugOperands() == 1 &&
+        Op->getOp() == dwarf::DW_OP_LLVM_arg)
+      ++Op;
+    else
+      return None;
+  }
   while (Op != DIExpr->expr_op_end()) {
     switch (Op->getOp()) {
     case dwarf::DW_OP_constu: {
@@ -261,7 +271,8 @@ void DebugHandlerBase::beginFunction(const MachineFunction *MF) {
       continue;
 
     auto IsDescribedByReg = [](const MachineInstr *MI) {
-      return MI->getDebugOperand(0).isReg() && MI->getDebugOperand(0).getReg();
+      return any_of(MI->debug_operands(),
+                    [](auto &MO) { return MO.isReg() && MO.getReg(); });
     };
 
     // The first mention of a function argument gets the CurrentFnBegin label,