On Wed, Apr 27, 2016 at 3:24 PM, Paul Robinson via cfe-commits < cfe-commits@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
