David Blaikie via llvm-dev
2016-Feb-11 21:50 UTC
[llvm-dev] [cfe-dev] Testing Best Practices/Goals (in the context of compiler-rt)
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.> 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?)> (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) 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. Most importantly, I'd expect only the latter to run in a "make check-all" run, as we do for Clang/LLVM, etc.> 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 > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160211/e5b0b09a/attachment.html>
Alexey Samsonov via llvm-dev
2016-Feb-13 01:43 UTC
[llvm-dev] [cfe-dev] Testing Best Practices/Goals (in the context of compiler-rt)
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).> > >> (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/20160212/3c476e84/attachment.html>
Sean Silva via llvm-dev
2016-Feb-13 01:48 UTC
[llvm-dev] [cfe-dev] Testing Best Practices/Goals (in the context of compiler-rt)
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). -- 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/llvm-dev/attachments/20160212/e7da7f38/attachment.html>
David Blaikie via llvm-dev
2016-Feb-17 16:45 UTC
[llvm-dev] [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>