<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 26, 2016 at 1:11 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Feb 26, 2016 at 1:07 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Feb 17, 2016 at 8:45 AM, David Blaikie via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Feb 12, 2016 at 5:43 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Feb 11, 2016 at 1:50 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Wed, Feb 10, 2016 at 3:55 PM, Alexey Samsonov via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">I mostly agree with what Richard and Justin said. Adding a few notes about the general strategy we use:</div><div class="gmail_extra"><br></div><div class="gmail_extra">(1) lit tests which look "end-to-end" proved to be way more convenient for testing runtime libraries than unit tests. </div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">We do have</div><div class="gmail_extra">the latter, and use them to provide test coverage for utility functions, but we quite often accompany fix to the runtime library with</div><div class="gmail_extra">"end-to-end" small reproducer extracted from the real-world code that exposed the issue.</div><div class="gmail_extra">Incidentally, this tests a whole lot of other functionality: Clang driver, frontend, LLVM passes, etc, but it's not the intent of the test.</div></div></blockquote><div><br></div></span><div>Indeed - this is analogous to the tests for, say, LLD that use llvm-mc to produce the inputs rather than checking in object files. That area is open to some discussion as to just how many tools we should rope in/how isolated we should make tests (eg: maybe building the json object file format was going too far towards isolation? Not clear - opinions differ). But the point of the test is to test the compiler-rt functionality that was added/removed/modified.<br><br>I think most people are in agreement with that, while acknowledging the fuzzy line about how isolated we might be.</div></div></div></div></blockquote><div><br></div></span><div>Yes.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">These tests are sometimes platform-specific and poorly portable, but they are more reliable (we make the same steps as the</div><div class="gmail_extra">user of the compiler), and serve the purpose of documentation.</div><div class="gmail_extra"><br></div><div class="gmail_extra">(2) If we change LLVM instrumentation - we add a test to LLVM. If we change Clang code generation or driver behavior - we add</div><div class="gmail_extra">a test to Clang. No excuses here.</div><div class="gmail_extra"><br></div><div class="gmail_extra">(3) Sometimes we still add a compiler-rt test for the change in LLVM or Clang: e.g. if we enhance Clang frontend to teach UBSan</div><div class="gmail_extra">to detecting yet another kind of overflow, it makes sense to add a test to UBSan test-suite that demonstrates it, in addition to</div><div class="gmail_extra">Clang test verifying that we emit a call to UBSan runtime. Also, compiler-rt test would allow us to verify that the actual error report</div><div class="gmail_extra">we present to the user is sane.</div></div></blockquote><div><br></div></span><div>This bit ^ is a bit unclear to me. If there was no change to the UBSan runtime, and the code generated by Clang is equivalent/similar to an existing use of the UBSan runtime - what is it that the new compiler-rt test is providing? (perhaps you could give a concrete example you had in mind to look at?)</div></div></div></div></blockquote><div><br></div></span><div>See r235568 (change to Clang) followed by r235569 (change to compiler-rt test). Now, it's a cheat because I'm fixing test, not adding it. However, I would have definitely added it, if it was missing.</div></div></div></div></blockquote><div><br></div></span><div>Right, I think the difference here is "if it was missing" - the test case itself seems like it could be a reasonable one (are there other tests of the same compiler-rt functionality? (I assume the compiler-rt functionality is the implementation of sadd/ssub?))</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> In this case, a change to Clang</div><div>instrumentation (arguments passed to UBSan runtime callbacks) improved the user-facing part of the tool, and compiler-rt test suite is a good place to verify that.</div></div></div></div></blockquote><div><br></div></span><div>This seems like the problematic part - changes to LLVM improve the user-facing part of Clang, but we don't add end-to-end tests of that, as a general rule. I'm trying to understand why the difference between that and compiler-rt</div></div></div></div></blockquote><div><br></div></div></div><div>In what way do changes in LLVM change the user-facing part of Clang?</div><div>It obviously depends on how broadly one defines user-facing. Is a 1% performance improvement from a particular optimization user-facing? Is better debug info accuracy user-facing? I'm not sure. But it seems clear that "the user sees a diagnostic or not" definitely is.</div></div></div></div></blockquote><div><br></div></div></div><div>There's more than just performance in LLVM - ABI features, and yes, I'd argue some pieces of debug info are pretty user facing (as are some optimizations). We also have the remarks system in place now. (also the compiler crashing (or not) is pretty user facing).<br></div></div></div></div></blockquote><div><br></div><div>I'd argue that we probably should have some sort of integration tests for ABI features. I think at the moment we're getting by thanks to self-hosting and regularly building lots of real-world programs with ToT-ish compilers.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Also, I think part of this is that in compiler-rt there are usually more moving parts we don't control. E.g. it isn't just the interface between LLVM and clang. The information needs to pass through archivers, linkers, runtime loaders, etc. that all may have issues that affect whether the user sees the final result. In general the interface between LLVM and clang has no middlemen so there really isn't anything to check.<br></div></div></div></div></blockquote><div><br></div></span><div>Correctness/behavior of the compiler depends on those things too (linkers, loaders, etc) to produce the final working product the user requested. If we emitted symbols with the wrong linkage we could produce linker errors, drop important entities, etc. But we don't generally test that the output of LLVM/Clang produces the right binary when linked, we test that it produces the right linkages on the resulting entities.<span class=""><font color="#888888"><br><br>- David</font></span></div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>You may argue that Clang test would have been enough (I disagree with that), or that it qualifies as "adding coverage" (maybe).<br></div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">(4) True, we're intimidated by test-suite :) I feel that current use of compiler-rt test suite to check compiler-rt libs better follows<br></div><div class="gmail_extra">the doctrine described by David.</div></div></blockquote><div><br></div></span><div>Which David? ;) (I guess David Li, not me)<br></div></div></div></div></blockquote><div><br></div></span><div>Nope, paragraph 2 from your original email.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I think maybe what could be worth doing would be separating out the broader/intentionally "end to end" sort of tests from the ones intended to test compiler-rt in relative isolation. <br></div></div></div></div></blockquote><div><br></div></span><div>It's really hard to draw the line here, even some of compiler-rt unit tests require instrumentation, therefore depend on new features of Clang/LLVM. Unlike builtins, which are</div><div>trivial to test in isolation, testing sanitizer runtimes in isolation (w/o compiler) is often hard to implement (we tried to do so for TSan, but found unit tests extremely hard to write),</div><div>and is barely useful - compiler-rt runtimes don't consist of modules (like LLVMCodeGen and LLVMMC for instance), and are never used w/o the compiler anyway.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>Most importantly, I'd expect only the latter to run in a "make check-all" run, as we do for Clang/LLVM, etc.</div></div></div></div></blockquote><div><br></div></span><div>And now we're getting to the goals :) Why would such a change be good? Do you worry about the time it takes to execute the test suite?</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"> Also, there's significant complexity in compiler-rt test suite that narrows the tests executed</div><div class="gmail_extra">to those supported by current host.</div></div></blockquote><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_quote"><div><div>On Wed, Feb 10, 2016 at 2:33 PM, Xinliang David Li via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Feb 10, 2016 at 2:11 PM, Justin Bogner via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>David Blaikie via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> writes:<br>
> Recently had a bit of a digression in a review thread related to some tests<br>
> going in to compiler-rt (<br>
> <a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html</a><br>
> ) and there seems to be some disconnect at least between my expectations<br>
> and reality. So I figured I'd have a bit of a discussion out here on the<br>
> dev lists where there's a bit more visibility.<br>
><br>
> My basic expectation is that the lit tests in any LLVM project except the<br>
> test-suite are targeted tests intended to test only the functionality in<br>
> the project. This seems like a pretty well accepted doctrine across most<br>
> LLVM projects - most visibly in Clang, where we make a concerted effort not<br>
> to have tests that execute LLVM optimizations, etc.<br>
><br>
> There are exceptions/middle ground to this - DIBuilder is in LLVM, but<br>
> essentially tested in Clang rather than writing LLVM unit tests. It's<br>
> somewhat unavoidable that any of the IR building code (IRBuilder,<br>
> DIBuilder, IR asm printing, etc) is 'tested' incidentally in Clang in<br>
> process of testing Clang's IR generation. But these are seen as incidental,<br>
> not intentionally trying to cover LLVM with Clang tests (we don't add a<br>
> Clang test if we add a new feature to IRBuilder just to test the IRBuilder).<br>
><br>
> Another case with some middle ground are things like linker tests and<br>
> objdump, dwarfdump, etc - in theory to isolate the test we would checkin<br>
> binaries (or the textual object representation lld had for a while, etc) to<br>
> test those tools. Some tests instead checkin assembly and assemble it with<br>
> llvm-mc. Again, not to cover llvm-mc, but on the assumption that llvm-mc is<br>
> tested, and just using it as a tool to make tests easier to maintain.<br>
><br>
> So I was surprised to find that the compiler-rt lit tests seem to diverge<br>
> from this philosophy & contain more intentional end-to-end tests (eg:<br>
> adding a test there when making a fix to Clang to add a counter to a<br>
> function that was otherwise missing a counter - I'd expect that to be<br>
> tested in Clang and that there would already be coverage in compiler-rt for<br>
> "if a function has a counter, does compiler-rt do the right thing with that<br>
> counter" (testing whatever code in compiler-rt needs to be tested)).<br>
><br>
> Am I off base here? Are compiler-rt's tests fundamentally different to the<br>
> rest of the LLVM project? Why? Should they continue to be?<br>
<br>
</div></div>I think there's a bit of grey area in terms testing the runtime -<br>
generally it's pretty hard to use the runtime without a fairly<br>
end-to-end test, so tests of the runtime often end up looking pretty<br>
close to an end-to-end test.<br>
<br>
That said, I don't think that should be used as an excuse to sneak<br>
arbitrary end-to-end tests into compiler-rt. We should absolutely write<br>
tests in clang and llvm that we're inputting what we expect to the<br>
runtime and try to keep the tests in compiler-rt as focused on just<br>
exercising the runtime code as possible.<br></blockquote><div><br></div></div></div><div>Yes, we should not use compiler-rt tests as an excuse of not adding clang/LLVM test. The latter should always be added if possible -- they are platform independent and is the first level of defense.  runtime test's focus is also more on the runtime lib itself and interaction between  runtime, compiler, binutils and other tools.  </div><div><br></div><div>David</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span>
<br>
IIUC, the correct place for integration tests in general is somewhere<br>
like test-suite, but I think it's a bit intimidating to some people to<br>
add new tests there (Are there docs on this?). I suspect some of the<br>
profiling related tests in compiler-rt are doing a bit much and should<br>
graduate to a spot in the test-suite (but I don't have time to volunteer<br>
to do the work, unfortunately).<br></span>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div><br></div></div>
<br></div></div>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</font></span></div></div>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</font></span></div></div>
</blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>