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

David Blaikie via cfe-dev cfe-dev at lists.llvm.org
Wed Feb 17 08:46:26 PST 2016 

On Fri, Feb 12, 2016 at 5:48 PM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> On Fri, Feb 12, 2016 at 5:43 PM, Alexey Samsonov via cfe-dev <
> cfe-dev at lists.llvm.org> 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. 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.
>>
>> You may argue that Clang test would have been enough (I disagree with
>> that), or that it qualifies as "adding coverage" (maybe).
>>
>
> Yeah, verifying the intended end-user experience is important (for changes
> that are done primarily for the purpose of changing the end-user
> experience).
>

This seems like the problematic part - changes to LLVM improve the
user-facing part of Clang (& Swift and any other LLVM user), 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.


>
> -- Sean Silva
>
>
>>
>>
>>>
>>>
>>>> (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/20160217/7f7773fc/attachment.html>

More information about the cfe-dev mailing list