[DebugInfo] Improve debug info accuracy for locals after inlining alloca uses [2/3]

[Orlando]

Summary

See PR47946.

Improve debug info accuracy for locals after inlining alloca uses by teaching
the inliner to run a localized LowerDbgDeclare over those uses.

We need to do this because InstCombine's LowerDbgDeclare marks variables backed
by an alloca as located in that alloca with a dbg.value+deref at call sites
using the alloca. These locations may be incorrect after inlining. For example,
it might become possible to eliminate some stores. It would be misleading to
use the alloca as the variable location after DSE has taken place because the
alloca location won't always hold the expected value. To counter this, we need
to track assignments to the caller's alloca-backed variables through the
inlined blocks, which we achieve with a localized LowerDbgDeclare.

Just as LowerDbgDeclare deletes the dbg.declare, InlineLowerDbgDeclare deletes
the dbg.value+deref as it's no longer always true that the variable lives on
the stack at this point. The example below illustrates why we must remove the
dbg.value+deref (marked XXX) after inlining. If we do not then we will emit
debug info saying that 'param' is located on the stack throughout the inlined
function and on line STEP, which isn't true due to DSE having removed the
redundant store 'param = 2'.

int g;
void escape(int*);
__attribute__((__always_inline__))
void no_deref(int* p) {
  if (p)
    g = 5;
}
int fun(int param) {
  param = 2;                   //< Store killed after inlining 'no_deref'.

  // XXX dbg.value(%param.alloca, "param", DW_OP_deref)
  no_deref(&param);

  volatile int step = 0;       //< STEP.
  param = 4;                   //< The killing store.

  escape(&param);
  return 0;
}

debuginfo-test/dexter-test changes:
Added memvars/inlining-unused-param-dse.c
Added memvars/inlining-dse-alloca-survives.c
Added memvars/two-inlined-calls.c, currently XFAILS, addressed in next patch.
Fixed memvars/inlining-dse.c and remove XFAIL as this patch fixes the issue.

Testing:
No unexpected failures with 'ninja check-clang' and 'llvm-lit llvm/test'.
Added llvm/test/DebugInfo/Generic/inline-alloca-use.ll

[Orlando]

Rebase and fix a typo in a comment.

[Orlando]

As @bjope points out on D91425, it should be mentioned that while this solution will improve the debugging experience in a number of cases it isn't bulletproof. For example a pass could insert a non-debug instruction between the dbg.value+deref and the to-be-inlined call.

[Orlando]

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

debuginfo-tests/dexter-tests/memvars/inlining-dse-alloca-survives.c

+// REQUIRES: lldb
+// UNSUPPORTED: system-windows
+// RUN: %dexter --fail-lt 1.0 -w --debugger lldb \
+// RUN:     --builder clang-c  --cflags "-O2 -glldb" -- %s
+//
+//// See PR47946.
+//// The store to 'param' in the inlined 'use_inlined' in 'fun' is redundant
+//// and will be removed. Check that 'param' can still be read and has the
+//// expected values throughout 'fun', inlined calls to 'use_inlined', and
+//// after calling 'use_callme'.
+
+int g;
+__attribute__((__always_inline__))
+static void use_inlined(int* p) {
+  g = *p;
+  *p = 255;
+  volatile int step = 0;  // DexLabel('inlined')
+}
+
+__attribute__((__noinline__))
+void use_callme(int* q) {
+  g = *q;
+  *q = 6;
+  volatile int step = 0;  // DexLabel('callme')
+}
+
+__attribute__((__noinline__))
+int fun(int param) {
+  //// Make sure first step is in 'fun'.
+  volatile int step = 0;  // DexLabel('fun1')
+  use_inlined(&param);
+  step = 1;               // DexLabel('fun2')
+
+  param = 511;
+  step = 2;               // DexLabel('fun3')
+
+  use_callme(&param);
+  return step;            // DexLabel('fun4')
+}
+
+int main() {
+  return fun(5);
+}
+
+/*
+DexExpectWatchValue('param', '5', on_line='fun1')
+DexExpectProgramState({
+  'frames': [
+    { 'function': 'use_inlined',
+      'location': { 'lineno': 'inlined' },
+    },
+    { 'function': 'fun',
+      'watches':  { 'param': '255' }
+    },
+  ]
+})
+DexExpectWatchValue('param', '255', on_line='fun2')
+DexExpectWatchValue('param', '511', on_line='fun3')
+DexExpectProgramState({
+  'frames': [
+    { 'function': 'use_callme',
+      'location': { 'lineno': 'callme' },
+    },
+    { 'function': 'fun',
+      'watches':  { 'param': '6' }
+    },
+  ]
+})
+DexExpectWatchValue('param', '6', on_line='fun4')
+*/

debuginfo-tests/dexter-tests/memvars/inlining-dse.c

-// XFAIL:*
-//// See PR47946.
-
 // REQUIRES: lldb
 // UNSUPPORTED: system-windows
 // RUN: %dexter --fail-lt 1.0 -w --debugger lldb \
 // RUN:     --builder clang-c  --cflags "-O2 -glldb" -- %s
 //
+//// See PR47946.
 //// Check that once-escaped variable 'param' can still be read after we
 //// perform inlining + mem2reg, and that we see the DSE'd value 255.
 
-
 int g;
 __attribute__((__always_inline__))
 static void use(int* p) {
   g = *p;
   *p = 255;
   volatile int step = 0;  // DexLabel('use1')
 }
 
 __attribute__((__noinline__))
-void fun(int param) {
+int fun(int param) {
   //// Make sure first step is in 'fun'.
   volatile int step = 0;  // DexLabel('fun1')
   use(&param);
-  return;                 // DexLabel('fun2')
+  return step;            // DexLabel('fun2')
 }
 
 int main() {
-  fun(5);
+  return fun(5);
 }
 
 /*
 # Expect param == 5 before stepping through inlined 'use'.
 DexExpectWatchValue('param', '5', on_line='fun1')
 
 # Expect param == 255 after assignment in inlined frame 'use'.
 DexExpectProgramState({
   'frames': [
     { 'function': 'use',
       'location': { 'lineno': 'use1' },
     },
     { 'function': 'fun',
-      'location': { 'lineno': 20 },
       'watches':  { 'param': '255' }
     },
   ]
 })
 
 # Expect param == 255 after inlined call to 'use'.
 DexExpectWatchValue('param', '255', on_line='fun2')
 */

debuginfo-tests/dexter-tests/memvars/inlining-unused-param-dse.c

+// REQUIRES: lldb
+// UNSUPPORTED: system-windows
+// RUN: %dexter --fail-lt 1.0 -w --debugger lldb \
+// RUN:     --builder clang-c  --cflags "-O2 -glldb" -- %s
+//
+//// clang understands that the first store to 'param' in 'fun' is redundant
+//// after it inlines 'unused'. Check that 'param' can still be read and has the
+//// expected values throughout.
+
+int g;
+__attribute__((__always_inline__))
+void unused(int* p) {
+  if (p)
+    g = 5;
+}
+
+__attribute__((__noinline__))
+void escape(int* q) {
+  g = *q;
+  *q = g;
+}
+
+__attribute__((__noinline__))
+int fun(int param) {
+  //// Make sure first step is in 'fun'.
+  volatile int step = 0;  // DexLabel('fun1')
+
+  //// Store killed after inlining 'unused'.
+  param = 2;
+  step = 0;               // DexLabel('fun2')
+
+  unused(&param);
+  step = 0;               // DexLabel('fun3')
+
+  //// Killing store.
+  param = 4;
+  step = 0;               // DexLabel('fun4')
+
+  escape(&param);
+  return 0;
+}
+
+int main() {
+  return fun(1);
+}
+
+// DexExpectWatchValue('param', '1', on_line='fun1')
+// DexExpectWatchValue('param', '2', on_line='fun2')
+// DexExpectWatchValue('param', '2', on_line='fun3')
+// DexExpectWatchValue('param', '4', on_line='fun4')

debuginfo-tests/dexter-tests/memvars/two-inlined-calls.c

+//// XFAIL:*
+//// RemoveRedundantDbgInstrs is removing the second dbg.value+DW_OP_deref.
+
+// REQUIRES: lldb
+// UNSUPPORTED: system-windows
+// RUN: %dexter --fail-lt 1.0 -w --debugger lldb \
+// RUN:     --builder clang-c  --cflags "-O2 -glldb" -- %s
+//
+//// The alloca 'param' uses can be promoted after inlining, and the final
+//// store to it is redundant and will be removed. Check that 'param' can still
+//// be read and has the expeted values throughout 'fun', and inlined calls to
+//// 'use'.
+
+int g;
+__attribute__((__always_inline__))
+static void use(int* p, int value) {
+  g = *p;
+  *p = value;
+  volatile int step = 0;  // DexLabel('inlined')
+}
+
+__attribute__((__noinline__))
+int fun(int param) {
+  //// Make sure first step is in 'fun'.
+  volatile int step = 0;  // DexLabel('fun1')
+  use(&param, 10);
+  step = 1;               // DexLabel('fun2')
+  use(&param, 15);
+  return step;            // DexLabel('fun3')
+}
+
+int main() {
+  return fun(5);
+}
+
+/*
+DexExpectWatchValue('param', '5', on_line='fun1')
+DexExpectProgramState({
+  'frames': [
+    { 'function': 'use',
+      'location': { 'lineno': 'inlined' },
+    },
+    { 'function': 'fun',
+      'watches':  { 'param': '10' }
+    },
+  ]
+})
+DexExpectWatchValue('param', '10', on_line='fun2')
+DexExpectProgramState({
+  'frames': [
+    { 'function': 'use',
+      'location': { 'lineno': 'inlined' },
+    },
+    { 'function': 'fun',
+      'watches':  { 'param': '15' }
+    },
+  ]
+})
+DexExpectWatchValue('param', '15', on_line='fun3')
+*/

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
   bool extractIfOffset(int64_t &Offset) const;
 
   /// Checks if the last 4 elements of the expression are DW_OP_constu <DWARF
   /// Address Space> DW_OP_swap DW_OP_xderef and extracts the <DWARF Address
   /// Space>.
   static const DIExpression *extractAddressClass(const DIExpression *Expr,
                                                  unsigned &AddrClass);
 
+  /// Return a copy of \p Expr without the first element. \p Expr must have
+  /// at least one element.
+  static DIExpression *getTail(const DIExpression *Expr);
+
   /// Used for DIExpression::prepend.
   enum PrependOps : uint8_t {
     ApplyOffset = 0,
     DerefBefore = 1 << 0,
     DerefAfter = 1 << 1,
     StackValue = 1 << 2,
     EntryValue = 1 << 3
   };

llvm/include/llvm/Transforms/Utils/Local.h

 
 /// Inserts a llvm.dbg.value intrinsic before a load of an alloca'd value
 /// that has an associated llvm.dbg.declare or llvm.dbg.addr intrinsic.
 void ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
                                      LoadInst *LI, DIBuilder &Builder);
 
 /// Inserts a llvm.dbg.value intrinsic after a phi that has an associated
 /// llvm.dbg.declare or llvm.dbg.addr intrinsic.
-void ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
-                                     PHINode *LI, DIBuilder &Builder);
+void ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII, PHINode *LI,
+                                     DIBuilder &Builder);
+
+/// Insert dbg.value intrinsics (direct) for uses of \p DVI's location operand
+/// (indirect). If BlockSet is not null, only uses in BlockSet are
+/// considered. Returns false if this operation isn't permitted. Note that this
+/// means the function may return true even if no insertions happened.
+bool ConvertIndirectDbgIntrinsicToDbgValues(
+    DbgVariableIntrinsic *DVI, DIBuilder &DIB,
+    DenseSet<BasicBlock *> *BlockSet = nullptr);
 
 /// Lowers llvm.dbg.declare intrinsics into appropriate set of
 /// llvm.dbg.value intrinsics.
 bool LowerDbgDeclare(Function &F);
 
 /// Propagate dbg.value intrinsics through the newly inserted PHIs.
 void insertDebugValuesForPHIs(BasicBlock *BB,
                               SmallVectorImpl<PHINode *> &InsertedPHIs);

llvm/lib/IR/DebugInfoMetadata.cpp

if (Expr->Elements.size() == PatternSize)
       return nullptr;
     return DIExpression::get(Expr->getContext(),
                              makeArrayRef(&*Expr->Elements.begin(),
                                           Expr->Elements.size() - PatternSize));
   }
   return Expr;
 }
 
+DIExpression *DIExpression::getTail(const DIExpression *Expr) {
+  assert(!Expr->Elements.empty() &&
+         "Expected Expr to have at least one element");
+  return DIExpression::get(
+      Expr->getContext(),
+      makeArrayRef(&*Expr->Elements.begin() + 1, Expr->Elements.size() - 1));
+}
+
 DIExpression *DIExpression::prepend(const DIExpression *Expr, uint8_t Flags,
                                     int64_t Offset) {
   SmallVector<uint64_t, 8> Ops;
   if (Flags & DIExpression::DerefBefore)
     Ops.push_back(dwarf::DW_OP_deref);
 
   appendOffset(Ops, Offset);
   if (Flags & DIExpression::DerefAfter)

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

 
 // FIXME: Remove this flag when it is no longer necessary to convert
 // llvm.dbg.declare to avoid inaccurate debug info. Setting this to false
 // increases variable availability at the cost of accuracy. Variables that
 // cannot be promoted by mem2reg or SROA will be described as living in memory
 // for their entire lifetime. However, passes like DSE and instcombine can
 // delete stores to the alloca, leading to misleading and inaccurate debug
 // information. This flag can be removed when those passes are fixed.
-static cl::opt<unsigned> ShouldLowerDbgDeclare("instcombine-lower-dbg-declare",
-                                               cl::Hidden, cl::init(true));
+cl::opt<unsigned> ShouldLowerDbgDeclare("instcombine-lower-dbg-declare",
 
 Optional<Instruction *>
 InstCombiner::targetInstCombineIntrinsic(IntrinsicInst &II) {
   // Handle target specific intrinsics
   if (II.getCalledFunction()->isTargetIntrinsic()) {
     return TTI.instCombineIntrinsic(*this, II);
   }
   return None;

llvm/lib/Transforms/Utils/InlineFunction.cpp

 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
-#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DIBuilder.h"

 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Transforms/Utils/AssumeBundleBuilder.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <iterator>
 #include <limits>
 #include <string>
 #include <utility>
 #include <vector>
 
 using namespace llvm;
 using ProfileCount = Function::ProfileCount;
 
-static cl::opt<bool>
-EnableNoAliasConversion("enable-noalias-to-md-conversion", cl::init(true),
-  cl::Hidden,
+extern cl::opt<unsigned> ShouldLowerDbgDeclare;
+static cl::opt<bool> EnableInlineLowerDbgDeclare(
+    "inline-lower-dbg-declare", cl::init(true), cl::Hidden,
+    cl::desc("Disabled if instcombine-lower-dbg-declare is zero"));
+
+static cl::opt<bool> EnableNoAliasConversion(
+    "enable-noalias-to-md-conversion", cl::init(true), cl::Hidden,
     cl::desc("Convert noalias attributes to metadata during inlining."));
 
 // Disabled by default, because the added alignment assumptions may increase
 // compile-time and block optimizations. This option is not suitable for use
 // with frontends that emit comprehensive parameter alignment annotations.
 static cl::opt<bool>
 PreserveAlignmentAssumptions("preserve-alignment-assumptions-during-inlining",
   cl::init(false), cl::Hidden,
   cl::desc("Convert align attributes to assumptions during inlining."));
for (BasicBlock &BB : *Callee)
       // No need to update the callsite if it is pruned during inlining.
       if (!VMap || VMap->count(&BB))
         for (Instruction &I : BB)
           if (CallInst *CI = dyn_cast<CallInst>(&I))
             CI->updateProfWeight(newEntryCount, priorEntryCount);
   }
 }
 
+/// Perform LowerDbgDeclare over the inlined callee blocks for caller local
+/// alloca variables.
+static void InlineLowerDbgDeclare(CallBase &CB, Function *Caller,
+                                  Function::iterator FirstNewBlock) {
+  // InstCombine's LowerDbgDeclare marks variables backed by an alloca as
+  // located in that alloca with a dbg.value+deref at call sites using the
+  // alloca. These locations may be incorrect after inlining. For example, it
+  // might become possible to eliminate some stores. It would be misleading to
+  // use the alloca as the variable location after DSE has taken place because
+  // the alloca location won't always hold the expected value. To counter this,
+  // we need to track assignments to the caller's alloca-backed variables
+  // through the inlined blocks, which we achieve with a localized
+  // LowerDbgDeclare.
+
+  // LowerDbgDeclare inserts dbg.value(%addr, "var", DW_OP_deref) intrinics
+  // before call sites using %addr. Walk backwards from the call site looking
+  // for these, collecting them into CallerLocalAllocaVars.
+  DenseMap<DebugVariable, DbgVariableIntrinsic *> CallerLocalAllocaVars;
+  auto StartRI = std::make_reverse_iterator(CB.getIterator());
+  auto EndRI = std::make_reverse_iterator(CB.getParent()->begin());
+  for (auto RI = StartRI; RI != EndRI; RI++) {
+    if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&*RI)) {
+      if (!DVI->getExpression()->startsWithDeref())
+        continue; // Try next if this dbg.value doesn't deref.
+      if (!isa<AllocaInst>(DVI->getVariableLocation()))
+        continue; // Try next if the location isn't an alloca.
+      if (!CB.hasArgument(DVI->getVariableLocation()))
+        continue; // Try next if location isn't an arg.
+      DebugVariable Var =
+          DebugVariable(DVI->getVariable(), DVI->getExpression(),
+                        DVI->getDebugLoc().getInlinedAt());
+      CallerLocalAllocaVars.insert(std::make_pair(Var, DVI));
+    } else {
+      break; // Discovered non-dbg instruction, jump out of loop.
+    }
+  }
+
+  // Perform a localized LowerDbgDeclare over the inlined blocks.
+  if (!CallerLocalAllocaVars.empty()) {
+    bool Changed = false;
+    DenseSet<BasicBlock *> Blocks;
+    DIBuilder DIB(*FirstNewBlock->getModule(), false);
+    for (auto BBIt = FirstNewBlock; BBIt != Caller->end(); BBIt++)
+      Blocks.insert(&*BBIt);
+    for (auto Pair : CallerLocalAllocaVars) {
+      if (ConvertIndirectDbgIntrinsicToDbgValues(Pair.second, DIB, &Blocks)) {
+        Pair.second->eraseFromParent();
+        Changed = true;
+      }
+    }
+    if (Changed)
+      for (BasicBlock *BB : Blocks)
+        RemoveRedundantDbgInstrs(BB);
+  }
+}
+
 /// This function inlines the called function into the basic block of the
 /// caller. This returns false if it is not possible to inline this call.
 /// The program is still in a well defined state if this occurs though.
 ///
 /// Note that this only does one level of inlining.  For example, if the
 /// instruction 'call B' is inlined, and 'B' calls 'C', then the call to 'C' now
 /// exists in the instruction stream.  Similarly this will inline a recursive
 /// function by one level.
if (IFI.CG)
       UpdateCallGraphAfterInlining(CB, FirstNewBlock, VMap, IFI);
 
     // For 'nodebug' functions, the associated DISubprogram is always null.
     // Conservatively avoid propagating the callsite debug location to
     // instructions inlined from a function whose DISubprogram is not null.
     fixupLineNumbers(Caller, FirstNewBlock, &CB,
                      CalledFunc->getSubprogram() != nullptr);
 
+    // It is essential that fixupLineNumbers is called before
+    // InlineLowerDbgDeclare because it updates the InlinedAt field of all
+    // debug locations. For debug variable intrinsics, InlinedAt is used to
+    // identify inlined instances of a variable. The debug intrinsics that
+    // InlineLowerDbgDeclare inserts refer to the caller's variables, not the
+    // callee's.
+    if (ShouldLowerDbgDeclare && EnableInlineLowerDbgDeclare)
+      InlineLowerDbgDeclare(CB, Caller, FirstNewBlock);
+
     // Clone existing noalias metadata if necessary.
     CloneAliasScopeMetadata(CB, VMap);
 
     // Add noalias metadata if necessary.
     AddAliasScopeMetadata(CB, VMap, DL, CalleeAAR);
 
     // Clone return attributes on the callsite into the calls within the inlined
     // function which feed into its return value.

llvm/lib/Transforms/Utils/Local.cpp

static DebugLoc getDebugValueLoc(DbgVariableIntrinsic *DII, Instruction *Src) {
   // Original dbg.declare must have a location.
   DebugLoc DeclareLoc = DII->getDebugLoc();
   MDNode *Scope = DeclareLoc.getScope();
   DILocation *InlinedAt = DeclareLoc.getInlinedAt();
   // Produce an unknown location with the correct scope / inlinedAt fields.
   return DILocation::get(DII->getContext(), 0, 0, Scope, InlinedAt);
 }
 
+/// Return the expression component of \p DII. If \p DII is a dbg.value and
+/// there is a leading DW_OP_deref, return a copy of the expression without it.
+static DIExpression *getValueExpression(const DbgVariableIntrinsic *DII) {
+  DIExpression *Expr = DII->getExpression();
+  if (isa<DbgValueInst>(DII) && Expr->startsWithDeref())
+    Expr = DIExpression::getTail(Expr);
+  return Expr;
+}
+
 /// Inserts a llvm.dbg.value intrinsic before a store to an alloca'd value
 /// that has an associated llvm.dbg.declare or llvm.dbg.addr intrinsic.
 void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
                                            StoreInst *SI, DIBuilder &Builder) {
-  assert(DII->isAddressOfVariable());
+  assert(DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref());
   auto *DIVar = DII->getVariable();
   assert(DIVar && "Missing variable");
-  auto *DIExpr = DII->getExpression();
+  auto *DIExpr = getValueExpression(DII);
   Value *DV = SI->getValueOperand();
 
   DebugLoc NewLoc = getDebugValueLoc(DII, SI);
 
   if (!valueCoversEntireFragment(DV->getType(), DII)) {
     // FIXME: If storing to a part of the variable described by the dbg.declare,
     // then we want to insert a dbg.value for the corresponding fragment.
     LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: "
                       << *DII << '\n');
     // For now, when there is a store to parts of the variable (but we do not
     // know which part) we insert an dbg.value instrinsic to indicate that we
     // know nothing about the variable's content.
     DV = UndefValue::get(DV->getType());
     Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
     return;
   }
 
   Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
 }
 
-/// Inserts a llvm.dbg.value intrinsic before a load of an alloca'd value
+/// Inserts a llvm.dbg.value intrinsic after a load of an alloca'd value
 /// that has an associated llvm.dbg.declare or llvm.dbg.addr intrinsic.
 void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
                                            LoadInst *LI, DIBuilder &Builder) {
+  assert(DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref());
   auto *DIVar = DII->getVariable();
-  auto *DIExpr = DII->getExpression();
+  auto *DIExpr = getValueExpression(DII);
   assert(DIVar && "Missing variable");
 
   if (!valueCoversEntireFragment(LI->getType(), DII)) {
     // FIXME: If only referring to a part of the variable described by the
     // dbg.declare, then we want to insert a dbg.value for the corresponding
     // fragment.
     LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: "
                       << *DII << '\n');
Instruction *DbgValue = Builder.insertDbgValueIntrinsic(
       LI, DIVar, DIExpr, NewLoc, (Instruction *)nullptr);
   DbgValue->insertAfter(LI);
 }
 
 /// Inserts a llvm.dbg.value intrinsic after a phi that has an associated
 /// llvm.dbg.declare or llvm.dbg.addr intrinsic.
 void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
                                            PHINode *APN, DIBuilder &Builder) {
+  assert(DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref());
   auto *DIVar = DII->getVariable();
-  auto *DIExpr = DII->getExpression();
+  auto *DIExpr = getValueExpression(DII);
   assert(DIVar && "Missing variable");
 
   if (PhiHasDebugValue(DIVar, DIExpr, APN))
     return;
 
   if (!valueCoversEntireFragment(APN->getType(), DII)) {
     // FIXME: If only referring to a part of the variable described by the
     // dbg.declare, then we want to insert a dbg.value for the corresponding
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();
 }
 
-static bool ConvertIndirectDbgIntrinsicToDbgValues(DbgDeclareInst *DII,
-                                                   DIBuilder &DIB) {
-  assert(DII->isAddressOfVariable());
-  AllocaInst *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocation());
+bool llvm::ConvertIndirectDbgIntrinsicToDbgValues(
+    DbgVariableIntrinsic *DVI, DIBuilder &DIB,
+    DenseSet<BasicBlock *> *BlockSet) {
+  assert(DVI->isAddressOfVariable() || DVI->getExpression()->startsWithDeref());
+  AllocaInst *AI = dyn_cast_or_null<AllocaInst>(DVI->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.
   if (!AI || isArray(AI) || isStructure(AI))
     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;
       }))
     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();
+      // Skip instructions which are not in BlockSet, if it exists.
+      if (BlockSet && !BlockSet->contains(cast<Instruction>(U)->getParent()))
+        continue;
       if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
         if (AIUse.getOperandNo() == 1)
-          ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+          ConvertDebugDeclareToDebugValue(DVI, SI, DIB);
       } else if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
-        ConvertDebugDeclareToDebugValue(DII, LI, DIB);
+        ConvertDebugDeclareToDebugValue(DVI, 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(DII, nullptr);
+          DebugLoc NewLoc = getDebugValueLoc(DVI, nullptr);
           auto *Expr =
-              DIExpression::append(DII->getExpression(), dwarf::DW_OP_deref);
-          DIB.insertDbgValueIntrinsic(AI, DII->getVariable(), Expr, NewLoc, CI);
+              DIExpression::append(getValueExpression(DVI), dwarf::DW_OP_deref);
+          DIB.insertDbgValueIntrinsic(AI, DVI->getVariable(), Expr, NewLoc, CI);
         }
       } else if (BitCastInst *BI = dyn_cast<BitCastInst>(U)) {
         if (BI->getType()->isPointerTy())
           WorkList.push_back(BI);
       }
     }
   }
   return true;
 }
 
 bool llvm::LowerDbgDeclare(Function &F) {
   bool Changed = false;
   DIBuilder DIB(*F.getParent(), /*AllowUnresolved*/ false);
-  SmallVector<DbgDeclareInst *, 4> Dbgs;
+  SmallVector<DbgVariableIntrinsic *, 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;
 

llvm/test/DebugInfo/Generic/inline-alloca-use.ll

+; RUN: opt -S -inline %s  | FileCheck %s
+
+;; https://llvm.org/PR47946
+;; Check that the inliner applies LowerDbgDeclare logic to inlined uses of a
+;; caller variable backed by an alloca at the call site. LowerDbgDeclare
+;; inserts dbg.value(%loaded_value) after loads, dbg.value(%stored_value)
+;; before stores, and dbg.value(%alloca, DW_OP_deref) before calls, looking
+;; through bitcasts.
+;;
+;; Generated from the following, with some metadata stripped out.
+;; $ clang test.c -O2 -g -Xclang -disable-llvm-passes -S -emit-llvm -o tmp.ll
+;; $ opt -S tmp.ll -o - -instcombine
+;; $ cat test.c
+;; int g;
+;; void two(char* p);
+;; __attribute__((__always_inline__))
+;; static void one(int* p) {
+;;   g = *p; // load
+;;   *p = 1; // store
+;;   two((char *)p); // bitcast and call
+;; }
+;; int fun(int param) {
+;;   one(&param);
+;;   return 0;
+;; }
+
+;; Get the caller variable info for 'param' from the dbg.value inserted before
+;; the first store in 'fun' by instcombine's LowerDbgDeclare. The dbg.values that
+;; the inliner inserts should have the same !dbg attachment because they refer
+;; to the caller's variable 'param'.
+; CHECK: call void @llvm.dbg.value(metadata i32 %param, metadata ![[PARAM:[0-9]+]], metadata !DIExpression()), !dbg ![[DBG:[0-9]+]]
+
+;; Check that the dbg.value+deref inserted before the (now inlined) call by
+;; instcombine's LowerDbgDeclare is removed.
+; CHECK-NOT: call void @llvm.dbg.value({{.*}}, metadata ![[PARAM]], metadata !DIExpression(DW_OP_deref)), !dbg ![[DBG]]
+
+;; Check that the inliner inserted a dbg.value(%loaded_value) after an inlined
+;; load.
+; CHECK: %0 = load i32, i32* %param.addr
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %0, metadata ![[PARAM]], metadata !DIExpression()), !dbg ![[DBG]]
+
+;; Check that the inliner inserted a dbg.value(%stored_value) after an inlined
+;; store.
+; CHECK: call void @llvm.dbg.value(metadata i32 1, metadata ![[PARAM]], metadata !DIExpression()), !dbg ![[DBG]]
+; CHECK-NEXT: store i32 1, i32* %param.addr
+
+;; Check that the inliner inserted a dbg.value(%alloca, DW_OP_deref) before an
+;; inlined call, looking through the bitcast
+;;   %.cast.i = bitcast i32* ;; %param.addr to i8*.
+; CHECK: call void @llvm.dbg.value(metadata i32* %param.addr, metadata ![[PARAM]], metadata !DIExpression(DW_OP_deref)), !dbg ![[DBG]]
+; CHECK-NEXT: call void @two(i8* nonnull %.cast.i)
+
+;; CHECK-DAG: ![[PARAM]] = !DILocalVariable(name: "param", arg: 1, scope: ![[SCOPE:[0-9]+]],
+;; CHECK-DAG: ![[DBG]] = !DILocation(line: 0, scope: ![[SCOPE]])
+;; CHECK-DAG: ![[SCOPE]] = distinct !DISubprogram(name: "fun",
+
+@g = dso_local global i32 0, align 4, !dbg !0
+
+define dso_local i32 @fun(i32 %param) !dbg !14 {
+entry:
+  %param.addr = alloca i32, align 4
+  call void @llvm.dbg.value(metadata i32 %param, metadata !18, metadata !DIExpression()), !dbg !19
+  store i32 %param, i32* %param.addr, align 4
+  call void @llvm.dbg.value(metadata i32* %param.addr, metadata !18, metadata !DIExpression(DW_OP_deref)), !dbg !19
+  call void @one(i32* nonnull %param.addr), !dbg !24
+  ret i32 0, !dbg !25
+}
+
+define internal void @one(i32* %p) !dbg !26 {
+entry:
+  call void @llvm.dbg.value(metadata i32* %p, metadata !31, metadata !DIExpression()), !dbg !32
+  call void @llvm.dbg.value(metadata i32* %p, metadata !31, metadata !DIExpression()), !dbg !32
+  %0 = load i32, i32* %p, align 4, !dbg !33
+  store i32 %0, i32* @g, align 4, !dbg !34
+  call void @llvm.dbg.value(metadata i32* %p, metadata !31, metadata !DIExpression()), !dbg !32
+  store i32 1, i32* %p, align 4, !dbg !35
+  %.cast = bitcast i32* %p to i8*, !dbg !36
+  call void @llvm.dbg.value(metadata i32* undef, metadata !31, metadata !DIExpression()), !dbg !32
+  call void @two(i8* nonnull %.cast), !dbg !37
+  ret void, !dbg !38
+}
+
+declare !dbg !39 dso_local void @two(i8*)
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+attributes #2 = { alwaysinline }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11, !12}
+!llvm.ident = !{!13}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "g", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, retainedTypes: !5, globals: !8, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "test.c", directory: "/")
+!4 = !{}
+!5 = !{!6}
+!6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+!7 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!8 = !{!0}
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !{i32 7, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 4}
+!13 = !{!"clang version 12.0.0"}
+!14 = distinct !DISubprogram(name: "fun", scope: !3, file: !3, line: 9, type: !15, scopeLine: 9, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !17)
+!15 = !DISubroutineType(types: !16)
+!16 = !{!9, !9}
+!17 = !{!18}
+!18 = !DILocalVariable(name: "param", arg: 1, scope: !14, file: !3, line: 9, type: !9)
+!19 = !DILocation(line: 0, scope: !14)
+!24 = !DILocation(line: 10, column: 3, scope: !14)
+!25 = !DILocation(line: 11, column: 3, scope: !14)
+!26 = distinct !DISubprogram(name: "one", scope: !3, file: !3, line: 4, type: !27, scopeLine: 4, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !30)
+!27 = !DISubroutineType(types: !28)
+!28 = !{null, !29}
+!29 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
+!30 = !{!31}
+!31 = !DILocalVariable(name: "p", arg: 1, scope: !26, file: !3, line: 4, type: !29)
+!32 = !DILocation(line: 0, scope: !26)
+!33 = !DILocation(line: 5, column: 7, scope: !26)
+!34 = !DILocation(line: 5, column: 5, scope: !26)
+!35 = !DILocation(line: 6, column: 6, scope: !26)
+!36 = !DILocation(line: 7, column: 15, scope: !26)
+!37 = !DILocation(line: 7, column: 3, scope: !26)
+!38 = !DILocation(line: 8, column: 1, scope: !26)
+!39 = !DISubprogram(name: "two", scope: !3, file: !3, line: 2, type: !40, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !4)
+!40 = !DISubroutineType(types: !41)
+!41 = !{null, !6}