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

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

>
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160217/dafc4a89/attachment-0001.html>

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.

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.

-- 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/llvm-dev/attachments/20160226/a9c90d3d/attachment-0001.html>

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).

> 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/llvm-dev/attachments/20160226/c90e2132/attachment.html>

Sorry, lost track of this thread.

On Wed, Feb 17, 2016 at 8:45 AM, David Blaikie <dblaikie at gmail.com>
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?))
>
No, that's the point - sadd/ssub is handled solely in Clang/LLVM, and UBSan
runtime is only responsible for reporting an already-detected issue. From
this POV you can actually consider majority of UBSan lit tests
"end-to-end": they check that driver understands -fsanitize= flags,
necessary instrumentation is generated, callbacks into UBSan runtime are
invoked at runtime, and they print properly formatted message.

>
>
>> 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
>
I would say the difference is the information we print to the screen :)
Normally, Clang prints nothing to stdout or stderr. If it prints compiler
warnings/errors, header search paths, etc., this behavior is coded in the
Clang itself, and doesn't depend on the LLVM layer.
The major goal of sanitizers (not all of compiler-rt, for sure) is to
provide information in a user-friendly way, and it's actively influenced by
Clang and/or LLVM layers.

These printed reports is something we can check against really
conveniently. True, Clang can be "improved" by, say, a change in LLVM
inliner, but how would you write a robust test for that doesn't inspect
implementation details (which the whole LLVM IR is, to some extent)? OTOH,
if a change to LLVM gained ASan new capabilities, we can easily verify that
with a snippet of C++ code.

>
>
>>
>>
>
>> 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
>>
>
>

-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160226/08038561/attachment.html>

On Fri, Feb 26, 2016 at 1:21 PM, Alexey Samsonov <vonosmas at gmail.com>
wrote:
> Sorry, lost track of this thread.
>
No worries :)

>
> On Wed, Feb 17, 2016 at 8:45 AM, David Blaikie <dblaikie at
gmail.com> 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?))
>>
>
> No, that's the point - sadd/ssub is handled solely in Clang/LLVM, and
> UBSan runtime is only responsible for reporting an already-detected issue.
> From this POV you can actually consider majority of UBSan lit tests
> "end-to-end": they check that driver understands -fsanitize=
flags,
> necessary instrumentation is generated, callbacks into UBSan runtime are
> invoked at runtime, and they print properly formatted message.
>
OK, then I'm not sure of the value in having multiple compiler-rt tests
here, for each different place where Clang/LLVM call into UBSan to print a
different diagnostic.

I would expect to see a single test, that demonstrates that UBSan can print
diagnostics, and exercise whatever options there are (that it supports
parameters, location information, etc) but not one for each diagnostic
Clang uses that infrastructure for. That seems repetitious/low value, no?

>
>
>>
>>
>>> 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
>>
>
> I would say the difference is the information we print to the screen :)
> Normally, Clang prints nothing to stdout or stderr. If it prints compiler
> warnings/errors, header search paths, etc., this behavior is coded in the
> Clang itself, and doesn't depend on the LLVM layer.
> The major goal of sanitizers (not all of compiler-rt, for sure) is to
> provide information in a user-friendly way, and it's actively
influenced by
> Clang and/or LLVM layers.
>
> These printed reports is something we can check against really
> conveniently. True, Clang can be "improved" by, say, a change in
LLVM
> inliner, but how would you write a robust test for that doesn't inspect
> implementation details (which the whole LLVM IR is, to some extent)?
>
Would be quite easy - indeed lots of tests in Clang in the early days (some
of which still remain) do things like this - testing all the way through
Clang and LLVM to observe certain assembly output. A simple way to test the
inliner would be to nm the symbols at the other end and demonstrate that
the inlined function is no longer emitted. This wouldn't require linkers,
etc, even. But even though that's possible, it's something we go out of
our
way not to do when testing Clang and LLVM.

> OTOH, if a change to LLVM gained ASan new capabilities, we can easily
> verify that with a snippet of C++ code.
>
>
>>
>>
>>>
>>>
>>
>>> 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
>>>
>>
>>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160226/24f73337/attachment.html>