[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
Fri Feb 26 12:31:20 PST 2016
Ping
On Wed, Feb 17, 2016 at 8:46 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> 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/20160226/b71dd72f/attachment.html>
More information about the cfe-dev
mailing list