Merge pull request #7486 from felipepiovezan/felipe/cherry-pick-stable

Cherry pick debug info fixes for single-location debug values

Author: felipepiovezan

llvm/include/llvm/IR/DebugInfoMetadata.h

@@ -2905,7 +2905,7 @@ class DIExpression : public MDNode {
   /// Similar to `convertToNonVariadicExpression`, but faster and cheaper - it
   /// does not check whether the expression is a single-location expression, and
   /// it returns elements rather than creating a new DIExpression.
-  ArrayRef<uint64_t> getSingleLocationExpressionElements() const;
+  std::optional<ArrayRef<uint64_t>> getSingleLocationExpressionElements() const;
 
   /// Removes all elements from \p Expr that do not apply to an undef debug
   /// value, which includes every operator that computes the value/location on

llvm/lib/CodeGen/AsmPrinter/DebugLocStream.cpp

@@ -40,8 +40,5 @@ void DebugLocStream::finalizeEntry() {
 DebugLocStream::ListBuilder::~ListBuilder() {
   if (!Locs.finalizeList(Asm))
     return;
-  V.initializeDbgValue(&MI);
-  V.setDebugLocListIndex(ListIndex);
-  if (TagOffset)
-    V.setDebugLocListTagOffset(*TagOffset);
+  V.emplace<Loc::Multi>(ListIndex, TagOffset);
 }

llvm/lib/CodeGen/AsmPrinter/DebugLocStream.h

@@ -156,14 +156,13 @@ class DebugLocStream::ListBuilder {
   DebugLocStream &Locs;
   AsmPrinter &Asm;
   DbgVariable &V;
-  const MachineInstr &MI;
   size_t ListIndex;
   std::optional<uint8_t> TagOffset;
 
 public:
   ListBuilder(DebugLocStream &Locs, DwarfCompileUnit &CU, AsmPrinter &Asm,
-              DbgVariable &V, const MachineInstr &MI)
-      : Locs(Locs), Asm(Asm), V(V), MI(MI), ListIndex(Locs.startList(&CU)),
+              DbgVariable &V)
+      : Locs(Locs), Asm(Asm), V(V), ListIndex(Locs.startList(&CU)),
         TagOffset(std::nullopt) {}
 
   void setTagOffset(uint8_t TO) {

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

@@ -342,7 +342,7 @@ class DwarfCompileUnit final : public DwarfUnit {
   /// DWARF information necessary to find the actual variable (navigating the
   /// extra location information encoded in the type) based on the starting
   /// location.  Add the DWARF information to the die.
-  void addComplexAddress(const DbgVariable &DV, DIE &Die,
+  void addComplexAddress(const DIExpression *DIExpr, DIE &Die,
                          dwarf::Attribute Attribute,
                          const MachineLocation &Location);
 

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

@@ -240,14 +240,14 @@ const DIType *DbgVariable::getType() const {
 /// Get .debug_loc entry for the instruction range starting at MI.
 static DbgValueLoc getDebugLocValue(const MachineInstr *MI) {
   const DIExpression *Expr = MI->getDebugExpression();
-  const bool IsVariadic = !Expr->isSingleLocationExpression();
+  auto SingleLocExprOpt = DIExpression::convertToNonVariadicExpression(Expr);
+  const bool IsVariadic = !SingleLocExprOpt;
   // If we have a variadic debug value instruction that is equivalent to a
   // non-variadic instruction, then convert it to non-variadic form here.
   if (!IsVariadic && !MI->isNonListDebugValue()) {
     assert(MI->getNumDebugOperands() == 1 &&
            "Mismatched DIExpression and debug operands for debug instruction.");
-    Expr = DIExpression::get(Expr->getContext(),
-                             Expr->getSingleLocationExpressionElements());
+    Expr = *SingleLocExprOpt;
   }
   assert(MI->getNumOperands() >= 3);
   SmallVector<DbgValueLocEntry, 4> DbgValueLocEntries;
@@ -271,31 +271,23 @@ static DbgValueLoc getDebugLocValue(const MachineInstr *MI) {
   return DbgValueLoc(Expr, DbgValueLocEntries, IsVariadic);
 }
 
-void DbgVariable::initializeDbgValue(const MachineInstr *DbgValue) {
-  assert(FrameIndexExprs.empty() && "Already initialized?");
-  assert(!ValueLoc.get() && "Already initialized?");
-
-  assert(getVariable() == DbgValue->getDebugVariable() && "Wrong variable");
-  assert(getInlinedAt() == DbgValue->getDebugLoc()->getInlinedAt() &&
-         "Wrong inlined-at");
-
-  ValueLoc = std::make_unique<DbgValueLoc>(getDebugLocValue(DbgValue));
-  // Use the debug value's expression as a FrameIndexExpr iff it is suitable,
-  // which requires it to be non-variadic.
-  if (auto E = DIExpression::convertToNonVariadicExpression(
-          DbgValue->getDebugExpression()))
-    if ((*E)->getNumElements())
-      FrameIndexExprs.push_back({0, *E});
+Loc::Single::Single(DbgValueLoc ValueLoc)
+    : ValueLoc(std::make_unique<DbgValueLoc>(ValueLoc)),
+      Expr(ValueLoc.getExpression()) {
+  if (!Expr->getNumElements())
+    Expr = nullptr;
 }
 
-ArrayRef<DbgVariable::FrameIndexExpr> DbgVariable::getFrameIndexExprs() const {
+Loc::Single::Single(const MachineInstr *DbgValue)
+    : Single(getDebugLocValue(DbgValue)) {}
+
+ArrayRef<FrameIndexExpr> Loc::MMI::getFrameIndexExprs() const {
   if (FrameIndexExprs.size() == 1)
     return FrameIndexExprs;
 
-  assert(llvm::all_of(FrameIndexExprs,
-                      [](const FrameIndexExpr &A) {
-                        return A.Expr->isFragment();
-                      }) &&
+  assert(llvm::all_of(
+             FrameIndexExprs,
+             [](const FrameIndexExpr &A) { return A.Expr->isFragment(); }) &&
          "multiple FI expressions without DW_OP_LLVM_fragment");
   llvm::sort(FrameIndexExprs,
              [](const FrameIndexExpr &A, const FrameIndexExpr &B) -> bool {
@@ -306,15 +298,7 @@ ArrayRef<DbgVariable::FrameIndexExpr> DbgVariable::getFrameIndexExprs() const {
   return FrameIndexExprs;
 }
 
-void DbgVariable::addMMIEntry(const DbgVariable &V) {
-  assert(DebugLocListIndex == ~0U && !ValueLoc.get() && "not an MMI entry");
-  assert(V.DebugLocListIndex == ~0U && !V.ValueLoc.get() && "not an MMI entry");
-  assert(V.getVariable() == getVariable() && "conflicting variable");
-  assert(V.getInlinedAt() == getInlinedAt() && "conflicting inlined-at location");
-
-  assert(!FrameIndexExprs.empty() && "Expected an MMI entry");
-  assert(!V.FrameIndexExprs.empty() && "Expected an MMI entry");
-
+void Loc::MMI::addFrameIndexExpr(const DIExpression *Expr, int FI) {
   // FIXME: This logic should not be necessary anymore, as we now have proper
   // deduplication. However, without it, we currently run into the assertion
   // below, which means that we are likely dealing with broken input, i.e. two
@@ -325,12 +309,10 @@ void DbgVariable::addMMIEntry(const DbgVariable &V) {
       return;
   }
 
-  for (const auto &FIE : V.FrameIndexExprs)
-    // Ignore duplicate entries.
-    if (llvm::none_of(FrameIndexExprs, [&](const FrameIndexExpr &Other) {
-          return FIE.FI == Other.FI && FIE.Expr == Other.Expr;
-        }))
-      FrameIndexExprs.push_back(FIE);
+  if (llvm::none_of(FrameIndexExprs, [&](const FrameIndexExpr &Other) {
+        return FI == Other.FI && Expr == Other.Expr;
+      }))
+    FrameIndexExprs.push_back({FI, Expr});
 
   assert((FrameIndexExprs.size() == 1 ||
           llvm::all_of(FrameIndexExprs,
@@ -1579,27 +1561,42 @@ void DwarfDebug::collectVariableInfoFromMFTable(
     }
 
     ensureAbstractEntityIsCreatedIfScoped(TheCU, Var.first, Scope->getScopeNode());
+
+    // If we have already seen information for this variable, add to what we
+    // already know.
+    if (DbgVariable *PreviousLoc = MFVars.lookup(Var)) {
+      auto *PreviousMMI = std::get_if<Loc::MMI>(PreviousLoc);
+      auto *PreviousEntryValue = std::get_if<Loc::EntryValue>(PreviousLoc);
+      // Previous and new locations are both stack slots (MMI).
+      if (PreviousMMI && VI.inStackSlot())
+        PreviousMMI->addFrameIndexExpr(VI.Expr, VI.getStackSlot());
+      // Previous and new locations are both entry values.
+      else if (PreviousEntryValue && VI.inEntryValueRegister())
+        PreviousEntryValue->addExpr(VI.getEntryValueRegister(), *VI.Expr);
+      else {
+        // Locations differ, this should (rarely) happen in optimized async
+        // coroutines.
+        // Prefer whichever location has an EntryValue.
+        if (PreviousLoc->holds<Loc::MMI>())
+          PreviousLoc->emplace<Loc::EntryValue>(VI.getEntryValueRegister(),
+                                                *VI.Expr);
+        LLVM_DEBUG(dbgs() << "Dropping debug info for " << VI.Var->getName()
+                          << ", conflicting fragment location types\n");
+      }
+      continue;
+    }
+
     auto RegVar = std::make_unique<DbgVariable>(
                     cast<DILocalVariable>(Var.first), Var.second);
     if (VI.inStackSlot())
-      RegVar->initializeMMI(VI.Expr, VI.getStackSlot());
+      RegVar->emplace<Loc::MMI>(VI.Expr, VI.getStackSlot());
     else
-      RegVar->initializeEntryValue(VI.getEntryValueRegister(), *VI.Expr);
+      RegVar->emplace<Loc::EntryValue>(VI.getEntryValueRegister(), *VI.Expr);
     LLVM_DEBUG(dbgs() << "Created DbgVariable for " << VI.Var->getName()
                       << "\n");
-
-    if (DbgVariable *DbgVar = MFVars.lookup(Var)) {
-      if (DbgVar->hasFrameIndexExprs() && RegVar->hasFrameIndexExprs())
-        DbgVar->addMMIEntry(*RegVar);
-      else if (VI.inEntryValueRegister())
-        DbgVar->getEntryValue()->addExpr(VI.getEntryValueRegister(), *VI.Expr);
-      else
-        LLVM_DEBUG(dbgs() << "Dropping debug info for " << VI.Var->getName()
-                          << ", conflicting fragment types\n");
-    } else if (InfoHolder.addScopeVariable(Scope, RegVar.get())) {
-      MFVars.insert({Var, RegVar.get()});
-      ConcreteEntities.push_back(std::move(RegVar));
-    }
+    InfoHolder.addScopeVariable(Scope, RegVar.get());
+    MFVars.insert({Var, RegVar.get()});
+    ConcreteEntities.push_back(std::move(RegVar));
   }
 }
 
@@ -1947,7 +1944,7 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU,
       const auto *End =
           SingleValueWithClobber ? HistoryMapEntries[1].getInstr() : nullptr;
       if (validThroughout(LScopes, MInsn, End, getInstOrdering())) {
-        RegVar->initializeDbgValue(MInsn);
+        RegVar->emplace<Loc::Single>(MInsn);
         continue;
       }
     }
@@ -1957,7 +1954,7 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU,
       continue;
 
     // Handle multiple DBG_VALUE instructions describing one variable.
-    DebugLocStream::ListBuilder List(DebugLocs, TheCU, *Asm, *RegVar, *MInsn);
+    DebugLocStream::ListBuilder List(DebugLocs, TheCU, *Asm, *RegVar);
 
     // Build the location list for this variable.
     SmallVector<DebugLocEntry, 8> Entries;
@@ -1967,7 +1964,7 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU,
     // that is valid throughout the variable's scope. If so, produce single
     // value location.
     if (isValidSingleLocation) {
-      RegVar->initializeDbgValue(Entries[0].getValues()[0]);
+      RegVar->emplace<Loc::Single>(Entries[0].getValues()[0]);
       continue;
     }