[PATCH] LowerDbgDeclare - get rid of redundant dbg.values when an alloca survives optimization

[Adrian Prantl]

On Oct 18, 2013, at 10:24, David Blaikie <dblaikie at gmail.com> wrote:

> 
> 
> 
> On Fri, Oct 18, 2013 at 10:12 AM, Adrian Prantl <aprantl at apple.com> wrote:
> Any final verdict for this change?
> 
> I'm not really familiar with this stuff to provide a 'final verdict' as it were, but I'll ask some questions anyway...
> 
> Is this the correct test? Is it possible that the variable might be reconstituted around the call, rather than kept live for the entire range?
> If there are things that prevent the alloca from being elided, are those things enshrined in a test somewhere that we could reuse, rather than reimplementing one part of them?

That’s a very good point indeed: I just discovered 
> bool llvm::isAllocaPromotable(const AllocaInst *AI)
in mem2reg.

-- adrian

> 
> (& if you felt like it, you could probably use find_if to write that loop - but that might not add a lot to the situation (& not sure it'll work with all the template machinery of 'isa'))
>  
> 
> -- adrian
> 
> 
> On Oct 15, 2013, at 13:53, Adrian Prantl <aprantl at apple.com> wrote:
> 
> > Looks like you answered that question yourself. Thanks!
> > ; RUN: opt -instcombine %s -verify -S -asm-verbose | FileCheck %s
> > works just fine.
> >
> > <0001-Debug-info-Reduce-the-amount-of-redundant-debug-info.patch>
> >
> > -- adrian
> >
> > On Oct 15, 2013, at 13:30, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >>
> >>
> >>
> >> On Tue, Oct 15, 2013 at 1:06 PM, Adrian Prantl <aprantl at apple.com> wrote:
> >> You still need to pass -O2 to clang.
> >>
> >> Right - but what I mean is, in your test case, rather than running -O2, could you run only the specific passes that cause the transformation to occur that's concerned? See, for example, the various tests in test/Transforms/*, each of which uses opt arguments to run just the transformation it's testing (-instcombine in the InstCombine tests, -globalopt in the GlobalOpt tests, etc). You could even put this test in the directory for the tests for the transform in question.
> >>
> >> It looks like it's InstCombine, since that's the only caller of this function - but you might need to run the IR through some other transformations first (and checkin the result of that) before passing it to InstCombine in the test.
> >>
> >>
> >> -- adrian
> >>
> >> On Oct 15, 2013, at 13:01, David Blaikie <dblaikie at gmail.com> wrote:
> >>
> >>>
> >>>
> >>>
> >>> On Tue, Oct 15, 2013 at 11:46 AM, Adrian Prantl <aprantl at apple.com> wrote:
> >>>
> >>> On Oct 15, 2013, at 11:42, David Blaikie <dblaikie at gmail.com> wrote:
> >>>
> >>>> Could you attach/paste the IR obtained without your change? I'm curious to see what dbg.values are produced.
> >>>
> >>> ; Function Attrs: nounwind ssp uwtable
> >>> define void @foo() #0 {
> >>>  %offset = alloca i64, align 8
> >>>  call void @llvm.dbg.declare(metadata !{i64* %offset}, metadata !10), !dbg !13
> >>>  call void @llvm.dbg.value(metadata !14, i64 0, metadata !10), !dbg !13
> >>>  store i64 0, i64* %offset, align 8, !dbg !13, !tbaa !15
> >>>  %1 = call i32 @bar(i64* %offset, i64 0) #3, !dbg !19
> >>>  %2 = icmp eq i32 %1, 0, !dbg !19
> >>>  br i1 %2, label %6, label %3, !dbg !19
> >>>
> >>> ; <label>:3                                       ; preds = %0
> >>> ; -----> This is the one I care about:
> >>>  call void @llvm.dbg.value(metadata !{i64* %offset}, i64 0, metadata !10), !dbg !21
> >>>  %4 = load i64* %offset, align 8, !dbg !21, !tbaa !15
> >>>  %5 = call i32 @bar(i64* %offset, i64 %4) #3, !dbg !21
> >>>  br label %6, !dbg !21
> >>>
> >>> ; <label>:6                                       ; preds = %3, %0
> >>>  ret void, !dbg !22
> >>> }
> >>>
> >>>> & a little bit of 'asking for a pony' - but is there one specific optimization you could run to demonstrate this problem rather than all of -O2?
> >>>
> >>> No ponies, unfortunately. The specific optimization _is_ LowerDbgDeclare and it usually gets run by clang even if you go straight to -emit-llvm.
> >>>
> >>> Sorry, I'm not sure I follow. When I run clang -g -O0 -S -emit-llvm on the source you listed (and without your change applied) I get one dbg.declare, no dbg.values). So I don't know what you mean by "usually gets run by clang even if you go straight to -emit-llvm" - if that were true, wouldn't I expect to see the spurious dbg.values you described in the output of -g -O0 -S -emit-llvm?
> >>
> >>
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
>