[cfe-dev] [llvm-dev] Testing Best Practices/Goals (in the context of compiler-rt)

[David Blaikie via cfe-dev]

On Fri, Feb 26, 2016 at 1:31 PM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> On Fri, Feb 26, 2016 at 1:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Fri, Feb 26, 2016 at 1:07 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Wed, Feb 17, 2016 at 8:45 AM, David Blaikie via cfe-dev <
>>> cfe-dev at lists.llvm.org> wrote:
>>>
>>>>
>>>>
>>>> On Fri, Feb 12, 2016 at 5:43 PM, Alexey Samsonov <vonosmas at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, Feb 11, 2016 at 1:50 PM, David Blaikie <dblaikie at gmail.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Feb 10, 2016 at 3:55 PM, Alexey Samsonov via cfe-dev <
>>>>>> cfe-dev at lists.llvm.org> wrote:
>>>>>>
>>>>>>> I mostly agree with what Richard and Justin said. Adding a few notes
>>>>>>> about the general strategy we use:
>>>>>>>
>>>>>>> (1) lit tests which look "end-to-end" proved to be way more
>>>>>>> convenient for testing runtime libraries than unit tests.
>>>>>>>
>>>>>> We do have
>>>>>>> the latter, and use them to provide test coverage for utility
>>>>>>> functions, but we quite often accompany fix to the runtime library with
>>>>>>> "end-to-end" small reproducer extracted from the real-world code
>>>>>>> that exposed the issue.
>>>>>>> Incidentally, this tests a whole lot of other functionality: Clang
>>>>>>> driver, frontend, LLVM passes, etc, but it's not the intent of the test.
>>>>>>>
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> I think most people are in agreement with that, while acknowledging
>>>>>> the fuzzy line about how isolated we might be.
>>>>>>
>>>>>
>>>>> Yes.
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>> These tests are sometimes platform-specific and poorly portable, but
>>>>>>> they are more reliable (we make the same steps as the
>>>>>>> user of the compiler), and serve the purpose of documentation.
>>>>>>>
>>>>>>> (2) If we change LLVM instrumentation - we add a test to LLVM. If we
>>>>>>> change Clang code generation or driver behavior - we add
>>>>>>> a test to Clang. No excuses here.
>>>>>>>
>>>>>>> (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
>>>>>>> to detecting yet another kind of overflow, it makes sense to add a
>>>>>>> test to UBSan test-suite that demonstrates it, in addition to
>>>>>>> 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
>>>>>>> we present to the user is sane.
>>>>>>>
>>>>>>
>>>>>> 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?)
>>>>>>
>>>>>
>>>>> 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.
>>>>>
>>>>
>>>> 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?))
>>>>
>>>>
>>>>> In this case, a change to Clang
>>>>> 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.
>>>>>
>>>>
>>>> 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
>>>>
>>>
>>> In what way do changes in LLVM change the user-facing part of Clang?
>>> 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.
>>>
>>
>> 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).
>>
>
> 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.
>

Perhaps so, but I'd argue that they shouldn't be run as part of "make
check" & should be in a separate test grouping (probably mostly run by
buildbots) for the purpose of integration testing. We've made a pretty
conscious, deliberate, and consistent effort to not do integration testing
across the LLVM projects in "make check"-like testing, and to fix it where
we do find it. It seems to me that compiler-rt diverged from that approach
and I'm not really in favor of that divergence.

- David


>
> -- Sean Silva
>
>
>>
>>
>>> 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.
>>>
>>
>> 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.
>>
>> - David
>>
>>
>>>
>>> -- Sean Silva
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>
>>>>> You may argue that Clang test would have been enough (I disagree with
>>>>> that), or that it qualifies as "adding coverage" (maybe).
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>> (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
>>>>>>> the doctrine described by David.
>>>>>>>
>>>>>>
>>>>>> Which David? ;) (I guess David Li, not me)
>>>>>>
>>>>>
>>>>> Nope, paragraph 2 from your original email.
>>>>>
>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> 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
>>>>> 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),
>>>>> 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.
>>>>>
>>>>>
>>>>>>
>>>>>> Most importantly, I'd expect only the latter to run in a "make
>>>>>> check-all" run, as we do for Clang/LLVM, etc.
>>>>>>
>>>>>
>>>>> 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?
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>> Also, there's significant complexity in compiler-rt test suite that
>>>>>>> narrows the tests executed
>>>>>>> to those supported by current host.
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> On Wed, Feb 10, 2016 at 2:33 PM, Xinliang David Li via cfe-dev <
>>>>>>> cfe-dev at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Feb 10, 2016 at 2:11 PM, Justin Bogner via llvm-dev <
>>>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>>>
>>>>>>>>> David Blaikie via cfe-dev <cfe-dev at lists.llvm.org> writes:
>>>>>>>>> > Recently had a bit of a digression in a review thread related to
>>>>>>>>> some tests
>>>>>>>>> > going in to compiler-rt (
>>>>>>>>> >
>>>>>>>>> http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160208/330759.html
>>>>>>>>> > ) and there seems to be some disconnect at least between my
>>>>>>>>> expectations
>>>>>>>>> > and reality. So I figured I'd have a bit of a discussion out
>>>>>>>>> here on the
>>>>>>>>> > dev lists where there's a bit more visibility.
>>>>>>>>> >
>>>>>>>>> > My basic expectation is that the lit tests in any LLVM project
>>>>>>>>> except the
>>>>>>>>> > test-suite are targeted tests intended to test only the
>>>>>>>>> functionality in
>>>>>>>>> > the project. This seems like a pretty well accepted doctrine
>>>>>>>>> across most
>>>>>>>>> > LLVM projects - most visibly in Clang, where we make a concerted
>>>>>>>>> effort not
>>>>>>>>> > to have tests that execute LLVM optimizations, etc.
>>>>>>>>> >
>>>>>>>>> > There are exceptions/middle ground to this - DIBuilder is in
>>>>>>>>> LLVM, but
>>>>>>>>> > essentially tested in Clang rather than writing LLVM unit tests.
>>>>>>>>> It's
>>>>>>>>> > somewhat unavoidable that any of the IR building code (IRBuilder,
>>>>>>>>> > DIBuilder, IR asm printing, etc) is 'tested' incidentally in
>>>>>>>>> Clang in
>>>>>>>>> > process of testing Clang's IR generation. But these are seen as
>>>>>>>>> incidental,
>>>>>>>>> > not intentionally trying to cover LLVM with Clang tests (we
>>>>>>>>> don't add a
>>>>>>>>> > Clang test if we add a new feature to IRBuilder just to test the
>>>>>>>>> IRBuilder).
>>>>>>>>> >
>>>>>>>>> > Another case with some middle ground are things like linker
>>>>>>>>> tests and
>>>>>>>>> > objdump, dwarfdump, etc - in theory to isolate the test we would
>>>>>>>>> checkin
>>>>>>>>> > binaries (or the textual object representation lld had for a
>>>>>>>>> while, etc) to
>>>>>>>>> > test those tools. Some tests instead checkin assembly and
>>>>>>>>> assemble it with
>>>>>>>>> > llvm-mc. Again, not to cover llvm-mc, but on the assumption that
>>>>>>>>> llvm-mc is
>>>>>>>>> > tested, and just using it as a tool to make tests easier to
>>>>>>>>> maintain.
>>>>>>>>> >
>>>>>>>>> > So I was surprised to find that the compiler-rt lit tests seem
>>>>>>>>> to diverge
>>>>>>>>> > from this philosophy & contain more intentional end-to-end tests
>>>>>>>>> (eg:
>>>>>>>>> > adding a test there when making a fix to Clang to add a counter
>>>>>>>>> to a
>>>>>>>>> > function that was otherwise missing a counter - I'd expect that
>>>>>>>>> to be
>>>>>>>>> > tested in Clang and that there would already be coverage in
>>>>>>>>> compiler-rt for
>>>>>>>>> > "if a function has a counter, does compiler-rt do the right
>>>>>>>>> thing with that
>>>>>>>>> > counter" (testing whatever code in compiler-rt needs to be
>>>>>>>>> tested)).
>>>>>>>>> >
>>>>>>>>> > Am I off base here? Are compiler-rt's tests fundamentally
>>>>>>>>> different to the
>>>>>>>>> > rest of the LLVM project? Why? Should they continue to be?
>>>>>>>>>
>>>>>>>>> I think there's a bit of grey area in terms testing the runtime -
>>>>>>>>> generally it's pretty hard to use the runtime without a fairly
>>>>>>>>> end-to-end test, so tests of the runtime often end up looking
>>>>>>>>> pretty
>>>>>>>>> close to an end-to-end test.
>>>>>>>>>
>>>>>>>>> That said, I don't think that should be used as an excuse to sneak
>>>>>>>>> arbitrary end-to-end tests into compiler-rt. We should absolutely
>>>>>>>>> write
>>>>>>>>> tests in clang and llvm that we're inputting what we expect to the
>>>>>>>>> runtime and try to keep the tests in compiler-rt as focused on just
>>>>>>>>> exercising the runtime code as possible.
>>>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>>>
>>>>>>>>> IIUC, the correct place for integration tests in general is
>>>>>>>>> somewhere
>>>>>>>>> like test-suite, but I think it's a bit intimidating to some
>>>>>>>>> people to
>>>>>>>>> add new tests there (Are there docs on this?). I suspect some of
>>>>>>>>> the
>>>>>>>>> profiling related tests in compiler-rt are doing a bit much and
>>>>>>>>> should
>>>>>>>>> graduate to a spot in the test-suite (but I don't have time to
>>>>>>>>> volunteer
>>>>>>>>> to do the work, unfortunately).
>>>>>>>>> _______________________________________________
>>>>>>>>> LLVM Developers mailing list
>>>>>>>>> llvm-dev at lists.llvm.org
>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> cfe-dev mailing list
>>>>>>>> cfe-dev at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Alexey Samsonov
>>>>>>> vonosmas at gmail.com
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> cfe-dev mailing list
>>>>>>> cfe-dev at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Alexey Samsonov
>>>>> vonosmas at gmail.com
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160226/c6feebdf/attachment.html>