[llvm-dev] [RFC] Generate Debug Information for Labels in Function

[via llvm-dev]

> -----Original Message-----
> From: Hsiangkai Wang [mailto:hsiangkai at gmail.com]
> Sent: Sunday, April 01, 2018 12:12 PM
> To: Reid Kleckner
> Cc: Adrian Prantl; llvm-dev; Robinson, Paul; David Blaikie; Eric
> Christopher
> Subject: Re: [llvm-dev] [RFC] Generate Debug Information for Labels in
> Function
> 
> Hi all,
> 
> I am sorry that I didn’t carefully think about how to handle labels in
> inlined function.
> 
> Today, I did some simple experiments and found that some basic block
> may be removed in CFGSimplifyPass and the attached metadata will be
> eliminated. So, if the optimization is turned on, label metadata will
> disappear. However, intrinsic will keep existing in the function if we
> handle it correctly. So, if we take care about optimization and
> inlining, to keep label metadata through intrinsic could be right for
> free.

It makes sense that basic blocks might be manipulated (split, merged, etc)
in ways such that it would require a lot of attention to preserve metadata 
correctly.  An intrinsic is harder to lose. :-)   I suppose if the entire 
basic block is optimized away, the label is really dead, and in that case 
it is okay to remove the intrinsic.

> 
> I learned a lot from discussions. Thanks for your comments and
> suggestions. If there is no other concern, I will keep my original
> implementation and send patches to Phabricator again. Sorry have
> bothered you.

No trouble.  This is what discussions are for; they have value, even if
the conclusion is "it was right the first time."

> 
> Any comments?

The previous patch set looked reasonable, however please post patches
in testable units, with their tests in the same review.  This helps
reviewers to understand what you are doing, and makes it easier to see
whether there are missing pieces.
Thanks!
--paulr

> 
> On Sat, Mar 31, 2018 at 5:38 AM, Reid Kleckner <rnk at google.com> wrote:
> > On Fri, Mar 30, 2018 at 9:39 AM Adrian Prantl <aprantl at apple.com> wrote:
> >>
> >> I'm really sorry for not realizing this yesterday, but the problems
> >> pertaining to inlining made me realize that your original design with
> the
> >> dbg.label intrinsic might actually be a better approach especially when
> >> considering optimized code. We will get inlining support for free
> because it
> >> is just another instruction and it can deal with more than one label at
> the
> >> same address. It looks a bit more complicated in unoptimized code, but
> that
> >> seems like a small price to pay.
> >>
> >> We just need to make sure that the backend doesn't get confused when
> loop
> >> unrolling duplicates a dbg.label but that should be doable.
> >
> >
> > +10, it should be an intrinsic.