[NFC][DebugInfo] Factor LowerDbgDeclare into two functions [1/3]

[Orlando]

Summary

Setup work for https://llvm.org/PR47946. Split out the core functionality from
LowerDbgDeclare into a function called ConvertIndirectDbgIntrinsicToDbgValues
which will be used in subsequent patches.

This has been separated out from D91424 to help highlight the functional changes
in that patch.

[aprantl]

This is fine apart from some nitpicks inside.

llvm/lib/Transforms/Utils/Local.cpp

1483–1484

Nit: remove LowerDbgDeclare -
Also, the doxygen comment should be in the header file and only there.

1484

Doxygen comment here to explain what this function does?
Which other indirect intrinsics are there?
I'm not sure if the name indirect captures the intention well unless it's part of a greater plan for the future. Otherwise I would just say DbgDeclare to avoid confusion.

[Orlando]

Thanks @aprantl for looking at this. I meant to state in the description that this NFC change is set up for D91424 to help highlight the functional change there.

llvm/lib/Transforms/Utils/Local.cpp

1483–1484

I left this as I found it, but I will happily update it.

1484

Naming this was a little tricky as in the next patch in this stack (D91424) the function is changed to work on dbg.value+derefs too.

[Orlando]

Updated based on @aprantl's comments. I removed LowerDbgDeclare's doxygen comment from Local.cpp (it still exists in the header Local.h). I didn't add a doxygen comment to the new function ConvertIndirectDbgIntrinsicToDbgValues because it is added in the header in the next patch D91424.

[Orlando]

Abandoning this old patch set; assignment tracking achieves a better result.

llvm/lib/Transforms/Utils/Local.cpp

static bool isArray(AllocaInst *AI) {
   return AI->isArrayAllocation() ||
          (AI->getAllocatedType() && AI->getAllocatedType()->isArrayTy());
 }
 
 /// Determine whether this alloca is a structure.
 static bool isStructure(AllocaInst *AI) {
   return AI->getAllocatedType() && AI->getAllocatedType()->isStructTy();
 }
 
-/// LowerDbgDeclare - Lowers llvm.dbg.declare intrinsics into appropriate set
-/// of llvm.dbg.value intrinsics.
-bool llvm::LowerDbgDeclare(Function &F) {
-  bool Changed = false;
-  DIBuilder DIB(*F.getParent(), /*AllowUnresolved*/ false);
-  SmallVector<DbgDeclareInst *, 4> Dbgs;
-  for (auto &FI : F)
-    for (Instruction &BI : FI)
-      if (auto DDI = dyn_cast<DbgDeclareInst>(&BI))
-        Dbgs.push_back(DDI);
-
-  if (Dbgs.empty())
-    return Changed;
-
-  for (auto &I : Dbgs) {
-    DbgDeclareInst *DDI = I;
-    AllocaInst *AI = dyn_cast_or_null<AllocaInst>(DDI->getAddress());
-    // If this is an alloca for a scalar variable, insert a dbg.value
-    // at each load and store to the alloca and erase the dbg.declare.
-    // The dbg.values allow tracking a variable even if it is not
-    // stored on the stack, while the dbg.declare can only describe
-    // the stack slot (and at a lexical-scope granularity). Later
-    // passes will attempt to elide the stack slot.
-    if (!AI || isArray(AI) || isStructure(AI))
-      continue;
+static bool ConvertIndirectDbgIntrinsicToDbgValues(DbgDeclareInst *DII,
+                                                   DIBuilder &DIB) {
+  assert(DII->isAddressOfVariable());
+  AllocaInst *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocation());
+  // If this is an alloca for a scalar variable, insert a dbg.value at each
+  // load and store to the alloca and erase the dbg.declare.  The dbg.values
+  // allow tracking a variable even if it is not stored on the stack, while the
+  // dbg.declare can only describe the stack slot (and at a lexical-scope
+  // granularity). Later passes will attempt to elide the stack slot.
+    return false;
 
-    // A volatile load/store means that the alloca can't be elided anyway.
-    if (llvm::any_of(AI->users(), [](User *U) -> bool {
-          if (LoadInst *LI = dyn_cast<LoadInst>(U))
-            return LI->isVolatile();
-          if (StoreInst *SI = dyn_cast<StoreInst>(U))
-            return SI->isVolatile();
-          return false;
-        }))
-      continue;
+    return false;
 
-    SmallVector<const Value *, 8> WorkList;
-    WorkList.push_back(AI);
-    while (!WorkList.empty()) {
-      const Value *V = WorkList.pop_back_val();
-      for (auto &AIUse : V->uses()) {
-        User *U = AIUse.getUser();
-        if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
-          if (AIUse.getOperandNo() == 1)
-            ConvertDebugDeclareToDebugValue(DDI, SI, DIB);
-        } else if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
-          ConvertDebugDeclareToDebugValue(DDI, LI, DIB);
-        } else if (CallInst *CI = dyn_cast<CallInst>(U)) {
-          // This is a call by-value or some other instruction that takes a
-          // pointer to the variable. Insert a *value* intrinsic that describes
-          // the variable by dereferencing the alloca.
-          if (!CI->isLifetimeStartOrEnd()) {
-            DebugLoc NewLoc = getDebugValueLoc(DDI, nullptr);
-            auto *DerefExpr =
-                DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref);
-            DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr,
-                                        NewLoc, CI);
-          }
-        } else if (BitCastInst *BI = dyn_cast<BitCastInst>(U)) {
-          if (BI->getType()->isPointerTy())
-            WorkList.push_back(BI);
-        }
-      }
-    }
+          ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+        ConvertDebugDeclareToDebugValue(DII, LI, DIB);
+          DebugLoc NewLoc = getDebugValueLoc(DII, nullptr);
+          auto *Expr =
+              DIExpression::append(DII->getExpression(), dwarf::DW_OP_deref);
+          DIB.insertDbgValueIntrinsic(AI, DII->getVariable(), Expr, NewLoc, CI);
+  return true;
+}
+
+bool llvm::LowerDbgDeclare(Function &F) {
+  bool Changed = false;
+  DIBuilder DIB(*F.getParent(), /*AllowUnresolved*/ false);
+  SmallVector<DbgDeclareInst *, 4> Dbgs;
+  for (auto &FI : F)
+    for (Instruction &BI : FI)
+      if (auto DDI = dyn_cast<DbgDeclareInst>(&BI))
+        Dbgs.push_back(DDI);
+
+  if (Dbgs.empty())
+    return Changed;
+
+  for (auto *DDI : Dbgs) {
+    if (ConvertIndirectDbgIntrinsicToDbgValues(DDI, DIB)) {
       DDI->eraseFromParent();
       Changed = true;
     }
+  }
 
   if (Changed)
     for (BasicBlock &BB : F)
       RemoveRedundantDbgInstrs(&BB);
 
   return Changed;
 }
 
 /// Propagate dbg.value intrinsics through the newly inserted PHIs.
 void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
                                     SmallVectorImpl<PHINode *> &InsertedPHIs) {
   assert(BB && "No BasicBlock to clone dbg.value(s) from.");