[DebugInfo] Avoid generating duplicate llvm.dbg.value

[alok]

Summary

This is to avoid generating duplicate llvm.dbg.value instrinsic if it already exists after the Instruction.

Summary:

 Before inserting llvm.dbg.value instruction, LLVM checks if the same instruction is already present before the instruction to avoid duplicates.

Currently it misses to check if it already exists after the instruction.
flang generates IR like this.

---

%4 = load i32, i32* %i1_311, align 4, !dbg !42
call void @llvm.dbg.value(metadata i32 %4, metadata !35, metadata !DIExpression()), !dbg !33

---

When this IR is processed in llvm, it ends up inserting duplicates.

---

%4 = load i32, i32* %i1_311, align 4, !dbg !42
call void @llvm.dbg.value(metadata i32 %4, metadata !35, metadata !DIExpression()), !dbg !33
call void @llvm.dbg.value(metadata i32 %4, metadata !35, metadata !DIExpression()), !dbg !33

---

We have now updated LdStHasDebugValue to include the cases when instruction is already

followed by same dbg.value instruction we intend to insert.

Now,

• • Definition and usage of function LdStHasDebugValue are deleted.
  • RemoveRedundantDbgInstrs is called for the cleanup of duplicate dbg.value's
    
    Testing:
• Added unit test for validation
• check-llvm
• check-debuginfo (the debug info integration tests)

[dstenb]

llvm/lib/Transforms/Utils/Local.cpp

1234

Nit: check -> Check, and a dot at the end of the instruction.

1239–1240

Can we move this into a isDbgValueWithValue(DVI, I, DIVar, DIExpr) helper or similar, and also use that for the "before I" check?

1241

I wonder if we also need to check that the locations' getInlinedAt() are identical. Otherwise, I think this may result in us not emitting a dbg.value for a different inlined instance of the variable. I'm not sure if we can land in such a case for a load or store in practice though.

(Same goes for the "before I" check.)

llvm/test/DebugInfo/duplicate_dbgvalue.ll

2

Unless I'm overlooking something, I think that this should be possible to make a single-pass test, which is preferable since it makes the test more stable, and makes the changes easier to review.

A suitable single-pass test case can often be found by looking at the opt output with -print-before-all -print-after-all -print-module-scope added.

[dstenb]

llvm/lib/Transforms/Utils/Local.cpp

1234

s/instruction/comment/

[alok]

llvm/lib/Transforms/Utils/Local.cpp

1234

Thanks a lot for your comment, i shall update patch for this.

1239–1240

Sure, I shall do that in next revision.

1241

Thanks for your comment, I shall include that in next revision.

llvm/test/DebugInfo/duplicate_dbgvalue.ll

2

Thanks for your comment. I shall incorporate this in next revision.

[alok]

Updated to incorporate comments from @dstenb

[alok]

Hi David (@dstenb), I have incorporated all your comments. I have also added you as reviewer. Kindly give your feedback.

[dstenb]

In D74030#1868655, @alok wrote:

Hi David (@dstenb), I have incorporated all your comments. I have also added you as reviewer. Kindly give your feedback.

Hi, sorry for the delay! I have some more minor comments.

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

[...] matches the given operands.

(Or something along those lines).

1234

isDbgValueMatches reads a bit awkward to me. Perhaps isDbgValueMatching?

1234–1235

Can these pointer variables be made const?

1242

To make this function more general, perhaps we should make it take a Value instead of an Instruction, and specify I or I->getOperand(0) at the call sites?

llvm/test/DebugInfo/duplicate_dbgvalue.ll

2

Thanks!

[alok]

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

Thanks. It will be updated in next version.

1234

Thanks a lot. It shall be updated in next version.

1234–1235

Sure. It shall be updated in next version. Thanks for input.

1242

It shall be updated in next version. Thanks for input.

[alok]

This version is updated to incorporate comments from David (@dstenb ).

[dstenb]

Thanks! No further comments from me.

I'll leave a chance for others that are more familiar with this code to have their say though.

[aprantl]

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

Is checking the first instruction after going to be good enough in the general case? Optimized code often has dozens of dbg.values in a row, all pointing to the same SSA value. (For example C++ code will often have many inlines "this" variables from inlined trivial getters).

[alok]

In D74030#1869863, @dstenb wrote:

Thanks! No further comments from me.

I'll leave a chance for others that are more familiar with this code to have their say though.

Thanks a lot David. Your comments really added a lot of value to fix.

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

Thanks for pointing this out. Even I though this and ignored. Yes it will be better to iterate through next instructions. I shall be updating patch for that.

[alok]

Updated to incorporate review comments from Adrian (@aprantl) .

[jmorse]

This generally sounds fine to me, I have some high level comments: if this functionality is likely to be repeated elsewhere, it might be better to use the DebugVariable class in DebugInfoMetadata.h. It'd be more concise to compare variable identities by equality of DebugVariable objects rather than by individual components. (Not necessary for this patch, but worth keeping in mind if there are similar upcoming patches).

Repeatedly scanning through all dbg.values after an instruction could be expensive when there are a lot of them, for example in asan builds, which has bitten me in the past. I'd recommend checking the performance impact of a small asan build; it might also be that in the specific circumstance that LdStHasDebugValue targets, it's sufficient for a shallow test of a single dbg.value.

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

Shouldn't this loop terminate when a non-debug-value instruction is seen? Otherwise it's not checking for the presence of a dbg.value for a single load/store, but instead a whole block.

[Orlando]

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

@aprantl said:

Is checking the first instruction after going to be good enough in the general case?

In this case shouldn't the 'already exists before I' check also be a backwards scan (through consecutive dbg.values)?

[aprantl]

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

Yeah, and doing a linear scan each time might be prohibitively expensive (i.e., quadratic). If we're implementing this we should take a look at the performance of compiling an optimized clang and an optimizes asanified clang to make sure this isn't a problem.

[vsk]

I'm concerned about introducing quadratic compile-time behavior here. Is the problem here that there are too many duplicate dbg.values after LowerDbgDeclare? Have you considered invoking the RemoveRedundantDbgInstrs routine from D71478 after LowerDbgDeclare as an alternative?

[vsk]

@alok I had a quick stab at this to see what it would take, and the result turned out to be relatively clean: https://github.com/vedantk/llvm-project/commit/85d7fe06810fa7f6166508be73eef80f676b0fc6.diff. This handles the situation in your 'duplicate_dbgvalue.ll' test case. Lmk what you think. Feel free to commandeer the patch if you'd like, or if you don't have the bandwidth for that I can kick off a fresh review. Thanks!

[alok]

In D74030#1888527, @vsk wrote:

@alok I had a quick stab at this to see what it would take, and the result turned out to be relatively clean: https://github.com/vedantk/llvm-project/commit/85d7fe06810fa7f6166508be73eef80f676b0fc6.diff. This handles the situation in your 'duplicate_dbgvalue.ll' test case. Lmk what you think. Feel free to commandeer the patch if you'd like, or if you don't have the bandwidth for that I can kick off a fresh review. Thanks!

Thanks for your comment/suggestion. I would update the patch to call RemoveRedundantDbgInstrs as you suggested. I would also limit the check before inserting dbg.value instruction to immediate next instruction in place of linear forward search considering the performance impact. This will enable us to avoid inserting consecutive duplicates, and remove duplicates if are far apart. Thanks again for your suggestion.

llvm/lib/Transforms/Utils/Local.cpp

1233–1234

Thanks for your comment. Yes I too feel that linear scan doesnt look like a good idea and it should be limited to check intimidate previous/next instruction that is where it has the greatest probability to exist.

1233–1234

Thanks for your comment. Looking at the possible performance impact, I am going to limit the scan to the immediate next instruction.

[alok]

This is updated to re-base and addressing common concern about performance and suggestion form @vsk to use RemoveRedundantDbgInstrs.

Now,

1. Avoid insertion of duplicate dbg.value if immediate next instruction is identical dbg.value. (avoids generating duplicates for same load/store, forward scan is not done)
2. Delete duplicate dbg.value in case duplicate dbg.value are present (may be inserted for different load/store) by calling RemoveRedundantDbgInstrs.

[vsk]

In D74030#1888655, @alok wrote:

Now,

1. Avoid insertion of duplicate dbg.value if immediate next instruction is identical dbg.value. (avoids generating duplicates for same load/store, forward scan is not done)

Why is it necessary to do (1)? If the goal is to have cleaner IR after LowerDbgDeclare, it seems like (2) achieves that.

2. Delete duplicate dbg.value in case duplicate dbg.value are present (may be inserted for different load/store) by calling RemoveRedundantDbgInstrs.

[alok]

In D74030#1889750, @vsk wrote:

In D74030#1888655, @alok wrote:

Now,

1. Avoid insertion of duplicate dbg.value if immediate next instruction is identical dbg.value. (avoids generating duplicates for same load/store, forward scan is not done)

Why is it necessary to do (1)? If the goal is to have cleaner IR after LowerDbgDeclare, it seems like (2) achieves that.

Yes for me also it seems a bit difficult decision. There were few reasons which made me think to keep 1)
a) It looks clearer to avoid generating than first let it generate and later delete when there is no performance penalty. In case of avoiding generation of dbg.value for same load/store again and again there is a pattern (immediate previous/next instruction in question).
b) There is existing functionality for avoiding generation in function named "LdStHasDebugValue", though the name suggests covering both load and store both but it had handling only for store, adding the case for load justifies its name and completes the functionality. Either we should remove this function completely or complete it.

2. Delete duplicate dbg.value in case duplicate dbg.value are present (may be inserted for different load/store) by calling RemoveRedundantDbgInstrs.

[vsk]

In D74030#1891319, @alok wrote:

In D74030#1889750, @vsk wrote:

In D74030#1888655, @alok wrote:

Now,

1. Avoid insertion of duplicate dbg.value if immediate next instruction is identical dbg.value. (avoids generating duplicates for same load/store, forward scan is not done)

Why is it necessary to do (1)? If the goal is to have cleaner IR after LowerDbgDeclare, it seems like (2) achieves that.

Yes for me also it seems a bit difficult decision. There were few reasons which made me think to keep 1)
a) It looks clearer to avoid generating than first let it generate and later delete when there is no performance penalty. In case of avoiding generation of dbg.value for same load/store again and again there is a pattern (immediate previous/next instruction in question).

I'm not sure I understand how the IR could look clearer. It shouldn't make any difference if the redundant instructions are eliminated after all insertions occur. Unless there's some kind of significant compile time reduction to be had by keeping LdStHasDebugValue, I think we should get rid of it, as it'd just be adding unnecessary complexity.

b) There is existing functionality for avoiding generation in function named "LdStHasDebugValue", though the name suggests covering both load and store both but it had handling only for store, adding the case for load justifies its name and completes the functionality. Either we should remove this function completely or complete it.

Agreed! Strong +1 to this, it's just that I suspect deleting LdStHasDebugValue is the simpler way to go.

[alok]

In D74030#1892727, @vsk wrote:

In D74030#1891319, @alok wrote:

In D74030#1889750, @vsk wrote:

In D74030#1888655, @alok wrote:

Now,

1. Avoid insertion of duplicate dbg.value if immediate next instruction is identical dbg.value. (avoids generating duplicates for same load/store, forward scan is not done)

Why is it necessary to do (1)? If the goal is to have cleaner IR after LowerDbgDeclare, it seems like (2) achieves that.

Yes for me also it seems a bit difficult decision. There were few reasons which made me think to keep 1)
a) It looks clearer to avoid generating than first let it generate and later delete when there is no performance penalty. In case of avoiding generation of dbg.value for same load/store again and again there is a pattern (immediate previous/next instruction in question).

I'm not sure I understand how the IR could look clearer. It shouldn't make any difference if the redundant instructions are eliminated after all insertions occur. Unless there's some kind of significant compile time reduction to be had by keeping LdStHasDebugValue, I think we should get rid of it, as it'd just be adding unnecessary complexity.

b) There is existing functionality for avoiding generation in function named "LdStHasDebugValue", though the name suggests covering both load and store both but it had handling only for store, adding the case for load justifies its name and completes the functionality. Either we should remove this function completely or complete it.

Agreed! Strong +1 to this, it's just that I suspect deleting LdStHasDebugValue is the simpler way to go.

I shall delete LdStHasDebugValue in updated patch.

[alok]

Updated for the comments from @vsk . Deleted definition and usage of function LdStHasDebugValue.

[vsk]

LGTM, thanks!

llvm/lib/Transforms/Utils/Local.cpp

 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/KnownBits.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 #include <algorithm>
 #include <cassert>
 #include <climits>
 #include <cstdint>
 #include <iterator>
 #include <map>
 #include <utility>
unsigned llvm::getOrEnforceKnownAlignment(Value *V, unsigned PrefAlign,
 
   // We don't need to make any adjustment.
   return Align;
 }
 
 ///===---------------------------------------------------------------------===//
 ///  Dbg Intrinsic utilities
 ///
 
-/// See if there is a dbg.value intrinsic for DIVar before I.
-static bool LdStHasDebugValue(DILocalVariable *DIVar, DIExpression *DIExpr,
-                              Instruction *I) {
-  // Since we can't guarantee that the original dbg.declare instrinsic
-  // is removed by LowerDbgDeclare(), we need to make sure that we are
-  // not inserting the same dbg.value intrinsic over and over.
-  BasicBlock::InstListType::iterator PrevI(I);
-  if (PrevI != I->getParent()->getInstList().begin()) {
-    --PrevI;
-    if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(PrevI))
-      if (DVI->getValue() == I->getOperand(0) &&
-          DVI->getVariable() == DIVar &&
-          DVI->getExpression() == DIExpr)
-        return true;
-  }
-  return false;
-}
-
 /// See if there is a dbg.value intrinsic for DIVar for the PHI node.
 static bool PhiHasDebugValue(DILocalVariable *DIVar,
                              DIExpression *DIExpr,
                              PHINode *APN) {
   // Since we can't guarantee that the original dbg.declare instrinsic
   // is removed by LowerDbgDeclare(), we need to make sure that we are
   // not inserting the same dbg.value intrinsic over and over.
   SmallVector<DbgValueInst *, 1> DbgValues;
   findDbgValues(DbgValues, APN);
   for (auto *DVI : DbgValues) {
     assert(DVI->getValue() == APN);
     if ((DVI->getVariable() == DIVar) && (DVI->getExpression() == DIExpr))
       return true;
   }
   return false;
 }
 
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());
-    if (!LdStHasDebugValue(DIVar, DIExpr, SI))
-      Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
     return;
   }
 
-  if (!LdStHasDebugValue(DIVar, DIExpr, SI))
-    Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
 }
 
 /// 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 llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
                                            LoadInst *LI, DIBuilder &Builder) {
   auto *DIVar = DII->getVariable();
   auto *DIExpr = DII->getExpression();
   assert(DIVar && "Missing variable");
 
-  if (LdStHasDebugValue(DIVar, DIExpr, LI))
-    return;
-
   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');
     return;
   }

 /// 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 false;
+    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
while (!WorkList.empty()) {
           }
         } else if (BitCastInst *BI = dyn_cast<BitCastInst>(U)) {
           if (BI->getType()->isPointerTy())
             WorkList.push_back(BI);
         }
       }
     }
     DDI->eraseFromParent();
+    Changed = true;
   }
-  return 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.");
   if (InsertedPHIs.size() == 0)
     return;

llvm/test/DebugInfo/duplicate_dbgvalue.ll

+; RUN: opt -instcombine -S -o - < %s | FileCheck %s
+
+; CHECK-LABEL: %4 = load i32, i32* %i1_311
+; CHECK: call void @llvm.dbg.value(metadata i32 %4
+; Next instruction should not be duplicate dbg.value intrinsic.
+; CHECK-NEXT: @f90io_sc_i_ldw
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;program main
+;integer :: res
+; res = mfun()
+; print *, res
+;contains
+; function mfun()
+;  integer :: i1
+;  i1 = 5
+;  mfun = fun(i1)
+;  write (*,*) i1
+; end function
+; function fun(a)
+;    integer, intent (in) :: a
+;    fun = a
+; end function
+;end program main
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+; ModuleID = 'duplicate_dbgvalue.ll'
+source_filename = "duplicate_dbgvalue.ll"
+target datalayout = "e-p:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.BSS1 = type <{ [4 x i8] }>
+
+@.BSS1 = internal unnamed_addr global %struct.BSS1 zeroinitializer, align 32, !dbg !0
+@.C303_MAIN_ = internal constant i32 6
+@.C300_MAIN_ = internal constant [22 x i8] c"duplicate_dbgvalue.f90"
+@.C302_MAIN_ = internal constant i32 4
+@.C283_MAIN_ = internal constant i32 0
+@.C283_main_mfun = internal constant i32 0
+@.C302_main_mfun = internal constant i32 6
+@.C300_main_mfun = internal constant [22 x i8] c"duplicate_dbgvalue.f90"
+@.C313_main_mfun = internal constant i32 10
+
+define void @MAIN_() local_unnamed_addr !dbg !2 {
+L.entry:
+  call void (i8*, ...) bitcast (void (...)* @fort_init to void (i8*, ...)*)(i8* bitcast (i32* @.C283_MAIN_ to i8*)), !dbg !16
+  %0 = call fastcc i32 @main_mfun(), !dbg !18
+  store i32 %0, i32* bitcast (%struct.BSS1* @.BSS1 to i32*), align 32, !dbg !18
+  call void (i8*, i8*, i64, ...) bitcast (void (...)* @f90io_src_info03a to void (i8*, i8*, i64, ...)*)(i8* bitcast (i32* @.C302_MAIN_ to i8*), i8* getelementptr inbounds ([22 x i8], [22 x i8]* @.C300_MAIN_, i64 0, i64 0), i64 22), !dbg !23
+  %1 = call i32 (i8*, i8*, i8*, i8*, ...) bitcast (i32 (...)* @f90io_print_init to i32 (i8*, i8*, i8*, i8*, ...)*)(i8* bitcast (i32* @.C303_MAIN_ to i8*), i8* null, i8* bitcast (i32* @.C283_MAIN_ to i8*), i8* bitcast (i32* @.C283_MAIN_ to i8*)), !dbg !23
+  call void @llvm.dbg.value(metadata i32 %1, metadata !24, metadata !DIExpression()), !dbg !25
+  %2 = load i32, i32* bitcast (%struct.BSS1* @.BSS1 to i32*), align 32, !dbg !23
+  %3 = call i32 (i32, i32, ...) bitcast (i32 (...)* @f90io_sc_i_ldw to i32 (i32, i32, ...)*)(i32 %2, i32 25), !dbg !23
+  call void @llvm.dbg.value(metadata i32 %3, metadata !24, metadata !DIExpression()), !dbg !25
+  %4 = call i32 (...) @f90io_ldw_end(), !dbg !23
+  call void @llvm.dbg.value(metadata i32 %4, metadata !24, metadata !DIExpression()), !dbg !25
+  ret void, !dbg !26
+}
+
+define internal fastcc signext i32 @main_mfun() unnamed_addr !dbg !27 {
+L.entry:
+  %i1_311 = alloca i32, align 4
+  call void @llvm.dbg.declare(metadata i64* undef, metadata !31, metadata !DIExpression()), !dbg !33
+  call void @llvm.dbg.declare(metadata i32* %i1_311, metadata !35, metadata !DIExpression()), !dbg !33
+  store i32 5, i32* %i1_311, align 4, !dbg !36
+  %0 = bitcast i32* %i1_311 to i64*, !dbg !41
+  %1 = call fastcc float @main_fun(i64* %0), !dbg !41
+  %2 = fptosi float %1 to i32, !dbg !41
+  call void (i8*, i8*, i64, ...) bitcast (void (...)* @f90io_src_info03a to void (i8*, i8*, i64, ...)*)(i8* bitcast (i32* @.C313_main_mfun to i8*), i8* getelementptr inbounds ([22 x i8], [22 x i8]* @.C300_main_mfun, i32 0, i32 0), i64 22), !dbg !42
+  %3 = call i32 (i8*, i8*, i8*, i8*, ...) bitcast (i32 (...)* @f90io_print_init to i32 (i8*, i8*, i8*, i8*, ...)*)(i8* bitcast (i32* @.C302_main_mfun to i8*), i8* null, i8* bitcast (i32* @.C283_main_mfun to i8*), i8* bitcast (i32* @.C283_main_mfun to i8*)), !dbg !42
+  call void @llvm.dbg.value(metadata i32 %3, metadata !43, metadata !DIExpression()), !dbg !33
+  %4 = load i32, i32* %i1_311, align 4, !dbg !42
+  call void @llvm.dbg.value(metadata i32 %4, metadata !35, metadata !DIExpression()), !dbg !33
+  %5 = call i32 (i32, i32, ...) bitcast (i32 (...)* @f90io_sc_i_ldw to i32 (i32, i32, ...)*)(i32 %4, i32 25), !dbg !42
+  call void @llvm.dbg.value(metadata i32 %5, metadata !43, metadata !DIExpression()), !dbg !33
+  %6 = call i32 (...) @f90io_ldw_end(), !dbg !42
+  call void @llvm.dbg.value(metadata i32 %6, metadata !43, metadata !DIExpression()), !dbg !33
+  ret i32 %2, !dbg !44
+}
+
+define internal fastcc float @main_fun(i64* noalias %a) unnamed_addr !dbg !45 {
+L.entry:
+  call void @llvm.dbg.declare(metadata i64* %a, metadata !50, metadata !DIExpression()), !dbg !51
+  call void @llvm.dbg.declare(metadata i64* undef, metadata !53, metadata !DIExpression()), !dbg !51
+  %0 = bitcast i64* %a to i32*, !dbg !54
+  %1 = load i32, i32* %0, align 4, !dbg !54
+  %2 = sitofp i32 %1 to float, !dbg !54
+  ret float %2, !dbg !59
+}
+
+declare signext i32 @f90io_ldw_end(...) local_unnamed_addr
+
+declare signext i32 @f90io_sc_i_ldw(...) local_unnamed_addr
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+declare signext i32 @f90io_print_init(...) local_unnamed_addr
+
+declare void @f90io_src_info03a(...) local_unnamed_addr
+
+declare void @fort_init(...) local_unnamed_addr
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.module.flags = !{!14, !15}
+!llvm.dbg.cu = !{!4}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "res", scope: !2, file: !3, type: !9, isLocal: true, isDefinition: true)
+!2 = distinct !DISubprogram(name: "main", scope: !4, file: !3, line: 1, type: !12, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagMainSubprogram, unit: !4)
+!3 = !DIFile(filename: "duplicate-dbgvalue.f90", directory: "/dir")
+!4 = distinct !DICompileUnit(language: DW_LANG_Fortran90, file: !3, producer: " F90 Flang - 1.5", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+!6 = !{!0, !7, !10}
+!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+!8 = distinct !DIGlobalVariable(name: "res", scope: !4, file: !3, type: !9, isLocal: true, isDefinition: true)
+!9 = !DIBasicType(name: "integer", size: 32, align: 32, encoding: DW_ATE_signed)
+!10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
+!11 = distinct !DIGlobalVariable(name: "res", scope: !4, file: !3, type: !9, isLocal: true, isDefinition: true)
+!12 = !DISubroutineType(cc: DW_CC_program, types: !13)
+!13 = !{null}
+!14 = !{i32 2, !"Dwarf Version", i32 4}
+!15 = !{i32 2, !"Debug Info Version", i32 3}
+!16 = !DILocation(line: 1, column: 1, scope: !17)
+!17 = !DILexicalBlock(scope: !2, file: !3, line: 1, column: 1)
+!18 = !DILocation(line: 3, column: 1, scope: !17)
+!19 = !{!20, !20, i64 0}
+!20 = !{!"t1.2", !21, i64 0}
+!21 = !{!"unlimited ptr", !22, i64 0}
+!22 = !{!"Flang FAA 1"}
+!23 = !DILocation(line: 4, column: 1, scope: !17)
+!24 = !DILocalVariable(scope: !17, file: !3, type: !9, flags: DIFlagArtificial)
+!25 = !DILocation(line: 0, scope: !17)
+!26 = !DILocation(line: 5, column: 1, scope: !17)
+!27 = distinct !DISubprogram(name: "mfun", scope: !2, file: !3, line: 6, type: !28, scopeLine: 6, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !4)
+!28 = !DISubroutineType(types: !29)
+!29 = !{!30}
+!30 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, align: 64)
+!31 = !DILocalVariable(arg: 1, scope: !27, file: !3, type: !32, flags: DIFlagArtificial)
+!32 = !DIBasicType(name: "uinteger*8", size: 64, align: 64, encoding: DW_ATE_unsigned)
+!33 = !DILocation(line: 0, scope: !34)
+!34 = !DILexicalBlock(scope: !27, file: !3, line: 6, column: 1)
+!35 = !DILocalVariable(name: "i1", scope: !34, file: !3, type: !9)
+!36 = !DILocation(line: 8, column: 1, scope: !34)
+!37 = !{!38, !38, i64 0}
+!38 = !{!"t2.2", !39, i64 0}
+!39 = !{!"unlimited ptr", !40, i64 0}
+!40 = !{!"Flang FAA 2"}
+!41 = !DILocation(line: 9, column: 1, scope: !34)
+!42 = !DILocation(line: 10, column: 1, scope: !34)
+!43 = !DILocalVariable(scope: !34, file: !3, type: !9, flags: DIFlagArtificial)
+!44 = !DILocation(line: 11, column: 1, scope: !34)
+!45 = distinct !DISubprogram(name: "fun", scope: !2, file: !3, line: 12, type: !46, scopeLine: 12, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !4)
+!46 = !DISubroutineType(types: !47)
+!47 = !{!48, !9}
+!48 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !49, size: 64, align: 64)
+!49 = !DIBasicType(name: "real", size: 32, align: 32, encoding: DW_ATE_float)
+!50 = !DILocalVariable(name: "a", arg: 1, scope: !45, file: !3, type: !9)
+!51 = !DILocation(line: 0, scope: !52)
+!52 = !DILexicalBlock(scope: !45, file: !3, line: 12, column: 1)
+!53 = !DILocalVariable(arg: 2, scope: !45, file: !3, type: !32, flags: DIFlagArtificial)
+!54 = !DILocation(line: 14, column: 1, scope: !52)
+!55 = !{!56, !56, i64 0}
+!56 = !{!"t3.2", !57, i64 0}
+!57 = !{!"unlimited ptr", !58, i64 0}
+!58 = !{!"Flang FAA 3"}
+!59 = !DILocation(line: 15, column: 1, scope: !52)

llvm/test/Transforms/Util/dbg-call-bitcast.ll

 ; RUN: opt -instcombine -S %s | FileCheck %s
 
 define dso_local void @_Z1fv() {
   %1 = alloca i32, align 4
   %2 = bitcast i32* %1 to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %2)
   call void @llvm.dbg.declare(metadata i32* %1, metadata !16, metadata !DIExpression()), !dbg !19
 ; CHECK: %[[A:.*]] = alloca i32, align 4
 ; CHECK: call void @llvm.dbg.value(metadata i32* %[[A]], {{.*}}, metadata !DIExpression(DW_OP_deref)
 ; CHECK: call void @_Z1gPv
   call void @_Z1gPv(i8* nonnull %2)
   %3 = bitcast i32* %1 to i8*
+; CHECK-NOT: call void @llvm.dbg.value
+; CHECK: call void @_Z1gPv
+  call void @_Z1gPv(i8* nonnull %3)
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %2)
+  ret void, !dbg !21
+}
+
+define dso_local void @_Z2fv() {
+  %1 = alloca i32, align 4
+  %2 = bitcast i32* %1 to i8*
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %2)
+  call void @llvm.dbg.declare(metadata i32* %1, metadata !16, metadata !DIExpression()), !dbg !19
+; CHECK: %[[A:.*]] = alloca i32, align 4
+; CHECK: call void @llvm.dbg.value(metadata i32* %[[A]], {{.*}}, metadata !DIExpression(DW_OP_deref)
+; CHECK: call void @_Z1gPv
+  call void @_Z1gPv(i8* nonnull %2)
+  br label %block2
+
+block2:
+  %3 = bitcast i32* %1 to i8*
 ; CHECK: call void @llvm.dbg.value(metadata i32* %[[A]], {{.*}}, metadata !DIExpression(DW_OP_deref)
 ; CHECK: call void @_Z1gPv
   call void @_Z1gPv(i8* nonnull %3)
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %2)
   ret void, !dbg !21
 }
 
 declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)