[Coroutine][DebugInfo] Enhance the ability of coroutine to debug parameters under O2

[dongAxis1944]

Summary

Coroutine copy arguments to the coroutine frame, so it will case coroutine might drop some debug info.
This patch will propagate the debugging information of the parameters to the resume function of the coroutine to help lldb and gdb print out the parameters in certain situations.

test plan:
check-llvm

[bruno]

Hi Yifeng,

Thanks for improving this. Can you add testcases to illustrate the "certain situations" you mentioned? Also, can you elaborate on the propagation approach? DeleteUselessDbgInfo looks potentially expensive, is it really needed?

[aprantl]

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

1103

Nit: According to the LLVM coding style this should be a full sentence:
// Copy debug intrinsics from the entry block if one exists.

1104

Why not just
DIBuilder DIB(*M, false)?

1147

This comment is redundant with the function name, I would just skip it.

1159

. at the end

1162

why filter out artificial arguments? This will filter out the this pointer for example.

1166

Why not handle all of them in loop? That would give the frontend more freedom.

1220

Aren't we unconditionally create DIB above?

1220

and then do for (DebugIntrinsic : DebugIntrinsics) here

1225

// Try to get the scope from the gep or use the original one.

1228

Why do we need to do this manually? Shouldn't salvageDebugInfo be able to translate a GEP into a combination of DW_OP_plus and DW_OP_deref?

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

99

This statement is not correct.

Imagine the following code:

void f(int i) {
  if (i==42) ...;
 ...
}

in IR:

if.then:
  dbg.value(i32 42, DILocalVariable("i"), DIExpression())
  // optimized out
  br %if.end
if.end:
  // Let's assume "i" has been optimized away for here on.
  dbg.value(i32 undef, DILocalVariable("i"), DIExpression())

If we remove the undef dbg.value, the debug info will make it look as if the value for i was unconditionally 42 on all paths, which would be wrong in all paths where the branch wasn't taken.

My point is, it is never safe to remove a debug intrinsic. We may be able to remove them inside of the CoroFrame code based on special knowledge that we have there, but it can't be a general helper function in Local.cpp, because what the function does isn't generally correct.

149

We can't do this here, unfortunately.

[aprantl]

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

[dongAxis1944]

Thanks for reviewing, I learn a lot.

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

[dongAxis1944]

i will upadte patch and remove DeleteUselessDbgInfo

[junparser]

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

[junparser]

In D94388#2499897, @junparser wrote:

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

So, this is not the bug, shold we avoid this by using anonymous fake variables?

[dongAxis1944]

I think if we use fake variable to solve this, it might cause the patch https://reviews.llvm.org/D93497 fail to print variable under O0 level

In D94388#2499901, @junparser wrote:

In D94388#2499897, @junparser wrote:

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

So, this is not the bug, shold we avoid this by using anonymous fake variables?

[aprantl]

In D94388#2500214, @dongAxis1944 wrote:

I think if we use fake variable to solve this, it might cause the patch https://reviews.llvm.org/D93497 fail to print variable under O0 level

In D94388#2499901, @junparser wrote:

In D94388#2499897, @junparser wrote:

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

So, this is not the bug, shold we avoid this by using anonymous fake variables?

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

[dongAxis1944]

This comment was removed by dongAxis1944.

[dongAxis1944]

Maybe we can make the two "variables" use only one dwarf var.
It might solve this problem, I think. Do you have any ideas?

In D94388#2535388, @aprantl wrote:

In D94388#2500214, @dongAxis1944 wrote:

I think if we use fake variable to solve this, it might cause the patch https://reviews.llvm.org/D93497 fail to print variable under O0 level

In D94388#2499901, @junparser wrote:

In D94388#2499897, @junparser wrote:

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

So, this is not the bug, shold we avoid this by using anonymous fake variables?

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

[junparser]

In D94388#2535388, @aprantl wrote:

In D94388#2500214, @dongAxis1944 wrote:

I think if we use fake variable to solve this, it might cause the patch https://reviews.llvm.org/D93497 fail to print variable under O0 level

In D94388#2499901, @junparser wrote:

In D94388#2499897, @junparser wrote:

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

So, this is not the bug, shold we avoid this by using anonymous fake variables?

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

Sorry for the late reply. With O1 or above, yes, we can only see artificial variables in .resume function. IIUC, we want see "real" function parameters in ramp function and artificial variables in resume function and they do have different scope, then the debugger may print them correctly. am i right?

[junparser]

In D94388#2570198, @junparser wrote:

In D94388#2535388, @aprantl wrote:

In D94388#2500214, @dongAxis1944 wrote:

I think if we use fake variable to solve this, it might cause the patch https://reviews.llvm.org/D93497 fail to print variable under O0 level

In D94388#2499901, @junparser wrote:

In D94388#2499897, @junparser wrote:

In D94388#2494294, @aprantl wrote:

Do we understand *why* the frontend is emitting the variables twice? If that just a bug in the clang frontend (which would be my first guess) then we should aim at fixing the frontend instead. If the frontend is trying to do something different, I would love to understand what that is.

Hi @aprantl,
According to the standard, coroutine needs to copy parameters (calling constructor) passed to the coroutine function by the original caller into the coroutine frame. So in frontend, we fake local variables for this behavior, that why we see two variables in debuginfo.

So, this is not the bug, shold we avoid this by using anonymous fake variables?

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

Sorry for the late reply. With O1 or above, yes, we can only see artificial variables in .resume function. IIUC, we want see "real" function parameters in ramp function and artificial variables in resume function and they do have different scope, then the debugger may print them correctly. am i right?

@aprantl any update on this?

[aprantl]

In D94388#2570198, @junparser wrote:

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

With O1 or above, yes, we can only see artificial variables in .resume function. IIUC, we want see "real" function parameters in ramp function and artificial variables in resume function and they do have different scope, then the debugger may print them correctly. am i right?

I don't think that we want the variables in the resume functions to be artificial. As far as the end-user is concerned, these are the real variables, the fact that they they have been processed by the compiler in order to pass them to the resume function is an implementation detail that the end-user shouldn't need to know about. I think it would be best to change the Clang frontend to emit these variables not as artificial. They are currently being marked as artificial because they are derived from an implicit declaration. We could either change the AST to drop the implicit bit, or special-case them in CGDebugInfo.

LLDB (and maybe other debuggers too) hide all artificial variables by default, so it would valuable to not mark them as artificial.

[junparser]

In D94388#2583321, @aprantl wrote:

In D94388#2570198, @junparser wrote:

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

With O1 or above, yes, we can only see artificial variables in .resume function. IIUC, we want see "real" function parameters in ramp function and artificial variables in resume function and they do have different scope, then the debugger may print them correctly. am i right?

I don't think that we want the variables in the resume functions to be artificial. As far as the end-user is concerned, these are the real variables, the fact that they they have been processed by the compiler in order to pass them to the resume function is an implementation detail that the end-user shouldn't need to know about. I think it would be best to change the Clang frontend to emit these variables not as artificial. They are currently being marked as artificial because they are derived from an implicit declaration. We could either change the AST to drop the implicit bit, or special-case them in CGDebugInfo.

LLDB (and maybe other debuggers too) hide all artificial variables by default, so it would valuable to not mark them as artificial.

@aprantl, In some cases, the ramp function may contains both of the variables, this may confuse the debuggers.

[dongAxis1944]

hi @junparser & @aprantl

As @junparser mentioned, there are some senses that both variables will be used. When it happens, gdb or lldb will fail to print some variables under o1 and above.
For solving this problems, we would like to merge two variables into one.
Do you have any suggestions?

Thanks in advance.

In D94388#2583862, @junparser wrote:

In D94388#2583321, @aprantl wrote:

In D94388#2570198, @junparser wrote:

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

With O1 or above, yes, we can only see artificial variables in .resume function. IIUC, we want see "real" function parameters in ramp function and artificial variables in resume function and they do have different scope, then the debugger may print them correctly. am i right?

I don't think that we want the variables in the resume functions to be artificial. As far as the end-user is concerned, these are the real variables, the fact that they they have been processed by the compiler in order to pass them to the resume function is an implementation detail that the end-user shouldn't need to know about. I think it would be best to change the Clang frontend to emit these variables not as artificial. They are currently being marked as artificial because they are derived from an implicit declaration. We could either change the AST to drop the implicit bit, or special-case them in CGDebugInfo.

LLDB (and maybe other debuggers too) hide all artificial variables by default, so it would valuable to not mark them as artificial.

@aprantl, In some cases, the ramp function may contains both of the variables, this may confuse the debuggers.

[junparser]

In D94388#2583881, @dongAxis1944 wrote:

hi @junparser & @aprantl

As @junparser mentioned, there are some senses that both variables will be used. When it happens, gdb or lldb will fail to print some variables under o1 and above.
For solving this problems, we would like to merge two variables into one.
Do you have any suggestions?

Thanks in advance.

In D94388#2583862, @junparser wrote:

In D94388#2583321, @aprantl wrote:

In D94388#2570198, @junparser wrote:

If I understand correctly, what ends up as the "real" function parameters in DWARF are the function arguments passed into the coro entry point, and the artificial variables are what is stored in the CoroFrame, and those are the only ones visible in the .resume funclets. Is this correct? If yes, we probably do want both to be visible, because they end up in different funclets.

With O1 or above, yes, we can only see artificial variables in .resume function. IIUC, we want see "real" function parameters in ramp function and artificial variables in resume function and they do have different scope, then the debugger may print them correctly. am i right?

I don't think that we want the variables in the resume functions to be artificial. As far as the end-user is concerned, these are the real variables, the fact that they they have been processed by the compiler in order to pass them to the resume function is an implementation detail that the end-user shouldn't need to know about. I think it would be best to change the Clang frontend to emit these variables not as artificial. They are currently being marked as artificial because they are derived from an implicit declaration. We could either change the AST to drop the implicit bit, or special-case them in CGDebugInfo.

LLDB (and maybe other debuggers too) hide all artificial variables by default, so it would valuable to not mark them as artificial.

@aprantl, In some cases, the ramp function may contains both of the variables, this may confuse the debuggers.

Also, under O0, the ramp and resume contain both of the variables as well since we keep everything.

llvm/include/llvm/IR/Metadata.h

public:
   /// Create a distinct version of \c N -- in place, if possible -- and return
   /// it.  Takes ownership of the temporary node.
   template <class T>
   static std::enable_if_t<std::is_base_of<MDNode, T>::value, T *>
   replaceWithDistinct(std::unique_ptr<T, TempMDNodeDeleter> N) {
     return cast<T>(N.release()->replaceWithDistinctImpl());
   }
 
+  void replaceName(MDString *NewName) { setOperand(1, NewName); }
+
 private:
   MDNode *replaceWithPermanentImpl();
   MDNode *replaceWithUniquedImpl();
   MDNode *replaceWithDistinctImpl();
 
 protected:
   /// Set an operand.
   ///

llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h

 /// Replace contents of every block in \p BBs with single unreachable
 /// instruction. If \p Updates is specified, collect all necessary DT updates
 /// into this vector. If \p KeepOneInputPHIs is true, one-input Phis in
 /// successors of blocks being deleted will be preserved.
 void DetatchDeadBlocks(ArrayRef <BasicBlock *> BBs,
                        SmallVectorImpl<DominatorTree::UpdateType> *Updates,
                        bool KeepOneInputPHIs = false);
 
+void DeleteUselessDbgInfo(ArrayRef<BasicBlock *> ToRemovedBBs);
+
 /// Delete the specified block, which must have no predecessors.
 void DeleteDeadBlock(BasicBlock *BB, DomTreeUpdater *DTU = nullptr,
                      bool KeepOneInputPHIs = false);
 
 /// Delete the specified blocks from \p BB. The set of deleted blocks must have
 /// no predecessors that are not being deleted themselves. \p BBs must have no
 /// duplicating blocks. If there are loops among this set of blocks, all
 /// relevant loop info updates should be done before this function is called.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/OptimizedStructLayout.h"
 #include "llvm/Support/circular_raw_ostream.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/PromoteMemToReg.h"
 #include <algorithm>
 
static Instruction *splitBeforeCatchSwitch(CatchSwitchInst *CatchSwitch) {
 
   auto *CleanupPad =
       CleanupPadInst::Create(CatchSwitch->getParentPad(), {}, "", CurrentBlock);
   auto *CleanupRet =
       CleanupReturnInst::Create(CleanupPad, NewBlock, CurrentBlock);
   return CleanupRet;
 }
 
+static void getNewValueStorage(Value *V, Value *&NewValue, DIExpression *&Expr) {
+  assert(V && "V should not be null");
+  while (true) {
+    if (auto *GEPInst = dyn_cast<GetElementPtrInst>(V)) {
+      Expr = llvm::salvageDebugInfoImpl(*GEPInst, Expr, false);
+      V = GEPInst->getOperand(0);
+    } else if (auto *BCInst = dyn_cast<BitCastInst>(V))
+      V = BCInst->getOperand(0);
+    else
+      break;
+  }
+  NewValue = V;
+}
+
 // Replace all alloca and SSA values that are accessed across suspend points
 // with GetElementPointer from coroutine frame + loads and stores. Create an
 // AllocaSpillBB that will become the new entry block for the resume parts of
 // the coroutine:
 //
 //    %hdl = coro.begin(...)
 //    whatever
 //

 //
 //  PostSpill:
 //    whatever
 //
 //
 static Instruction *insertSpills(const FrameDataInfo &FrameData,
                                  coro::Shape &Shape) {
   auto *CB = Shape.CoroBegin;
+  auto *M = CB->getModule();
   LLVMContext &C = CB->getContext();
   IRBuilder<> Builder(CB->getNextNode());
   StructType *FrameTy = Shape.FrameTy;
   PointerType *FramePtrTy = FrameTy->getPointerTo();
   auto *FramePtr =
       cast<Instruction>(Builder.CreateBitCast(CB, FramePtrTy, "FramePtr"));
   DominatorTree DT(*CB->getFunction());
+  // copy dbg from entry block if the dbg intrinsic existed
+  std::unique_ptr<DIBuilder> DIB = std::make_unique<DIBuilder>(*M, false);
 
   // Create a GEP with the given index into the coroutine frame for the original
   // value Orig. Appends an extra 0 index for array-allocas, preserving the
   // original type.
   auto GetFramePointer = [&](Value *Orig) -> Value * {
     FieldIDType Index = FrameData.getFieldIndex(Orig);
     SmallVector<Value *, 3> Indices = {
         ConstantInt::get(Type::getInt32Ty(C), 0),
auto GetFramePointer = [&](Value *Orig) -> Value * {
     return GEP;
   };
 
   for (auto const &E : FrameData.Spills) {
     Value *Def = E.first;
     // Create a store instruction storing the value into the
     // coroutine frame.
     Instruction *InsertPt = nullptr;
+    DbgVariableIntrinsic *DebugInstrinsic = nullptr;
+    SmallVector<DbgVariableIntrinsic *, 8> DbgUsers;
+    // find all dbg user
+    findDbgUsers(DbgUsers, Def);
     if (auto *Arg = dyn_cast<Argument>(Def)) {
       // For arguments, we will place the store instruction right after
       // the coroutine frame pointer instruction, i.e. bitcast of
       // coro.begin from i8* to %f.frame*.
       InsertPt = FramePtr->getNextNode();
 
       // If we're spilling an Argument, make sure we clear 'nocapture'
       // from the coroutine function.
       Arg->getParent()->removeParamAttr(Arg->getArgNo(), Attribute::NoCapture);
 
+      // Get debug intrinsic for argument
+      auto Iter = llvm::find_if(DbgUsers, [](DbgVariableIntrinsic *Dbg) {
+        DILocalVariable *Variable = Dbg->getVariable();
+        return Variable->isParameter() && !Variable->isArtificial();
+      });
+
+      // if we get multiple results, just pick one
+      if (Iter != DbgUsers.end())
+        DebugInstrinsic = *Iter;
     } else if (auto *CSI = dyn_cast<AnyCoroSuspendInst>(Def)) {
       // Don't spill immediately after a suspend; splitting assumes
       // that the suspend will be followed by a branch.
       InsertPt = CSI->getParent()->getSingleSuccessor()->getFirstNonPHI();
     } else {
       auto *I = cast<Instruction>(Def);
       if (!DT.dominates(CB, I)) {
         // If it is not dominated by CoroBegin, then spill should be
for (auto *U : E.second) {
         CurrentBlock = U->getParent();
         Builder.SetInsertPoint(&*CurrentBlock->getFirstInsertionPt());
 
         auto *GEP = GetFramePointer(E.first);
         GEP->setName(E.first->getName() + Twine(".reload.addr"));
         CurrentReload = Builder.CreateLoad(
             FrameTy->getElementType(FrameData.getFieldIndex(E.first)), GEP,
             E.first->getName() + Twine(".reload"));
+
+        if (DIB && DebugInstrinsic) {
+            auto *CurVar = DebugInstrinsic->getVariable();
+            assert(CurVar && "CurVar should not be null");
+            const DILocation *Loc = DebugInstrinsic->getDebugLoc().get();
+
+            // try to get scope form gep or use original one
+            Instruction *InstGEP = cast<Instruction>(GEP);
+            DIExpression *NewExpr = DIExpression::append(
+                DebugInstrinsic->getExpression(), dwarf::DW_OP_deref);
+            Value *NewValue = InstGEP;
+            getNewValueStorage(InstGEP, NewValue, NewExpr);
+            DIB->insertDbgValueIntrinsic(NewValue, CurVar, NewExpr, Loc,
+                                         cast<Instruction>(CurrentReload));
+        }
       }
 
       // If we have a single edge PHINode, remove it and replace it with a
       // reload from the coroutine frame. (We already took care of multi edge
       // PHINodes by rewriting them in the rewritePHIs function).
       if (auto *PN = dyn_cast<PHINode>(U)) {
         assert(PN->getNumIncomingValues() == 1 &&
                "unexpected number of incoming "

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

for (auto *BB : BBs) {
     new UnreachableInst(BB->getContext(), BB);
     assert(BB->getInstList().size() == 1 &&
            isa<UnreachableInst>(BB->getTerminator()) &&
            "The successor list of BB isn't empty before "
            "applying corresponding DTU updates.");
   }
 }
 
+void llvm::DeleteUselessDbgInfo(ArrayRef<BasicBlock *> ToRemovedBBs) {
+  for (BasicBlock *BB : ToRemovedBBs) {
+    for (auto &I : *BB) {
+      if (auto *Dbg = dyn_cast_or_null<DbgVariableIntrinsic>(&I)) {
+        bool SafeToRemove = false;
+        auto *Variable = Dbg->getVariable();
+        if (Variable == nullptr)
+          continue;
+
+        auto *MetaVal = MetadataAsValue::get(BB->getContext(), Variable);
+        do {
+          // 1. if user is empty, we can safe to remove it
+          if (MetaVal->user_empty()) {
+            SafeToRemove = true;
+            break;
+          }
+
+          // 2. if has use, we will keep it.
+          SafeToRemove = true;
+          for (Use &U : MetaVal->uses()) {
+            if (auto *I = dyn_cast_or_null<Instruction>(U.getUser())) {
+              BasicBlock *CurBB = I->getParent();
+              auto FindBBFn = [&](BasicBlock *BB) { return CurBB == BB; };
+              if (!llvm::any_of(ToRemovedBBs, FindBBFn)) {
+                SafeToRemove = false;
+                break;
+              }
+            }
+          }
+        } while (0);
+
+        if (SafeToRemove) {
+          // TODO: it is better to remove the useless metadata from Module, but
+          // we can not do it now
+          LLVM_DEBUG(dbgs() << "remove metadata:\n"; MetaVal->dump(););
+          std::string Name = Variable->getName().data();
+          Name += "__fake_param_" + Name;
+          Variable->replaceName(MDString::get(BB->getContext(), Name));
+        }
+      }
+    }
+  }
+}
+
 void llvm::DeleteDeadBlock(BasicBlock *BB, DomTreeUpdater *DTU,
                            bool KeepOneInputPHIs) {
   DeleteDeadBlocks({BB}, DTU, KeepOneInputPHIs);
 }
 
 void llvm::DeleteDeadBlocks(ArrayRef <BasicBlock *> BBs, DomTreeUpdater *DTU,
                             bool KeepOneInputPHIs) {
 #ifndef NDEBUG
   // Make sure that all predecessors of each dead block is also dead.
   SmallPtrSet<BasicBlock *, 4> Dead(BBs.begin(), BBs.end());
   assert(Dead.size() == BBs.size() && "Duplicating blocks?");
   for (auto *BB : Dead)
     for (BasicBlock *Pred : predecessors(BB))
       assert(Dead.count(Pred) && "All predecessors must be dead!");
 #endif
 
+  // remove dbgs
+  DeleteUselessDbgInfo(BBs);
+
   SmallVector<DominatorTree::UpdateType, 4> Updates;
   DetatchDeadBlocks(BBs, DTU ? &Updates : nullptr, KeepOneInputPHIs);
 
   if (DTU)
     DTU->applyUpdates(Updates);
 
   for (BasicBlock *BB : BBs)
     if (DTU)

llvm/lib/Transforms/Utils/Local.cpp

bool llvm::removeUnreachableBlocks(Function &F, DomTreeUpdater *DTU,
   if (MSSAU)
     MSSAU->removeBlocks(DeadBlockSet);
 
   // Loop over all of the basic blocks that are not reachable, dropping all of
   // their internal references. Update DTU if available.
   std::vector<DominatorTree::UpdateType> Updates;
   for (auto *BB : DeadBlockSet) {
     SmallSetVector<BasicBlock *, 8> UniqueSuccessors;
+    DeleteUselessDbgInfo({BB});
     for (BasicBlock *Successor : successors(BB)) {
       if (!DeadBlockSet.count(Successor))
         Successor->removePredecessor(BB);
       if (DTU)
         UniqueSuccessors.insert(Successor);
     }
     if (DTU)
       for (auto *UniqueSuccessor : UniqueSuccessors)