[NFC][AsmPrinter] Remove dead multi-MMI handling from DwarfFile::addScopeVariable

[fdeazeve]

LGTM! Thanks for doing this

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

324

As a follow up comment on the mutable thing in the other revision, if FrameIndexExprs were a set we wouldn't need this call to none_of nor the mutable member. Also not something for this patch though!

1572

Yes! Really glad we now query _before_ creating the new Dbg var!

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

public:
   const DbgValueLoc *getValueLoc() const {
     if (auto *M = std::get_if<SingleLoc>(this))
       return M->ValueLoc.get();
     return nullptr;
   }
   /// Get the FI entries, sorted by fragment offset.
   ArrayRef<FrameIndexExpr> getFrameIndexExprs() const;
   bool hasFrameIndexExprs() const { return holds<MMILoc>(); }
-  void addMMIEntry(const DbgVariable &V);
+  void addMMIEntry(const DIExpression *Expr, int FI);
 
   // Translate tag to proper Dwarf tag.
   dwarf::Tag getTag() const {
     // FIXME: Why don't we just infer this tag and store it all along?
     if (getVariable()->isParameter())
       return dwarf::DW_TAG_formal_parameter;
 
     return dwarf::DW_TAG_variable;

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

llvm::sort(FrameIndexExprs,
              [](const FrameIndexExpr &A, const FrameIndexExpr &B) -> bool {
                return A.Expr->getFragmentInfo()->OffsetInBits <
                       B.Expr->getFragmentInfo()->OffsetInBits;
              });
 
   return FrameIndexExprs;
 }
 
-void DbgVariable::addMMIEntry(const DbgVariable &V) {
-  assert(V.getVariable() == getVariable() && "conflicting variable");
-  assert(V.getInlinedAt() == getInlinedAt() && "conflicting inlined-at location");
-
+void DbgVariable::addMMIEntry(const DIExpression *Expr, int FI) {
   auto &FrameIndexExprs = get<MMILoc>().FrameIndexExprs;
-  auto &VFrameIndexExprs = V.get<MMILoc>().FrameIndexExprs;
 
   // 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
   // non-fragment entries for the same variable at different frame indices.
   if (FrameIndexExprs.size()) {
     auto *Expr = FrameIndexExprs.back().Expr;
     if (!Expr || !Expr->isFragment())
       return;
   }
 
-  for (const auto &FIE : VFrameIndexExprs)
-    // Ignore duplicate entries.
-    if (llvm::none_of(FrameIndexExprs, [&](const FrameIndexExpr &Other) {
-          return FIE.FI == Other.FI && FIE.Expr == Other.Expr;
-        }))
-      FrameIndexExprs.push_back(FIE);
+        return FI == Other.FI && Expr == Other.Expr;
+    FrameIndexExprs.push_back({FI, Expr});
 
   assert((FrameIndexExprs.size() == 1 ||
           llvm::all_of(FrameIndexExprs,
                        [](FrameIndexExpr &FIE) {
                          return FIE.Expr && FIE.Expr->isFragment();
                        })) &&
          "conflicting locations for variable");
 }
for (const auto &VI : Asm->MF->getVariableDbgInfo()) {
     // If variable scope is not found then skip this variable.
     if (!Scope) {
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << VI.Var->getName()
                         << ", no variable scope found\n");
       continue;
     }
 
     ensureAbstractEntityIsCreatedIfScoped(TheCU, Var.first, Scope->getScopeNode());
+
+    if (DbgVariable *DbgVar = MFVars.lookup(Var)) {
+      if (DbgVar->hasFrameIndexExprs())
+        DbgVar->addMMIEntry(VI.Expr, VI.getStackSlot());
+      else
+        DbgVar->addEntryValueExpr(VI.getEntryValueRegister(), *VI.Expr);
+      continue;
+    }
+
     auto RegVar = std::make_unique<DbgVariable>(
                     cast<DILocalVariable>(Var.first), Var.second);
     if (VI.inStackSlot())
       RegVar->initializeMMI(VI.Expr, VI.getStackSlot());
     else
       RegVar->initializeEntryValue(VI.getEntryValueRegister(), *VI.Expr);
     LLVM_DEBUG(dbgs() << "Created DbgVariable for " << VI.Var->getName()
                       << "\n");
-
-    if (DbgVariable *DbgVar = MFVars.lookup(Var)) {
-      if (DbgVar->hasFrameIndexExprs())
-        DbgVar->addMMIEntry(*RegVar);
-      else
-        DbgVar->addEntryValueExpr(VI.getEntryValueRegister(), *VI.Expr);
-    } else if (InfoHolder.addScopeVariable(Scope, RegVar.get())) {
-      MFVars.insert({Var, RegVar.get()});
-      ConcreteEntities.push_back(std::move(RegVar));
-    }
-  }
-}
+    InfoHolder.addScopeVariable(Scope, RegVar.get());
 
 /// Determine whether a *singular* DBG_VALUE is valid for the entirety of its
 /// enclosing lexical scope. The check ensures there are no other instructions
 /// in the same lexical scope preceding the DBG_VALUE and that its range is
 /// either open or otherwise rolls off the end of the scope.
 static bool validThroughout(LexicalScopes &LScopes,
                             const MachineInstr *DbgValue,
                             const MachineInstr *RangeEnd,

llvm/lib/CodeGen/AsmPrinter/DwarfFile.h

public:
   DwarfStringPool &getStringPool() { return StrPool; }
 
   MCSymbol *getStringOffsetsStartSym() const { return StringOffsetsStartSym; }
   void setStringOffsetsStartSym(MCSymbol *Sym) { StringOffsetsStartSym = Sym; }
 
   MCSymbol *getRnglistsTableBaseSym() const { return RnglistsTableBaseSym; }
   void setRnglistsTableBaseSym(MCSymbol *Sym) { RnglistsTableBaseSym = Sym; }
 
-  /// \returns false if the variable was merged with a previous one.
-  bool addScopeVariable(LexicalScope *LS, DbgVariable *Var);
+  void addScopeVariable(LexicalScope *LS, DbgVariable *Var);
 
   void addScopeLabel(LexicalScope *LS, DbgLabel *Label);
 
   DenseMap<LexicalScope *, ScopeVars> &getScopeVariables() {
     return ScopeVariables;
   }
 
   DenseMap<LexicalScope *, LabelList> &getScopeLabels() {

llvm/lib/CodeGen/AsmPrinter/DwarfFile.cpp

 void DwarfFile::emitAbbrevs(MCSection *Section) { Abbrevs.Emit(Asm, Section); }
 
 // Emit strings into a string section.
 void DwarfFile::emitStrings(MCSection *StrSection, MCSection *OffsetSection,
                             bool UseRelativeOffsets) {
   StrPool.emit(*Asm, StrSection, OffsetSection, UseRelativeOffsets);
 }
 
-bool DwarfFile::addScopeVariable(LexicalScope *LS, DbgVariable *Var) {
+void DwarfFile::addScopeVariable(LexicalScope *LS, DbgVariable *Var) {
   auto &ScopeVars = ScopeVariables[LS];
   const DILocalVariable *DV = Var->getVariable();
   if (unsigned ArgNum = DV->getArg()) {
-    auto Cached = ScopeVars.Args.find(ArgNum);
-    if (Cached == ScopeVars.Args.end())
-      ScopeVars.Args[ArgNum] = Var;
-    else {
-      Cached->second->addMMIEntry(*Var);
-      return false;
-    }
+    auto Ret = ScopeVars.Args.insert({ArgNum, Var});
+    assert(Ret.second);
   } else {
     ScopeVars.Locals.push_back(Var);
   }
-  return true;
 }
 
 void DwarfFile::addScopeLabel(LexicalScope *LS, DbgLabel *Label) {
   SmallVectorImpl<DbgLabel *> &Labels = ScopeLabels[LS];
   Labels.push_back(Label);
 }
 
 std::pair<uint32_t, RangeSpanList *>
 DwarfFile::addRange(const DwarfCompileUnit &CU, SmallVector<RangeSpan, 2> R) {
   CURangeLists.push_back(
       RangeSpanList{Asm->createTempSymbol("debug_ranges"), &CU, std::move(R)});
   return std::make_pair(CURangeLists.size() - 1, &CURangeLists.back());
 }