[InstCombine] Add a flag to disable LowerDbgDeclare

[rnk]

Summary

This should improve optimized debug info for address-taken variables at
the cost of inaccurate debug info in some situations.

We patched this into clang and deployed this change to Chromium
developers, and this significantly improved debuggability of optimized
code. The long-term solution to PR34136 seems more and more like it's
going to take a while, so I would like to commit this change under a
flag so that it can be used as a stop-gap measure.

This flag should really help so for C++ aggregates like std::string and
std::vector, which are typically address-taken, even after inlining, and
cannot be SROA-ed.

[aprantl]

What happens when there are only loads from the alloca, shouldn't they be handled similarly?

int i;
f(&i)
return i+1;

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

2061 ↗

(On Diff #110643)

What's the effect of this change?

2097 ↗

(On Diff #110643)

Maybe "If we are removing an alloca ..."?

[aprantl]

It can also be problematic if stack coloring arranges to reuse the stack memory for other data.

If lifetime intrinsics survive into MIR, then we could teach DebugValueHistoryCalculator about them. Otherwise we could have the stack coloring pass lower dbg.declares describing allocas with lifetime intrinsics into a dbg.value(%alloca, ...) at the beginning and a dbg.value(null, ...) at the end of the live range.

[aprantl]

Oh, and we'd have to teach LiveDebugValues about memory locations (probably not very hard).

[aprantl]

It is possible that this change could regress debug info quality when other passes like GVN delete stores to locals.

Could we create a utility function updateDebugInfoForDeletedStore that finds any dbg.declare describing the alloca and runs ConvertDebugDeclareToDebugValue?

[aprantl]

Sorry for updating this so often :-)
If the stack coloring change I outlined above turns out to be too complicated and the deferred ConvertDebugDeclareToDebugValue in GVN works, we could do the same thing in the stack coloring pass. (Perhaps even hook the functionality into the llvm::salvageDebugInfo() API that is there to be called on instructions before they are deleted).

[rnk]

Long term, yes, I think we'll want to push the lifetime intrinsics down to MI so we can get better PC ranges for variable locations. I don't know when they are removed right now.

Trying to think more broadly, here's how things work today:

• In the C abstract machine, variables live in memory. LLVM's job is to erode that abstraction.
• dbg.declare says simply "this variable lives here in memory, its value changes whenever it is stored to"
• Any pass that deletes dead stores (or more generally dead memset/memcpy/writes) will cause the user to see a stale value in the debugger from a missed write
• dbg.value gives us a way to describe dead stores in debug info

The problem is that we can't mix dbg.value and dbg.declare. For variables that we can promote to SSA, we can describe all the stores with dbg.value, and that's good enough. It sounds like we have problems for values that are spilled from registers, but that's a QoI issue that doesn't require redesign.

For everything else, we need a way of describing the effects of more powerful optimizations like GVN, DSE, and memcpyopt. We need a way to say, "this variable usually lives in memory, but I deleted the store at .Ltmp1 because it was made dead by the store at .Ltmp2. The value at .Ltmp1 is 42, and at .Ltmp2 it is whatever is in memory." I'm starting to think we should basically use dbg.value with DW_OP_deref as a way to say "at this program point, the variable lives in memory". If that works, we could probably eliminate dbg.declare in favor of dbg.value+DW_OP_deref. Most store-eliminating optimizations know exactly what makes the store dead (free, lifetime.end, memset, or plain store), so they know where to insert the dbg.value+DW_OP_deref to put the variable back in memory.

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

2061 ↗

(On Diff #110643)

Now that we have ValueAsMetadata, these no longer appear in use lists. These case labels are just stale. I'll just commit this separately.

[rnk]

I think I forgot to write my conclusion, i.e. what we should do with this patch. :)

I think this patch is an incremental step towards what I sketched out. In the long run, the frontend wouldn't emit dbg.declare, it would emit dbg.value+DW_OP_deref. We would never call LowerDbgDeclare. Passes that remove stores would use utilities similar to salvageDebugInfo and ConvertDebugDeclareToDebugValue to insert dbg.values when deleting stores.

[aprantl]

I think this patch is an incremental step towards what I sketched out. In the long run, the frontend wouldn't emit dbg.declare, it would emit dbg.value+DW_OP_deref.

That's something I'm not yet sure about. With a dbg.declare it is very clear that the location is not just describing the value of the source variable, but can also be written to by the debugger, when the user wants to assign the variable a new value. The DWARF standard call this implicit vs. explicit location descriptions. We don't really distinguish between these two cases in LLVM yet, but dbg.value vs dbg.declare seems to make sense for it.

That said, this is not something we need to decide on for this patch :-)

Do you think you could factor the code in this patch into llvm::salvageDebugInfo() (found in Local.cpp)? There doesn't seem to be anything specific to InstCombine and this will make it easier to apply it to other passes later.

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

2061 ↗

(On Diff #110643)

Makes sense, thanks!

2138 ↗

(On Diff #110643)

I think this got lost in my flurry of other comments...
Don't we need to handle loads (and potentially calls) similarly (see the example in my previous comment)?

[rnk]

In D36596#841456, @aprantl wrote:

I think this patch is an incremental step towards what I sketched out. In the long run, the frontend wouldn't emit dbg.declare, it would emit dbg.value+DW_OP_deref.

That's something I'm not yet sure about. With a dbg.declare it is very clear that the location is not just describing the value of the source variable, but can also be written to by the debugger, when the user wants to assign the variable a new value. The DWARF standard call this implicit vs. explicit location descriptions. We don't really distinguish between these two cases in LLVM yet, but dbg.value vs dbg.declare seems to make sense for it.

Hm, I hadn't considered that. I skimmed section 2.6 of the dwarf 5 standard, and it talks about "single location descriptions" and "location lists". I assumed that the single location case was just an encoding size optimization, and didn't affect whether the variable could be assigned.

I think variable assignment will be even harder to make work than ensuring that we always present accurate debug variable information. Passes like GVN will eliminate loads that were present in the source program. If the user updates a variable, it may not be reflected in the load immediately on the next line. We might want to disable variable assignment completely when a function isn't marked optnone.

That said, this is not something we need to decide on for this patch :-)

Do you think you could factor the code in this patch into llvm::salvageDebugInfo() (found in Local.cpp)? There doesn't seem to be anything specific to InstCombine and this will make it easier to apply it to other passes later.

Are you suggesting that salvageDebugInfo should call ConvertDbgDeclareToDbgValue(StoreInst*)? Seems doable.

---

I put together a test case that shows problems after this patch:

void escape(int *);
void g(void);
void f(void) {
  int x = 1;
  g();
  x = 2;
  escape(&x);
}

Without this patch, instcombine inserts dbg.value calls. After this patch, if you break on g, x will probably appear uninitialized. It probably isn't hard to fix DSE in this patch, but I suspect there is a long tail of other passes that delete stores as well.

I wasn't able to craft a test case that causes stack coloring to do anything mean.

[rnk]

• Conditionalize things under -instcombine-lower-dbg-declare

[rnk]

I rewrote this to use a cl::opt so that users can try this patch out with an -mllvm flag. It should allow them to get more debug info at the cost of possible innacurate debug info when DSE kicks in as described in PR34136. The long-term solution to PR34136 looks like it's going to take a while, so I want to have a workaround that we can offer to Chromium in the meantime. I expect they might actually use it for Linux and Android crash analysis as well as on Windows.

Does that seem reasonable?

[aprantl]

Is this obsoleted by the dbg.addr proposal or orthogonal?

[rnk]

In D36596#864027, @aprantl wrote:

Is this obsoleted by the dbg.addr proposal or orthogonal?

Orthogonal, I want to do both.

[aprantl]

Ok. I think there should be a PR to revert this change that is blocked on PR34136. The description of the option should explicitly spell out a big warning that this will introduce false positives and that it is experimental/temporary only. We should aim to get this reverted as soon as possible.

[aprantl]

And we should also stake out a clear path to what is necessary to make this patch obsolete and document what is missing.

[rnk]

In D36596#864089, @aprantl wrote:

Ok. I think there should be a PR to revert this change that is blocked on PR34136. The description of the option should explicitly spell out a big warning that this will introduce false positives and that it is experimental/temporary only. We should aim to get this reverted as soon as possible.

Are you sure we need a new PR? To fix PR34136, we effectively need to enable the new behavior all the time: stop converting dbg.declare or dbg.addr to dbg.value in instcombine. We just can't do that today because we don't have the infrastructure to fix all the inaccurate debug info that such a change would create. Once we have dbg.addr, we can fix DSE to avoid inaccuracies, but only if we leave dbg.addr alone in instcombine.

Anyway, uploading a new patch shortly with a FIXME comment describing what the flag does and when it can be removed.

[rnk]

• FIXME comment

[rnk]

Does this look good as is? I'd like to use this to gather some user feedback.

[aprantl]

With the FIXME in the code this LGTM since it leaves no doubt that we want to migrate away from it.

llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/CFG.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"

 static cl::opt<bool>
 EnableExpensiveCombines("expensive-combines",
                         cl::desc("Enable expensive instruction combines"));
 
 static cl::opt<unsigned>
 MaxArraySize("instcombine-maxarray-size", cl::init(1024),
              cl::desc("Maximum array size considered when doing a combine"));
 
+// FIXME: Remove this flag when it is no longer necessary to convert
+// llvm.dbg.declare to avoid inaccurate debug info. Setting this to false
+// increases variable availability at the cost of accuracy. Variables that
+// cannot be promoted by mem2reg or SROA will be described as living in memory
+// for their entire lifetime. However, passes like DSE and instcombine can
+// delete stores to the alloca, leading to misleading and inaccurate debug
+// information. This flag can be removed when those passes are fixed.
+static cl::opt<unsigned> ShouldLowerDbgDeclare("instcombine-lower-dbg-declare",
+                                               cl::Hidden, cl::init(true));
+
 Value *InstCombiner::EmitGEPOffset(User *GEP) {
   return llvm::EmitGEPOffset(&Builder, DL, GEP);
 }
 
 /// Return true if it is desirable to convert an integer computation from a
 /// given bit width to a new bit width.
 /// We don't want to convert from a legal to an illegal type or from a smaller
 /// to a larger illegal type. A width of '1' is always treated as a legal type
static bool isAllocSiteRemovable(Instruction *AI,
   return true;
 }
 
 Instruction *InstCombiner::visitAllocSite(Instruction &MI) {
   // If we have a malloc call which is only used in any amount of comparisons
   // to null and free calls, delete the calls and replace the comparisons with
   // true or false as appropriate.
   SmallVector<WeakTrackingVH, 64> Users;
+
+  // If we are removing an alloca with a dbg.declare, insert dbg.value calls
+  // before each store.
+  DbgDeclareInst *DDI = nullptr;
+  std::unique_ptr<DIBuilder> DIB;
+  if (isa<AllocaInst>(MI)) {
+    DDI = FindAllocaDbgDeclare(&MI);
+    DIB.reset(new DIBuilder(*MI.getModule(), /*AllowUnresolved=*/false));
+  }
+
   if (isAllocSiteRemovable(&MI, Users, &TLI)) {
     for (unsigned i = 0, e = Users.size(); i != e; ++i) {
       // Lowering all @llvm.objectsize calls first because they may
       // use a bitcast/GEP of the alloca we are removing.
       if (!Users[i])
        continue;
 
       Instruction *I = cast<Instruction>(&*Users[i]);
for (unsigned i = 0, e = Users.size(); i != e; ++i) {
 
       if (ICmpInst *C = dyn_cast<ICmpInst>(I)) {
         replaceInstUsesWith(*C,
                             ConstantInt::get(Type::getInt1Ty(C->getContext()),
                                              C->isFalseWhenEqual()));
       } else if (isa<BitCastInst>(I) || isa<GetElementPtrInst>(I) ||
                  isa<AddrSpaceCastInst>(I)) {
         replaceInstUsesWith(*I, UndefValue::get(I->getType()));
+      } else if (DDI && isa<StoreInst>(I)) {
+        ConvertDebugDeclareToDebugValue(DDI, cast<StoreInst>(I), *DIB);
       }
       eraseInstFromFunction(*I);
     }
 
     if (InvokeInst *II = dyn_cast<InvokeInst>(&MI)) {
       // Replace invoke with a NOP intrinsic to maintain the original CFG
       Module *M = II->getModule();
       Function *F = Intrinsic::getDeclaration(M, Intrinsic::donothing);
       InvokeInst::Create(F, II->getNormalDest(), II->getUnwindDest(),
                          None, "", II->getParent());
     }
+
+    if (DDI)
+      eraseInstFromFunction(*DDI);
+
     return eraseInstFromFunction(MI);
   }
   return nullptr;
 }
 
 /// \brief Move the call to free before a NULL test.
 ///
 /// Check if this free is accessed after its argument has been test
IRBuilder<TargetFolder, IRBuilderCallbackInserter> Builder(
 
         using namespace llvm::PatternMatch;
         if (match(I, m_Intrinsic<Intrinsic::assume>()))
           AC.registerAssumption(cast<CallInst>(I));
       }));
 
   // Lower dbg.declare intrinsics otherwise their value may be clobbered
   // by instcombiner.
-  bool MadeIRChange = LowerDbgDeclare(F);
+  bool MadeIRChange = false;
+  if (ShouldLowerDbgDeclare)
+    MadeIRChange = LowerDbgDeclare(F);
 
   // Iterate while there is work to do.
   int Iteration = 0;
   for (;;) {
     ++Iteration;
     DEBUG(dbgs() << "\n\nINSTCOMBINE ITERATION #" << Iteration << " on "
                  << F.getName() << "\n");
 

llvm/trunk/test/Transforms/InstCombine/debuginfo-skip.ll

-; RUN: opt < %s -instcombine -debug -S -o %t 2>&1 | FileCheck %s
-; RUN: cat %t | FileCheck %s --check-prefix=CHECK-IR
-; REQUIRES: asserts
-
-; Debug output from InstCombine should not have any @llvm.dbg.* instructions visited
-; CHECK-NOT: call void @llvm.dbg.
-
-; The resulting IR should still have them
-; CHECK-IR: call void @llvm.dbg.
+; RUN: opt -instcombine-lower-dbg-declare=0 < %s -instcombine -S | FileCheck %s
+; RUN: opt -instcombine-lower-dbg-declare=1 < %s -instcombine -S | FileCheck %s
 
 define i32 @foo(i32 %j) #0 !dbg !7 {
 entry:
   %j.addr = alloca i32, align 4
   store i32 %j, i32* %j.addr, align 4
   call void @llvm.dbg.declare(metadata i32* %j.addr, metadata !11, metadata !12), !dbg !13
   call void @llvm.dbg.value(metadata i32 10, metadata !16, metadata !12), !dbg !15
   %0 = load i32, i32* %j.addr, align 4, !dbg !14
   ret i32 %0, !dbg !15
 }
 
+; Instcombine can remove the alloca and forward the load to store, but it
+; should convert the declare to dbg value.
+; CHECK-LABEL: define i32 @foo(i32 %j)
+; CHECK-NOT: alloca
+; CHECK: call void @llvm.dbg.value(metadata i32 %j, {{.*}})
+; CHECK: call void @llvm.dbg.value(metadata i32 10, {{.*}})
+; CHECK: ret i32 %j
+
 declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
 declare void @llvm.dbg.value(metadata, metadata, metadata) #1
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5}
 !llvm.ident = !{!6}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang 5.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)

llvm/trunk/test/Transforms/InstCombine/debuginfo.ll

-; RUN: opt < %s -instcombine -S | FileCheck %s
+; RUN: opt < %s -instcombine -instcombine-lower-dbg-declare=0 -S \
+; RUN:      | FileCheck %s --check-prefix=CHECK --check-prefix=NOLOWER
+; RUN: opt < %s -instcombine -instcombine-lower-dbg-declare=1 -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux"
+
+%struct.TwoRegs = type { i64, i64 }
 
 declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone
 
 declare i64 @llvm.objectsize.i64.p0i8(i8*, i1) nounwind readnone
 
-declare i8* @foo(i8*, i32, i64, i64) nounwind
+declare i8* @passthru_callee(i8*, i32, i64, i64)
 
-define hidden i8* @foobar(i8* %__dest, i32 %__val, i64 %__len) nounwind inlinehint ssp !dbg !1 {
+define i8* @passthru(i8* %a, i32 %b, i64 %c) !dbg !1 {
 entry:
-  %__dest.addr = alloca i8*, align 8
-  %__val.addr = alloca i32, align 4
-  %__len.addr = alloca i64, align 8
-  store i8* %__dest, i8** %__dest.addr, align 8
-; CHECK-NOT: call void @llvm.dbg.declare
-; CHECK: call void @llvm.dbg.value
-  call void @llvm.dbg.declare(metadata i8** %__dest.addr, metadata !0, metadata !DIExpression()), !dbg !16
-  store i32 %__val, i32* %__val.addr, align 4
-  call void @llvm.dbg.declare(metadata i32* %__val.addr, metadata !7, metadata !DIExpression()), !dbg !18
-  store i64 %__len, i64* %__len.addr, align 8
-  call void @llvm.dbg.declare(metadata i64* %__len.addr, metadata !9, metadata !DIExpression()), !dbg !20
-  %tmp = load i8*, i8** %__dest.addr, align 8, !dbg !21
-  %tmp1 = load i32, i32* %__val.addr, align 4, !dbg !21
-  %tmp2 = load i64, i64* %__len.addr, align 8, !dbg !21
-  %tmp3 = load i8*, i8** %__dest.addr, align 8, !dbg !21
+  %a.addr = alloca i8*, align 8
+  %b.addr = alloca i32, align 4
+  %c.addr = alloca i64, align 8
+  store i8* %a, i8** %a.addr, align 8
+  call void @llvm.dbg.declare(metadata i8** %a.addr, metadata !0, metadata !DIExpression()), !dbg !16
+  store i32 %b, i32* %b.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %b.addr, metadata !7, metadata !DIExpression()), !dbg !18
+  store i64 %c, i64* %c.addr, align 8
+  call void @llvm.dbg.declare(metadata i64* %c.addr, metadata !9, metadata !DIExpression()), !dbg !20
+  %tmp = load i8*, i8** %a.addr, align 8, !dbg !21
+  %tmp1 = load i32, i32* %b.addr, align 4, !dbg !21
+  %tmp2 = load i64, i64* %c.addr, align 8, !dbg !21
+  %tmp3 = load i8*, i8** %a.addr, align 8, !dbg !21
   %0 = call i64 @llvm.objectsize.i64.p0i8(i8* %tmp3, i1 false), !dbg !21
-  %call = call i8* @foo(i8* %tmp, i32 %tmp1, i64 %tmp2, i64 %0), !dbg !21
+  %call = call i8* @passthru_callee(i8* %tmp, i32 %tmp1, i64 %tmp2, i64 %0), !dbg !21
   ret i8* %call, !dbg !21
 }
 
+; CHECK-LABEL: define i8* @passthru(i8* %a, i32 %b, i64 %c)
+; CHECK-NOT: alloca
+; CHECK-NOT: store
+; CHECK-NOT: call void @llvm.dbg.declare
+; CHECK: call void @llvm.dbg.value(metadata i8* %a, {{.*}})
+; CHECK-NOT: store
+; CHECK: call void @llvm.dbg.value(metadata i32 %b, {{.*}})
+; CHECK-NOT: store
+; CHECK: call void @llvm.dbg.value(metadata i64 %c, {{.*}})
+; CHECK-NOT: store
+; CHECK: call i8* @passthru_callee(i8* %a, i32 %b, i64 %c, i64 %{{.*}})
+
+declare void @tworegs_callee(i64, i64)
+
+; Lowering dbg.declare in instcombine doesn't handle this case very well.
+
+define void @tworegs(i64 %o.coerce0, i64 %o.coerce1) !dbg !31 {
+entry:
+  %o = alloca %struct.TwoRegs, align 8
+  %0 = bitcast %struct.TwoRegs* %o to { i64, i64 }*
+  %1 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %0, i32 0, i32 0
+  store i64 %o.coerce0, i64* %1, align 8
+  %2 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %0, i32 0, i32 1
+  store i64 %o.coerce1, i64* %2, align 8
+  call void @llvm.dbg.declare(metadata %struct.TwoRegs* %o, metadata !35, metadata !DIExpression()), !dbg !32
+  %3 = bitcast %struct.TwoRegs* %o to { i64, i64 }*, !dbg !33
+  %4 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %3, i32 0, i32 0, !dbg !33
+  %5 = load i64, i64* %4, align 8, !dbg !33
+  %6 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %3, i32 0, i32 1, !dbg !33
+  %7 = load i64, i64* %6, align 8, !dbg !33
+  call void @tworegs_callee(i64 %5, i64 %7), !dbg !33
+  ret void, !dbg !33
+}
+
+; NOLOWER-LABEL: define void @tworegs(i64 %o.coerce0, i64 %o.coerce1)
+; NOLOWER-NOT: alloca
+; NOLOWER-NOT: store
+; NOLOWER-NOT: call void @llvm.dbg.declare
+; NOLOWER: call void @llvm.dbg.value(metadata i64 %o.coerce0, {{.*}})
+; NOLOWER-NOT: store
+; NOLOWER: call void @llvm.dbg.value(metadata i64 %o.coerce1, {{.*}})
+; NOLOWER-NOT: store
+; NOLOWER: call void @tworegs_callee(i64 %o.coerce0, i64 %o.coerce1)
+
+
 !llvm.dbg.cu = !{!3}
 !llvm.module.flags = !{!30}
 
-!0 = !DILocalVariable(name: "__dest", line: 78, arg: 1, scope: !1, file: !2, type: !6)
-!1 = distinct !DISubprogram(name: "foobar", line: 79, isLocal: true, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !3, scopeLine: 79, file: !27, scope: !2, type: !4, variables: !25)
+!0 = !DILocalVariable(name: "a", line: 78, arg: 1, scope: !1, file: !2, type: !6)
+!1 = distinct !DISubprogram(name: "passthru", line: 79, isLocal: true, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: true, unit: !3, scopeLine: 79, file: !27, scope: !2, type: !4, variables: !25)
 !2 = !DIFile(filename: "string.h", directory: "Game")
 !3 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang version 3.0 (trunk 127710)", isOptimized: true, emissionKind: FullDebug, file: !28, enums: !29, retainedTypes: !29)
 !4 = !DISubroutineType(types: !5)
 !5 = !{!6}
 !6 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, scope: !3, baseType: null)
-!7 = !DILocalVariable(name: "__val", line: 78, arg: 2, scope: !1, file: !2, type: !8)
+!7 = !DILocalVariable(name: "b", line: 78, arg: 2, scope: !1, file: !2, type: !8)
 !8 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-!9 = !DILocalVariable(name: "__len", line: 78, arg: 3, scope: !1, file: !2, type: !10)
-!10 = !DIDerivedType(tag: DW_TAG_typedef, name: "size_t", line: 80, file: !27, scope: !3, baseType: !11)
-!11 = !DIDerivedType(tag: DW_TAG_typedef, name: "__darwin_size_t", line: 90, file: !27, scope: !3, baseType: !12)
+!9 = !DILocalVariable(name: "c", line: 78, arg: 3, scope: !1, file: !2, type: !12)
 !12 = !DIBasicType(tag: DW_TAG_base_type, name: "long unsigned int", size: 64, align: 64, encoding: DW_ATE_unsigned)
 !16 = !DILocation(line: 78, column: 28, scope: !1)
 !18 = !DILocation(line: 78, column: 40, scope: !1)
 !20 = !DILocation(line: 78, column: 54, scope: !1)
 !21 = !DILocation(line: 80, column: 3, scope: !22)
 !22 = distinct !DILexicalBlock(line: 80, column: 3, file: !27, scope: !23)
 !23 = distinct !DILexicalBlock(line: 79, column: 1, file: !27, scope: !1)
 !25 = !{!0, !7, !9}
-!26 = !DIFile(filename: "bits.c", directory: "Game")
 !27 = !DIFile(filename: "string.h", directory: "Game")
 !28 = !DIFile(filename: "bits.c", directory: "Game")
 !29 = !{}
 !30 = !{i32 1, !"Debug Info Version", i32 3}
+
+!31 = distinct !DISubprogram(name: "tworegs", scope: !28, file: !28, line: 4, type: !4, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: true, unit: !3, variables: !34)
+!32 = !DILocation(line: 4, column: 23, scope: !31)
+!33 = !DILocation(line: 5, column: 3, scope: !31)
+!34 = !{!35}
+!35 = !DILocalVariable(name: "o", arg: 1, scope: !31, file: !28, line: 4, type: !36)
+!36 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "TwoRegs", file: !28, line: 1, size: 128, elements: !37)
+!37 = !{!38, !39}
+!38 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !36, file: !28, line: 1, baseType: !12, size: 64)
+!39 = !DIDerivedType(tag: DW_TAG_member, name: "y", scope: !36, file: !28, line: 1, baseType: !12, size: 64)
+!40 = !DISubroutineType(types: !41)
+!41 = !{!36}

llvm/trunk/test/Transforms/Util/simplify-dbg-declare-load.ll

-; RUN: opt -instcombine -S < %s | FileCheck %s
+; RUN: opt -instcombine -instcombine-lower-dbg-declare=1 -S < %s | FileCheck %s
+; RUN: opt -instcombine -instcombine-lower-dbg-declare=0 -S < %s | FileCheck %s --check-prefix=DECLARE
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 %foo = type { i64, i32, i32 }
 
 ; Function Attrs: nounwind readnone
 declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
 declare void @llvm.dbg.value(metadata, metadata, metadata) #0

 idxend:                                           ; preds = %top
 ; CHECK-NOT: call void @llvm.dbg.value(metadata %foo* %cp,
   %0 = load volatile %foo, %foo* %cp, align 8
 ; CHECK: call void @llvm.dbg.value(metadata %foo %0,
   store volatile %foo %0, %foo* undef, align 8
   ret void
 }
 
+; Keep the declare if we keep the alloca.
+; DECLARE-LABEL: define void @julia_fastshortest_6256()
+; DECLARE: %cp = alloca %foo, align 8
+; DECLARE: call void @llvm.dbg.declare(metadata %foo* %cp,
+
 attributes #0 = { nounwind readnone }
 attributes #1 = { sspreq }
 
 !llvm.module.flags = !{!0}
 !llvm.dbg.cu = !{!18}
 
 !0 = !{i32 1, !"Debug Info Version", i32 3}
 !1 = !DILocalVariable(name: "cp", scope: !2, file: !3, line: 106, type: !12)