[ADCE] Follow inlinedAt when determining if a DbgInfoIntrinsic is dead

[uabelho]

Summary

When inlining has occured there can be calls to dbg.value with debug info
pointing to a scope in the inlined function, and without this patch, such
dbg.values were removed.

However, if we follow the information in the inlinedAt fields we can reach
the scope the call was originally in, and thus we can keep the call and
improve debug info.

[aprantl]

I don't understand the example in the testcase. There is no instruction belonging to f1, so the debugger can never stop at f1 to print the value of the anonymous variable !5.

[aprantl]

Besides that not (yet?) being convinced that this is useful, I'm also concerned by the performance impact of the change. Did you do any measurements with large C++ programs with lots of small inlined functions?

[uabelho]

In D37095#851557, @aprantl wrote:

I don't understand the example in the testcase. There is no instruction belonging to f1, so the debugger can never stop at f1 to print the value of the anonymous variable !5.

Very good point. Thanks!

We did this change (for some now unknown reason unfortunately) locally in our tree a year and a half ago and I thought I would try to
upstream it, but as you're saying, it's very questionable what it's actually good for.

If I make f1 more complicated (e.g. adding "i = i + 1;" to it), both that instruction as well as a dbg.value survives in f2.

So instead of pushing this, I'll remove our local change and drop the patch. If it's really needed I suppose it will pop up again
and I can come back with a proper patch.

Thanks and sorry for wasting you time.

lib/Transforms/Scalar/ADCE.cpp

void AggressiveDeadCodeElimination::markLiveBranchesFromControlDependences() {
   }
 }
 
 //===----------------------------------------------------------------------===//
 //
 //  Routines to update the CFG and SSA information before removing dead code.
 //
 //===----------------------------------------------------------------------===//
+
+/// Traverse through the inlinedAt field of \p DL and each subsequent DebugLoc
+/// until no more inlinedAt field is found.
+static DebugLoc getInlinedAtLeaf(DebugLoc DL) {
+  while (DebugLoc InlinedAt = DL.getInlinedAt()) {
+    DL = InlinedAt;
+  }
+
+  return DL;
+}
+
 bool AggressiveDeadCodeElimination::removeDeadInstructions() {
 
   // Updates control and dataflow around dead blocks
   updateDeadRegions();
 
   DEBUG({
     for (Instruction &I : instructions(F)) {
       // Check if the instruction is alive.
       if (isLive(&I))
         continue;
 
       if (auto *DII = dyn_cast<DbgInfoIntrinsic>(&I)) {
-        // Check if the scope of this variable location is alive.
-        if (AliveScopes.count(DII->getDebugLoc()->getScope()))
+        // Check if the scope of this variable location is alive. Follow each
+        // "inlinedAt" if present.
+        if (AliveScopes.count(getInlinedAtLeaf(DII->getDebugLoc()).getScope()))
           continue;
 
         // If intrinsic is pointing at a live SSA value, there may be an
         // earlier optimization bug: if we know the location of the variable,
         // why isn't the scope of the location alive?
         if (Value *V = DII->getVariableLocation())
           if (Instruction *II = dyn_cast<Instruction>(V))
             if (isLive(II))
               dbgs() << "Dropping debug info for " << *DII << "\n";
       }
     }
   });
 
   // The inverse of the live set is the dead set.  These are those instructions
   // that have no side effects and do not influence the control flow or return
   // value of the function, and may therefore be deleted safely.
   // NOTE: We reuse the Worklist vector here for memory efficiency.
   for (Instruction &I : instructions(F)) {
     // Check if the instruction is alive.
     if (isLive(&I))
       continue;
 
     if (auto *DII = dyn_cast<DbgInfoIntrinsic>(&I)) {
-      // Check if the scope of this variable location is alive.
-      if (AliveScopes.count(DII->getDebugLoc()->getScope()))
+      // Check if the scope of this variable location is alive. Follow each
+      // "inlinedAt" if present.
+      if (AliveScopes.count(getInlinedAtLeaf(DII->getDebugLoc()).getScope()))
         continue;
 
       // Fallthrough and drop the intrinsic.
     }
 
     // Prepare to delete.
     Worklist.push_back(&I);
     I.dropAllReferences();

test/Transforms/ADCE/debug-info-intrinsic-inlinedat.ll

+; RUN: opt -S -adce %s -o - | FileCheck %s
+
+; The call to dbg.value is the result of a previously inlined call to function
+; f1. Thus !9 uses a scope in f1, and "inlinedAt" is set, pointing to the
+; location of the call.
+
+; This dbg.value can be useful and shouldn't be removed. Make sure it's still there
+; after ADCE has run.
+
+@a = external global i16
+@b = external global i16
+
+define void @f2() {
+  %_tmp = load i16, i16* @a, !dbg !8
+  call void @llvm.dbg.value(metadata i16 %_tmp, metadata !5, metadata !6), !dbg !9
+  store i16 %_tmp, i16* @b
+  ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!2, !3}
+
+!0 = !DIFile(filename: "foo.c", directory: "bar")
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !0)
+!2 = !{i32 2, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = distinct !DISubprogram(name: "f1", unit: !1)
+!5 = !DILocalVariable(scope: !4)
+!6 = !DIExpression()
+!7 = distinct !DISubprogram(name: "f2", unit: !1)
+!8 = !DILocation(scope: !7)
+!9 = !DILocation(scope: !4, inlinedAt: !10)
+!10 = distinct !DILocation(scope: !7)
+
+; Make sure there is still a dbg.value for %_tmp
+
+; CHECK: define void @f2
+; CHECK: %_tmp = load
+; CHECK: call void @llvm.dbg.value(metadata i16 %_tmp
+; CHECK: store i16 %_tmp