AsmPrinter: Rewrite initialization of DbgVariable, NFC

There are three types of `DbgVariable`:
  - alloca variables, created based on the MMI table,
  - register variables, created based on DBG_VALUE instructions, and
  - optimized-out variables.

This commit reconfigures `DbgVariable` to make it easier to tell which
kind we have, and make initialization a little clearer.

For MMI/alloca variables, `FrameIndex.size()` must always equal
`Expr.size()`, and there shouldn't be an `MInsn`.  For register
variables (with a `MInsn`), `FrameIndex` must be empty, and `Expr`
should have 0 or 1 element depending on whether it has a complex
expression (registers with multiple locations use `DebugLocListIndex`).
Optimized-out variables shouldn't have any of these fields.

Moreover, this separates DBG_VALUE initialization until after the
variable is created, simplifying logic in a future commit that changes
`collectVariableInfo()` to stop creating empty .debug_loc entries/lists.

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

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

@@ -508,7 +508,7 @@ DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
   }
 
   // .. else use frame index.
-  if (DV.getFrameIndex().back() == ~0)
+  if (DV.getFrameIndex().empty())
     return VariableDie;
 
   auto Expr = DV.getExpression().begin();
@@ -686,7 +686,7 @@ void DwarfCompileUnit::collectDeadVariables(const DISubprogram *SP) {
     SPDIE = getDIE(SP);
   assert(SPDIE);
   for (const DILocalVariable *DV : Variables) {
-    DbgVariable NewVar(DV, /* IA */ nullptr, /* Expr */ nullptr, DD);
+    DbgVariable NewVar(DV, /* IA */ nullptr, DD);
     auto VariableDie = constructVariableDIE(NewVar);
     applyVariableAttributes(NewVar, *VariableDie);
     SPDIE->addChild(std::move(VariableDie));
@@ -725,7 +725,7 @@ void DwarfCompileUnit::addGlobalType(const DIType *Ty, const DIE &Die,
 /// DbgVariable based on provided MachineLocation.
 void DwarfCompileUnit::addVariableAddress(const DbgVariable &DV, DIE &Die,
                                           MachineLocation Location) {
-  if (DV.variableHasComplexAddress())
+  if (DV.hasComplexAddress())
     addComplexAddress(DV, Die, dwarf::DW_AT_location, Location);
   else if (DV.isBlockByrefVariable())
     addBlockByrefAddress(DV, Die, dwarf::DW_AT_location, Location);

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -678,8 +678,7 @@ DbgVariable *DwarfDebug::getExistingAbstractVariable(InlinedVariable IV) {
 
 void DwarfDebug::createAbstractVariable(const DILocalVariable *Var,
                                         LexicalScope *Scope) {
-  auto AbsDbgVariable =
+  auto AbsDbgVariable = make_unique<DbgVariable>(Var, /* IA */ nullptr, this);
-      make_unique<DbgVariable>(Var, /* IA */ nullptr, /* Expr */ nullptr, this);
   InfoHolder.addScopeVariable(Scope, AbsDbgVariable.get());
   AbstractVariables[Var] = std::move(AbsDbgVariable);
 }
@@ -722,10 +721,9 @@ void DwarfDebug::collectVariableInfoFromMMITable(
     if (!Scope)
       continue;
 
-    const DIExpression *Expr = cast_or_null<DIExpression>(VI.Expr);
     ensureAbstractVariableIsCreatedIfScoped(Var, Scope->getScopeNode());
-    auto RegVar =
+    auto RegVar = make_unique<DbgVariable>(Var.first, Var.second, this);
-        make_unique<DbgVariable>(Var.first, Var.second, Expr, this, VI.Slot);
+    RegVar->initializeMMI(VI.Expr, VI.Slot);
     if (InfoHolder.addScopeVariable(Scope, RegVar.get()))
       ConcreteVariables.push_back(std::move(RegVar));
   }
@@ -870,6 +868,14 @@ DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
   }
 }
 
+DbgVariable *DwarfDebug::createConcreteVariable(LexicalScope &Scope,
+                                                InlinedVariable IV) {
+  ensureAbstractVariableIsCreatedIfScoped(IV, Scope.getScopeNode());
+  ConcreteVariables.push_back(
+      make_unique<DbgVariable>(IV.first, IV.second, this));
+  InfoHolder.addScopeVariable(&Scope, ConcreteVariables.back().get());
+  return ConcreteVariables.back().get();
+}
 
 // Find variables for each lexical scope.
 void DwarfDebug::collectVariableInfo(DwarfCompileUnit &TheCU,
@@ -898,12 +904,11 @@ void DwarfDebug::collectVariableInfo(DwarfCompileUnit &TheCU,
       continue;
 
     Processed.insert(IV);
+    DbgVariable *RegVar = createConcreteVariable(*Scope, IV);
+
     const MachineInstr *MInsn = Ranges.front().first;
     assert(MInsn->isDebugValue() && "History must begin with debug value");
-    ensureAbstractVariableIsCreatedIfScoped(IV, Scope->getScopeNode());
+    RegVar->initializeDbgValue(MInsn);
-    ConcreteVariables.push_back(make_unique<DbgVariable>(MInsn, this));
-    DbgVariable *RegVar = ConcreteVariables.back().get();
-    InfoHolder.addScopeVariable(Scope, RegVar);
 
     // Check if the first DBG_VALUE is valid for the rest of the function.
     if (Ranges.size() == 1 && Ranges.front().second == nullptr)
@@ -930,15 +935,9 @@ void DwarfDebug::collectVariableInfo(DwarfCompileUnit &TheCU,
 
   // Collect info for variables that were optimized out.
   for (const DILocalVariable *DV : SP->getVariables()) {
-    if (!Processed.insert(InlinedVariable(DV, nullptr)).second)
+    if (Processed.insert(InlinedVariable(DV, nullptr)).second)
-      continue;
+      if (LexicalScope *Scope = LScopes.findLexicalScope(DV->getScope()))
-    if (LexicalScope *Scope = LScopes.findLexicalScope(DV->getScope())) {
+        createConcreteVariable(*Scope, InlinedVariable(DV, nullptr));
-      ensureAbstractVariableIsCreatedIfScoped(InlinedVariable(DV, nullptr),
-                                              Scope->getScopeNode());
-      ConcreteVariables.push_back(make_unique<DbgVariable>(
-          DV, /* IA */ nullptr, /* Expr */ nullptr, this));
-      InfoHolder.addScopeVariable(Scope, ConcreteVariables.back().get());
-    }
   }
 }
 

lib/CodeGen/AsmPrinter/DwarfDebug.h

@@ -67,42 +67,61 @@ public:
 };
 
 //===----------------------------------------------------------------------===//
-/// \brief This class is used to track local variable information.
+/// This class is used to track local variable information.
 ///
-/// - Variables whose location changes over time have a DebugLocListIndex and
+/// Variables can be created from allocas, in which case they're generated from
-///   the other fields are not used.
+/// the MMI table.  Such variables can have multiple expressions and frame
+/// indices.  The \a Expr and \a FrameIndices array must match.
 ///
-/// - Variables that are described by multiple MMI table entries have multiple
+/// Variables can be created from \c DBG_VALUE instructions.  Those whose
-///   expressions and frame indices.
+/// location changes over time use \a DebugLocListIndex, while those with a
+/// single instruction use \a MInsn and (optionally) a single entry of \a Expr.
+///
+/// Variables that have been optimized out use none of these fields.
 class DbgVariable {
-  const DILocalVariable *Var; /// Variable Descriptor.
+  const DILocalVariable *Var;                /// Variable Descriptor.
-  const DILocation *IA;       /// Inlined at location.
+  const DILocation *IA;                      /// Inlined at location.
-  SmallVector<const DIExpression *, 1>
+  SmallVector<const DIExpression *, 1> Expr; /// Complex address.
-      Expr;                          /// Complex address location expression.
+  DIE *TheDIE = nullptr;                     /// Variable DIE.
-  DIE *TheDIE;                /// Variable DIE.
+  unsigned DebugLocListIndex = ~0u;          /// Offset in DebugLocs.
-  unsigned DebugLocListIndex;        /// Offset in DebugLocs.
+  const MachineInstr *MInsn = nullptr;       /// DBG_VALUE instruction.
-  const MachineInstr *MInsn;  /// DBG_VALUE instruction of the variable.
+  SmallVector<int, 1> FrameIndex;            /// Frame index.
-  SmallVector<int, 1> FrameIndex; /// Frame index of the variable.
   DwarfDebug *DD;
 
 public:
-  /// Construct a DbgVariable from a variable.
+  /// Construct a DbgVariable.
-  DbgVariable(const DILocalVariable *V, const DILocation *IA,
+  ///
-              const DIExpression *E, DwarfDebug *DD, int FI = ~0)
+  /// Creates a variable without any DW_AT_location.  Call \a initializeMMI()
-      : Var(V), IA(IA), Expr(1, E), TheDIE(nullptr), DebugLocListIndex(~0U),
+  /// for MMI entries, or \a initializeDbgValue() for DBG_VALUE instructions.
-        MInsn(nullptr), DD(DD) {
+  DbgVariable(const DILocalVariable *V, const DILocation *IA, DwarfDebug *DD)
+      : Var(V), IA(IA), DD(DD) {}
+
+  /// Initialize from the MMI table.
+  void initializeMMI(const DIExpression *E, int FI) {
+    assert(Expr.empty() && "Already initialized?");
+    assert(FrameIndex.empty() && "Already initialized?");
+    assert(!MInsn && "Already initialized?");
+
+    assert((!E || E->isValid()) && "Expected valid expression");
+    assert(~FI && "Expected valid index");
+
+    Expr.push_back(E);
     FrameIndex.push_back(FI);
-    assert(!E || E->isValid());
   }
 
-  /// Construct a DbgVariable from a DEBUG_VALUE.
+  /// Initialize from a DBG_VALUE instruction.
-  /// AbstractVar may be NULL.
+  void initializeDbgValue(const MachineInstr *DbgValue) {
-  DbgVariable(const MachineInstr *DbgValue, DwarfDebug *DD)
+    assert(Expr.empty() && "Already initialized?");
-      : Var(DbgValue->getDebugVariable()),
+    assert(FrameIndex.empty() && "Already initialized?");
-        IA(DbgValue->getDebugLoc()->getInlinedAt()),
+    assert(!MInsn && "Already initialized?");
-        Expr(1, DbgValue->getDebugExpression()), TheDIE(nullptr),
+
-        DebugLocListIndex(~0U), MInsn(DbgValue), DD(DD) {
+    assert(Var == DbgValue->getDebugVariable() && "Wrong variable");
-    FrameIndex.push_back(~0);
+    assert(IA == DbgValue->getDebugLoc()->getInlinedAt() && "Wrong inlined-at");
+
+    MInsn = DbgValue;
+    if (auto *E = DbgValue->getDebugExpression())
+      if (E->getNumElements())
+        Expr.push_back(E);
   }
 
   // Accessors.
@@ -123,17 +142,16 @@ public:
     assert(V.Var == Var && "conflicting variable");
     assert(V.IA == IA && "conflicting inlined-at location");
 
-    if (V.getFrameIndex().back() != ~0) {
+    assert(!FrameIndex.empty() && "Expected an MMI entry");
-      auto E = V.getExpression();
+    assert(!V.FrameIndex.empty() && "Expected an MMI entry");
-      auto FI = V.getFrameIndex();
+    assert(Expr.size() == FrameIndex.size() && "Mismatched expressions");
-      Expr.append(E.begin(), E.end());
+    assert(V.Expr.size() == V.FrameIndex.size() && "Mismatched expressions");
-      FrameIndex.append(FI.begin(), FI.end());
+
-    }
+    Expr.append(V.Expr.begin(), V.Expr.end());
-    assert(Expr.size() > 1 ? std::all_of(Expr.begin(), Expr.end(),
+    FrameIndex.append(V.FrameIndex.begin(), V.FrameIndex.end());
-                                         [](const DIExpression *E) {
+    assert(std::all_of(Expr.begin(), Expr.end(), [](const DIExpression *E) {
-                                           return E->isBitPiece();
+             return E && E->isBitPiece();
-                                         })
+           }) && "conflicting locations for variable");
-                           : (true && "conflicting locations for variable"));
   }
 
   // Translate tag to proper Dwarf tag.
@@ -160,11 +178,13 @@ public:
     return false;
   }
 
-  bool variableHasComplexAddress() const {
+  bool hasComplexAddress() const {
-    assert(Var && "Invalid complex DbgVariable!");
+    assert(MInsn && "Expected DBG_VALUE, not MMI variable");
-    assert(Expr.size() == 1 &&
+    assert(FrameIndex.empty() && "Expected DBG_VALUE, not MMI variable");
-           "variableHasComplexAddress() invoked on multi-FI variable");
+    assert(
-    return Expr.back()->getNumElements() > 0;
+        (Expr.empty() || (Expr.size() == 1 && Expr.back()->getNumElements())) &&
+        "Invalid Expr for DBG_VALUE");
+    return !Expr.empty();
   }
   bool isBlockByrefVariable() const;
   const DIType *getType() const;
@@ -344,6 +364,8 @@ class DwarfDebug : public AsmPrinterHandler {
   void ensureAbstractVariableIsCreatedIfScoped(InlinedVariable Var,
                                                const MDNode *Scope);
 
+  DbgVariable *createConcreteVariable(LexicalScope &Scope, InlinedVariable IV);
+
   /// \brief Construct a DIE for this abstract scope.
   void constructAbstractSubprogramScopeDIE(LexicalScope *Scope);