[DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare

[jmorse]

Summary

This patch changes the line-numbers given to dbg.value intrinsics when an alloca/dbg.declare is promoted to an SSA register. Currently we use the original line number of the dbg.declare, i.e. the variable declaration. I believe there is risk that well-meaning optimisations will accidentally acquire these line numbers from dbg.values, artificially introducing line-number changes that needlessly step back to the variable declaration. I wasn't able to immediately produce an input that demonstrated this, below is an explanation of the patch, followed by some evidence to demonstrate this is a desirable change.

As far as I understand it, the actual line number that a dbg.value gets is meaningless (although the scope / inlining information is important). The issue is that the line number could leak into surrounding instructions during optimisation.

~

What I've changed: In the three ConvertDebugDeclareToDebugValue functions changed, previously we assigned each created dbg.value intrinsic the DebugLoc from the dbg.declare it originates from. Now, if the store or PHI node that the dbg.value corresponds to has a line number in the same scope, the 'getDebugValueLoc' helper picks that, otherwise an unknown-location line number is used.

The added test tests three scenarios (explained in the file) of no-location, suitable location, and inlined location.

The formal_parameter.ll test that changes, is checking that the dbg.values for a lowered dbg.declare have the right scope. They now get different DILocations, so I've added extra CHECK lines for their scope fields.

~

To try and get some perspective on whether this is a good change to make, I built clang-3.4 with and without this patch and diff'd the line tables. Roughly 900 entries were altered, in some whole lines went away, in others they were replaced by unknown-locations.

Example 1: in "llvm::GPRGetRawAllocationOrder(llvm::MachineFunction const&)", some tablegen'd function for the ARM backend, this materializeFrameBaseRegister function [0] is inlined. Without the patch, the line table progression is:

Line 577 (conditional test)
Constructor for a DebugLoc
Line 578 (conditional assignment to DL)
This [1] variable declaration in BuildMI
MachineBasicBlock::getParent
MachineFunction::getTarget
MachineFunction::getRegInfo
Line 582

Most of that makes sense for lines 577 to 582 of the function [0], except for the early step into BuildMI [1], which seems to be part of the BuildMI call on line 586. That particular linetable entry disappears with this patch. It's unclear to me how it got there in the first place, but I don't believe it's helpful.

Example 2, in "llvm::ARMFrameLowering::hasFP", this [2] inlined call to emitRegPlusImmediate steps through the evaluation of the arguments, then to the declaration of one of the formal parameters in the target function [3], then back to evaluating the arguments again. This extra spurious step into the destination function isn't helpful IMO.

While I don't know where exactly instructions pick up line numbers from dbg.values, I believe these differences demonstrate that they do, and so it should be avoided.

~

tl;dr, if we don't spread the line numbers of variable declarations throughout instructions in the function, we'll have less jumpy line tables.

Credit to @CarlosAlbertoEnciso who found this originally but didn't get around to filing a patch.

[0] https://github.com/llvm-mirror/llvm/blob/release_34/lib/Target/ARM/ARMBaseRegisterInfo.cpp#L577
[1] https://github.com/llvm-mirror/llvm/blob/release_34/include/llvm/CodeGen/MachineInstrBuilder.h#L245
[2] https://github.com/llvm-mirror/llvm/blob/release_34/lib/Target/ARM/ARMFrameLowering.cpp#L285
[3] https://github.com/llvm-mirror/llvm/blob/release_34/lib/Target/ARM/ARMFrameLowering.cpp#L108

[rnk]

This makes me nervous. The pair of the DIVariable and DILocation is used to uniquely identify the variable that a dbg.value applies to. In practice, inlining is usually what creates multiple instances of the same variable that get updated by dbg.values. You can see how the location is used in DwarfDebug::collectVariableInfoFromMFTable.

I see other places in the code that seem to assume that the scope of the location of a dbg.value will get the scope in which the variable is declared. Your change breaks that invariant. I think we could update such code to get the scope from the DIVariable, but maybe that would be wrong since it would lack inlining information.

[aprantl]

In D59272#1426904, @rnk wrote:

This makes me nervous. The pair of the DIVariable and DILocation is used to uniquely identify the variable that a dbg.value applies to. In practice, inlining is usually what creates multiple instances of the same variable that get updated by dbg.values. You can see how the location is used in DwarfDebug::collectVariableInfoFromMFTable.

Last time I checked, the file, line number and column of a dbg.value was completely unused. To uniquely identify and inlined instance of a variable you need to the combination of inlinedAt field of the dbg.value's DILocation and the dbg.value's DILocalVariable.

I see other places in the code that seem to assume that the scope of the location of a dbg.value will get the scope in which the variable is declared. Your change breaks that invariant. I think we could update such code to get the scope from the DIVariable, but maybe that would be wrong since it would lack inlining information.

Isn't the scope coming from the DILocalVariable though?

[dblaikie]

In D59272#1426970, @aprantl wrote:

In D59272#1426904, @rnk wrote:

This makes me nervous. The pair of the DIVariable and DILocation is used to uniquely identify the variable that a dbg.value applies to. In practice, inlining is usually what creates multiple instances of the same variable that get updated by dbg.values. You can see how the location is used in DwarfDebug::collectVariableInfoFromMFTable.

Last time I checked, the file, line number and column of a dbg.value was completely unused. To uniquely identify and inlined instance of a variable you need to the combination of inlinedAt field of the dbg.value's DILocation and the dbg.value's DILocalVariable.

I see other places in the code that seem to assume that the scope of the location of a dbg.value will get the scope in which the variable is declared. Your change breaks that invariant. I think we could update such code to get the scope from the DIVariable, but maybe that would be wrong since it would lack inlining information.

Isn't the scope coming from the DILocalVariable though?

The scope, perhaps/probably - but as you say, not the "inlinedAt" part. So, yeah, picking the wrong DebugLoc for dbg.value instruction could still drop/break/mess up inlining info due to the inlinedAt part of the DebugLoc.

[rnk]

@aprantl, I think you are correct for all the code that I can find today. The only thing that matters is the inlinedAt part of the location. But I did see this code that handles dbg.declare, which uses the scope from the location:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L1042

I guess my issue is that using the insertion point just to get "smooth" line numbers seems fragile. If the insertion point isn't a store, I think we'd be better off using an unknown location (line 0). Using the location of the store, assuming it has the right inlinedAt info, as this does, seems good. That just leaves us to ask what should happen at the other dbg.value creation places, call sites, loads, etc.

[jmorse]

Update to use unknown-locations for everything but promoted store instructions,

rnk wrote:

@aprantl, I think you are correct for all the code that I can find today. The only thing that matters is the inlinedAt part of the location. But I did see this code that handles dbg.declare, which uses the scope from the location:

So that I understand correctly: the concern was that dbg.values with different DILocations could be seen as different variable instances (bad), but actually only the inlinedAt field is used for that (which is co-incidentally is also a DILocation).

For the avoidance of doubt, the code in getDebugValueLoc always uses the scope and inlinedAt field from the dbg.declare.

I guess my issue is that using the insertion point just to get "smooth" line numbers seems fragile. If the insertion point isn't a store, I think we'd be better off using an unknown location (line 0). Using the location of the store, assuming it has the right inlinedAt info, as this does, seems good. That just leaves us to ask what should happen at the other dbg.value creation places, call sites, loads, etc.

This update now uses unknown-location for everything except stores (and covers call-sites which I missed before). Using unknown-location for everything would be perfectly fine too IMO, I'm not wed to either idea.

[rnk]

Thanks! I haven't read the code in detail, but this all sounds good to me. (/me goes back to cave).

[bjope]

Isn't there a risk that this (partially) is hiding bugs in some other passes, where the DebugLoc is picked from the wrong place. I guess that either it should be picked from an adjacent non-meta instruction. Or it should be set to unknown.
With this patch we only eliminate the risk that the faulty passes are picking an "incorrect" DebugLoc from a dbg.value intrinsic (or DBG_VALUE instruction). But the "faulty" pass could still pick incorrect DebugLoc from other instructions, right? Or is it always correct to take the DebugLoc from all other non-dbg.value instructions, including all other kinds of meta-instructions?

Might be hard to find all such bugs. But with your known examples it shouldn't be too hard to find at least some places in opt/llc where the DebugLoc from the dbg.value is infecting some other instruction. Then perhaps we want to land this patch as well, just do avoid some not-yet-detected problems.

[jmorse]

Hi,

In D59272#1439778, @bjope wrote:

Isn't there a risk that this (partially) is hiding bugs in some other passes, where the DebugLoc is picked from the wrong place. I guess that either it should be picked from an adjacent non-meta instruction. Or it should be set to unknown.
With this patch we only eliminate the risk that the faulty passes are picking an "incorrect" DebugLoc from a dbg.value intrinsic (or DBG_VALUE instruction). But the "faulty" pass could still pick incorrect DebugLoc from other instructions, right? Or is it always correct to take the DebugLoc from all other non-dbg.value instructions, including all other kinds of meta-instructions?

Might be hard to find all such bugs. But with your known examples it shouldn't be too hard to find at least some places in opt/llc where the DebugLoc from the dbg.value is infecting some other instruction. Then perhaps we want to land this patch as well, just do avoid some not-yet-detected problems.

All true -- this patch can be characterised as simply reducing the risk of leaking definitely-wrong line numbers into non-meta IR instructions, which I think has some value.

Quickly testing what happens if IRBuilder is adjusted to not pick take DebugLocs from debug-intrinsics [0], roughly 750 of the 900 entries this patch changes, are also changed, possibly to different lines though. (I'll upload this trivial change sometime soon too).

That's likely the bulk of the leaking line numbers; while the rest could be hunted down, they're likely to be scattered through many places, so IMO this patch is the most time-efficient solution going forwards.

[0] https://github.com/llvm-mirror/llvm/blob/e3696113b639c8bf0b72d6c27dd76d6fdd8ebf61/include/llvm/IR/IRBuilder.h#L137

[bjope]

In D59272#1441475, @jmorse wrote:

Hi,

In D59272#1439778, @bjope wrote:

Isn't there a risk that this (partially) is hiding bugs in some other passes, where the DebugLoc is picked from the wrong place. I guess that either it should be picked from an adjacent non-meta instruction. Or it should be set to unknown.
With this patch we only eliminate the risk that the faulty passes are picking an "incorrect" DebugLoc from a dbg.value intrinsic (or DBG_VALUE instruction). But the "faulty" pass could still pick incorrect DebugLoc from other instructions, right? Or is it always correct to take the DebugLoc from all other non-dbg.value instructions, including all other kinds of meta-instructions?

Might be hard to find all such bugs. But with your known examples it shouldn't be too hard to find at least some places in opt/llc where the DebugLoc from the dbg.value is infecting some other instruction. Then perhaps we want to land this patch as well, just do avoid some not-yet-detected problems.

All true -- this patch can be characterised as simply reducing the risk of leaking definitely-wrong line numbers into non-meta IR instructions, which I think has some value.

Quickly testing what happens if IRBuilder is adjusted to not pick take DebugLocs from debug-intrinsics [0], roughly 750 of the 900 entries this patch changes, are also changed, possibly to different lines though. (I'll upload this trivial change sometime soon too).

That's likely the bulk of the leaking line numbers; while the rest could be hunted down, they're likely to be scattered through many places, so IMO this patch is the most time-efficient solution going forwards.

[0] https://github.com/llvm-mirror/llvm/blob/e3696113b639c8bf0b72d6c27dd76d6fdd8ebf61/include/llvm/IR/IRBuilder.h#L137

Ok, great. I'm happy with that explanation.

[jmorse]

Ping; I've uploaded D61198 which likely eliminates the lions share of DebugLocs-leaking-from-DbgIntrinsics. This patch would still be required however to reduce the damage from other scenarios where DebugLocs leak from debug intrinsics.

[aprantl]

lib/Transforms/Utils/Local.cpp

1287

Should we add a comment that everything except the inlinedAt is basically unused and that we are only putting it there as a safety guard for other transformations?

[jmorse]

Add comment explaining the choice of zero line numbers in getDebugValueLoc. After discussion in D61198, I've removed the clause preserving the line numbers of store instructions -- It's probably better to make it clear that the line number information in debug intrinsics is completely meaningless, and thus not try to preserve it at all. The selection of a zero line number is thus just a defence against leaky optimisations.

[aprantl]

lib/Transforms/Utils/Local.cpp

1289

///

1293

I think this is redundant with the access on the next line?

[bjope]

lib/Transforms/Utils/Local.cpp

1319

Current solution is fine with me (as it seems to be one step in the right direction).

Alternatives solutions could be:

• Let DIBuilder::insertDbgValueIntrinsic strip the line number? (that would also cover the creation of dbg.value in LoopUtils, Debugify and LLVMDIBuilder*)
• Strip line/col number already when creating dbg.declare? (or do we need it in dbg.declare)

Some additional thing in this patch could be (if this is a new rule):

• Also fix calls to DIBuilder::insertDbgValueIntrinsic in LoopUtils, Debugify and LLVMDIBuilder* the same way.
• Assert that line/col is zero in DIBuilder::insertDbgValueIntrinsic.
• Add checks in Verifier and MachineVerifier that line/col is zero for dbg.value/DBG_VALUE.

[jmorse]

lib/Transforms/Utils/Local.cpp

1319

Bjorn wrote:

Some additional thing in this patch could be (if this is a new rule):

Also fix calls to DIBuilder::insertDbgValueIntrinsic in LoopUtils, Debugify and LLVMDIBuilder* the same way.
Assert that line/col is zero in DIBuilder::insertDbgValueIntrinsic.
Add checks in Verifier and MachineVerifier that line/col is zero for dbg.value/DBG_VALUE.

Sounds good -- I've dropped those details in this bug report [0] so that they don't get lost, the Verifier checks in particular should be good for detecting error states.

[0] https://bugs.llvm.org/show_bug.cgi?id=41827

lib/Transforms/Utils/Local.cpp

static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   if (DII->isAddressOfVariable())
     if (auto *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocation()))
       if (auto FragmentSize = AI->getAllocationSizeInBits(DL))
         return ValueSize >= *FragmentSize;
   // Could not determine size of variable. Conservatively return false.
   return false;
 }
 
+// Determine what DebugLoc to use for each dbg.declare/inst pair that are
+// promoted to a dbg.value.
+static DebugLoc getDebugValueLoc(DbgVariableIntrinsic *DII, Instruction *Src) {
+  // Original dbg.declare must have a location.
+  DebugLoc DeclareLoc = DII->getDebugLoc();
+  assert(DeclareLoc);
+  MDNode *Scope = DeclareLoc.getScope();
+  DILocation *InlinedAt = DeclareLoc.getInlinedAt();
+
+  // Can we use the source location of the instruction causing the debug
+  // promotion? If it's in the same scope and inlining instance, then yes.
+  if (Src)
+    if (DebugLoc SrcLoc = Src->getDebugLoc())
+      if (SrcLoc.getScope() == Scope && SrcLoc.getInlinedAt() == InlinedAt)
+        return SrcLoc;
+
+  // Otherwise, we will be generating a debug intrinsic for a variable accessed
+  // by code not in the original scope, likely due to inlining. Neither the
+  // accessing instruction nor the place the variable was declared would make
+  // sense: use an unknown location instead.
+  return DebugLoc::get(0, 0, Scope, InlinedAt);
+}
+
 /// 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());
   auto *DIVar = DII->getVariable();
   assert(DIVar && "Missing variable");
   auto *DIExpr = DII->getExpression();
   Value *DV = SI->getOperand(0);
 
+  DebugLoc NewLoc = getDebugValueLoc(DII, SI);
+
   if (!valueCoversEntireFragment(SI->getValueOperand()->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, DII->getDebugLoc(),
-                                      SI);
+      Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
     return;
   }
 
   if (!LdStHasDebugValue(DIVar, DIExpr, SI))
-    Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, DII->getDebugLoc(),
-                                    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;
   }
 
+  DebugLoc NewLoc = getDebugValueLoc(DII, nullptr);
+
   // We are now tracking the loaded value instead of the address. In the
   // future if multi-location support is added to the IR, it might be
   // preferable to keep tracking both the loaded value and the original
   // address in case the alloca can not be elided.
   Instruction *DbgValue = Builder.insertDbgValueIntrinsic(
-      LI, DIVar, DIExpr, DII->getDebugLoc(), (Instruction *)nullptr);
+      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) {
   auto *DIVar = DII->getVariable();
if (!valueCoversEntireFragment(APN->getType(), DII)) {
     LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: "
                       << *DII << '\n');
     return;
   }
 
   BasicBlock *BB = APN->getParent();
   auto InsertionPt = BB->getFirstInsertionPt();
 
+  DebugLoc NewLoc = getDebugValueLoc(DII, nullptr);
+
   // The block may be a catchswitch block, which does not have a valid
   // insertion point.
   // FIXME: Insert dbg.value markers in the successors when appropriate.
   if (InsertionPt != BB->end())
-    Builder.insertDbgValueIntrinsic(APN, DIVar, DIExpr, DII->getDebugLoc(),
-                                    &*InsertionPt);
+    Builder.insertDbgValueIntrinsic(APN, DIVar, DIExpr, NewLoc, &*InsertionPt);
 }
 
 /// Determine whether this alloca is either a VLA or an array.
 static bool isArray(AllocaInst *AI) {
   return AI->isArrayAllocation() ||
     AI->getType()->getElementType()->isArrayTy();
 }
 
for (auto &AIUse : AI->uses()) {
         if (AIUse.getOperandNo() == 1)
           ConvertDebugDeclareToDebugValue(DDI, SI, DIB);
       } else if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
         ConvertDebugDeclareToDebugValue(DDI, LI, DIB);
       } else if (CallInst *CI = dyn_cast<CallInst>(U)) {
         // This is a call by-value or some other instruction that takes a
         // pointer to the variable. Insert a *value* intrinsic that describes
         // the variable by dereferencing the alloca.
+        DebugLoc NewLoc = getDebugValueLoc(DDI, nullptr);
         auto *DerefExpr =
             DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref);
-        DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr,
-                                    DDI->getDebugLoc(), CI);
+        DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr, NewLoc,
+                                    CI);
       }
     }
     DDI->eraseFromParent();
   }
   return true;
 }
 
 /// Propagate dbg.value intrinsics through the newly inserted PHIs.

test/DebugInfo/Generic/dbg-value-lower-linenos.ll

+; RUN: opt < %s -S -mem2reg -instcombine | FileCheck %s
+
+; The '%bar' alloca will be promoted to an SSA register by mem2reg: test that
+; sane line number are assigned to the dbg.value intrinsics that are inserted
+; to represent changes in variable value. There are three flavours:
+;  * bb1 will start with a PHI node that has no line number. The associated
+;    dbg.value should have an 'unknown' line number as a result.
+;  * The mid-bb1 store will have a representative dbg.value created, and should
+;    take on the line number of the store that it replaces.
+;  * The store in bb2 deliberately has a line number in a different scope, with
+;    an inlinedAt record. The dbg.value that will be created should get an
+;    unknown line number.
+
+; In the second function here, LowerDbgDeclare will promote various variable
+; accesses of a dbg.declare'd alloca into dbg.values. Check that their line
+; numbers are sane too. (IR copied from DebugInfo/X86/formal_parameter.ll).
+
+; CHECK-LABEL: define i32 @foo
+
+; CHECK-LABEL: bb1:
+; CHECK-NEXT:  %bar.0 = phi i32
+; CHECK-NEXT:  dbg.value(metadata i32 %bar.0,{{.*}}), !dbg ![[UNKNOWN:[0-9]+]]
+; CHECK-NEXT:  %totest = load
+; CHECK-NEXT:  %add = add i32 %bar.0
+; CHECK-NEXT:  dbg.value(metadata i32 %add, {{.*}}), !dbg ![[STORELINE:[0-9]+]]
+; CHECK-NEXT:  %cond = icmp ult
+; CHECK-NEXT:  br i1 %cond, label %bb1, label %bb2
+;
+; CHECK-LABEL: bb2:
+; CHECK-NEXT:  %toret = add i32 %bar.0, 3
+; CHECK-NEXT:  dbg.value(metadata i32 %toret, {{.*}}), !dbg ![[UNKNOWN]]
+; CHECK-NEXT:  ret i32 %toret
+
+define i32 @foo(i32 *%bees, i32 *%output) {
+entry:
+  %bar = alloca i32
+  call void @llvm.dbg.declare(metadata i32 *%bar, metadata !7, metadata !DIExpression()), !dbg !6
+  store i32 0, i32 *%bar
+  br label %bb1, !dbg !6
+
+bb1:
+  %totest = load i32, i32 *%bees, !dbg !8
+  %load1 = load i32, i32 *%bar, !dbg !9
+  %add = add i32 %load1, 1, !dbg !10
+  store i32 %add, i32 *%bar, !dbg !11
+  %toret = add i32 %add, 2, !dbg !12
+  %cond = icmp ult i32 %totest, %load1, !dbg !13
+  br i1 %cond, label %bb1, label %bb2, !dbg !14
+
+bb2:
+  store i32 %toret, i32 *%bar, !dbg !16
+  ret i32 %toret
+}
+
+; In the following, the dbg.value created for the store should get the stores
+; line number, the other dbg.values should be unknown.
+; CHECK-LABEL: define void @bar
+;
+; CHECK:      dbg.value(metadata i32 %map, metadata ![[MAPVAR:[0-9]+]],{{.*}}),
+; CHECK-SAME:           !dbg ![[STORE2:[0-9]+]]
+; CHECK-NEXT: store
+; CHECK-NEXT: dbg.value(metadata i32* %map.addr, metadata ![[MAPVAR]],
+; CHECK-SAME:           metadata !DIExpression(DW_OP_deref)),
+; CHECK-SAME:           !dbg ![[UNKNOWN2:[0-9]+]]
+; CHECK-NEXT: call
+; CHECK-NEXT: load
+; CHECK-NEXT: dbg.value(metadata i32 %{{[0-9]+}}, metadata ![[MAPVAR]],
+; CHECK-SAME:           !dbg ![[UNKNOWN2]]
+
+define void @bar(i32 %map) !dbg !20 {
+entry:
+  %map.addr = alloca i32, align 4
+  store i32 %map, i32* %map.addr, align 4, !dbg !27
+  call void @llvm.dbg.declare(metadata i32* %map.addr, metadata !21, metadata !DIExpression()), !dbg !22
+  %call = call i32 (i32*, ...) bitcast (i32 (...)* @lookup to i32 (i32*, ...)*)(i32* %map.addr), !dbg !23
+%0 = load i32, i32* %map.addr, align 4, !dbg !24
+  %call1 = call i32 (i32, ...) bitcast (i32 (...)* @verify to i32 (i32, ...)*)(i32 %0), !dbg !25
+  ret void, !dbg !26
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+declare i32 @verify(...)
+declare i32 @lookup(...)
+
+; CHECK: ![[SUBPROG:[0-9]+]] = distinct !DISubprogram(name: "nope",
+; CHECK: ![[UNKNOWN]] = !DILocation(line: 0, scope: ![[SUBPROG]])
+; CHECK: ![[STORELINE]] = !DILocation(line: 5, scope: ![[SUBPROG]])
+
+; CHECK: ![[SUBPROG2:[0-9]+]] = distinct !DISubprogram(name: "thin",
+; CHECK: ![[MAPVAR]] = !DILocalVariable(name: "floogie",
+; CHECK: ![[STORE2]] = !DILocation(line: 20
+; CHECK: ![[UNKNOWN2]] = !DILocation(line: 0
+
+!llvm.module.flags = !{!4}
+!llvm.dbg.cu = !{!2}
+!1 = !DILocalVariable(name: "bees", scope: !5, type: null)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug)
+!3 = !DIFile(filename: "bees.cpp", directory: "")
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "nope", scope: !3, file: !3, line: 1, unit: !2)
+!6 = !DILocation(line: 1, scope: !5)
+!7 = !DILocalVariable(name: "flannel", scope: !5, type: null)
+!8 = !DILocation(line: 2, scope: !5)
+!9 = !DILocation(line: 3, scope: !5)
+!10 = !DILocation(line: 4, scope: !5)
+!11 = !DILocation(line: 5, scope: !5)
+!12 = !DILocation(line: 6, scope: !5)
+!13 = !DILocation(line: 7, scope: !5)
+!14 = !DILocation(line: 8, scope: !5)
+!15 = distinct !DISubprogram(name: "wat", scope: !2, file: !3, line: 10, unit: !2)
+!16 = !DILocation(line: 9, scope: !15, inlinedAt: !14)
+!20 = distinct !DISubprogram(name: "thin", scope: !3, file: !3, line: 20, unit: !2)
+!21 = !DILocalVariable(name: "floogie", scope: !20, type: null)
+!22 = !DILocation(line: 21, scope: !20)
+!23 = !DILocation(line: 22, scope: !20)
+!24 = !DILocation(line: 23, scope: !20)
+!25 = !DILocation(line: 24, scope: !20)
+!26 = !DILocation(line: 25, scope: !20)
+!27 = !DILocation(line: 20, scope: !20)

test/DebugInfo/X86/formal_parameter.ll

entry:
   ; LOWERING: call void @llvm.dbg.value{{.*}}, !dbg ![[LOC:.*]]
   ; LOWERING: call void @llvm.dbg.value{{.*}}, !dbg ![[LOC]]
   ; LOWERING: call void @llvm.dbg.value{{.*}}, !dbg ![[LOC]]
 %0 = load i32, i32* %map.addr, align 4, !dbg !20, !tbaa !15
   %call1 = call i32 (i32, ...) bitcast (i32 (...)* @verify to i32 (i32, ...)*)(i32 %0) #3, !dbg !20
   ret void, !dbg !22
 }
 
+; LOWERING: ![[SCOPE:[0-9]+]] = distinct !DISubprogram(name: "foo",
+; LOWERING: ![[LOC]] = !DILocation(line: 0, scope: ![[SCOPE]]
+
 ; Function Attrs: nounwind readnone
 declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
 
 declare i32 @lookup(...)
 
 declare i32 @verify(...)
 
 ; Function Attrs: nounwind readnone