[PATCH] D19522: Read discriminators correctly from object file.

[David Blaikie via llvm-commits]

Oh, yes, sorry - I know that discriminators are necessary even in the
presence of column info for precisely that reason. But what I mean is, it
looks like in your test case we have column info but we are still using
discriminators, even though we don't have any duplicate locations (they're
only duplicate lines, but they all have distinct columns). so I'm wondering
if the discriminator assignment is using columns and if it isn't, why not?

On Wed, Apr 27, 2016 at 1:19 PM, Dehao Chen <danielcdh at gmail.com> wrote:

> From your comment in previous patch http://reviews.llvm.org/D14464:
>
> Even with column info, its insufficient if both calls came from the same
> macro use:
>
> #define X f(); f()
> int main() {
>
> X;
> }
>
> Both calls to 'f' will have the same line and column, the location of the X
> use.
>
>
> On Wed, Apr 27, 2016 at 11:34 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> >
> > On Tue, Apr 26, 2016 at 1:58 PM, Dehao Chen via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Unfortunately, we can't because I need to have two consecutive
> >> instructions that are both from discriminator 1, but different lines
> >> to trigger this bug. With your proposed change, the code sequence will
> >> be:
> >>
> >> line 4 discriminator 0
> >> line 4 discriminator 1
> >> line 5 discriminator 0
> >> line 5 discriminator 1
> >>
> >> But with my test case, the code sequence will be:
> >>
> >> line 4 discriminator 0
> >> line 5 discriminator 0
> >> line 4 discriminator 1
> >> line 5 discriminator 1
> >
> >
> > Ah, OK - thanks for the description (might be worth including that in the
> > test case, but I'm not sure - up to you)
> >
> > Side note: Are we not including the column info when determining when to
> add
> > discriminators? Is there a reason for that (I vaguely recall asking this
> > before, so I assume we are intentionally disregarding column info for
> some
> > good reason I've forgotten). Because these locations are all unique once
> you
> > include column info (which we are including by default now).
> >
> > - Dave
> >
> >>
> >>
> >> On Tue, Apr 26, 2016 at 12:46 PM, David Blaikie <dblaikie at gmail.com>
> >> wrote:
> >> > dblaikie added inline comments.
> >> >
> >> > ================
> >> > Comment at: test/DebugInfo/X86/discriminator2.ll:9
> >> > @@ +8,3 @@
> >> > +; #3 void baz() {
> >> > +; #4   foo/*discriminator 1*/(bar(),
> >> > +; #5       bar());bar()/*discriminator 1*/;
> >> > ----------------
> >> > Would it be simpler/sufficient to do this:
> >> >
> >> >   void f1();
> >> >   void f2() {
> >> >     f1(); f1();
> >> >     f1(); f1();
> >> >   }
> >> >
> >> > (with column info off? Or, to keep column info on, #define CALLS f1();
> >> > f1() and have the body of the function be:
> >> >   CALLS;
> >> >   CALLS;
> >> > )
> >> >
> >> >
> >> > http://reviews.llvm.org/D19522
> >> >
> >> >
> >> >
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160427/4850a415/attachment.html>