<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 27, 2016 at 3:24 PM, Paul Robinson via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">probinson added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D19567#413997" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19567#413997</a>, @dblaikie wrote:<br>
<br>
> 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?<br>
<br>
<br>
</span>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.<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> 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:<br>
<br>
><br>
<br>
> CHECK: DIGlobalVariable<br>
<br>
>  CHECK-NEXT: "name"<br>
<br>
><br>
<br>
> 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.<br>
<br>
<br>
</span>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.<br></blockquote><div><br></div><div>But that codepath is already tested by any test for a TU that has a static member without a definition, right?<br><br>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>
<br>
(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...)<br>
<span class="im HOEnZb"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D19567" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19567</a><br>
<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>