[PATCH] D19567: PR21823: 'nodebug' attribute on global/static variables

[David Blaikie via cfe-commits]

On Wed, Apr 27, 2016 at 3:24 PM, Paul Robinson via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> probinson added a comment.
>
> In http://reviews.llvm.org/D19567#413997, @dblaikie wrote:
>
> > For 3 code paths (that seem fairly independent from one another) I'd
> only really expect to see 3 variables in the test - one to exercise each
> codepath. What's the reason for the larger set of test cases?
>
>
> I'm not interested in testing code-paths, I'm interested in testing a
> feature, and relying on the fact that (at the moment) only 3 code paths are
> involved is not really robust.  Granted there is some duplication, and I
> took that out (r267804), but there are more than 3 relevant cases from an
> end-user-feature perspective.
>

We generally have to assume /something/ about the implementation to
constrain testing. eg: we don't assume that putting a variable inside a
namespace would make a difference here so we don't go & test all these case
inside a namespace as well as in the global namespace. So I'm not sure why
we wouldn't go further down that path, knowing that there are only a small
number of ways global variable definitions can impact debug info generation.


>
> > Then it might be simpler just to include 6 variables, one of each kind
> with the attribute, one of each kind without. & I think you could check
> this reliably with:
>
> >
>
> > CHECK: DIGlobalVariable
>
> >  CHECK-NEXT: "name"
>
> >
>
> > repeated three times with a CHECK-NOT: DIGlobalVariable at the end -
> that way the CHECK wouldn't skip past an existing variable, and you'd check
> you got the right ones. I think.
>
>
> If I cared only about DIGlobalVariable that does sound like it would
> work.  Sadly, the static const data member comes out as a DIDerivedType,
> not a DIGlobalVariable.  Also, it's *really* important to us that the
> DICompositeType for "S1" is not present; that's actually where we get the
> major benefit.
>

But that codepath is already tested by any test for a TU that has a static
member without a definition, right?

As for the composite type - this is much like testing in LLVM where we test
that a certain inlining occurred when we test the inliner - not the
knock-on effect that that has on some other optimization which we've
already tested separately.


> Once you have more than one kind of thing to look for (or -NOT look for)
> it gets a lot more tedious to show it's all correct in one pass like you
> are suggesting.
>
> (Re. getting rid of the types:  We have licensees with major template
> metaprogramming going on, but if they can mark those variables 'nodebug'
> and consequently suppress the instantiated types from the debug info, they
> should see a major size win.  One licensee reported that reducing some name
> from 19 characters to 1 char got them a 5MB reduction. Imagine if they
> could get rid of all of those types...)
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D19567
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160427/d47b8a84/attachment.html>