Alina Sbirlea via llvm-dev
2016-Feb-29 19:16 UTC
[llvm-dev] RFC: Add bitcode tests to test-suite
All, To get the discussion going in a focused manner, here is an initial patch with a running test. The test is from the Halide suite and is checking the correctness of several simd operations. (Notes: the patch is large due to the number of operations being tested; I expect a lot of changes before actually landing it, this is simply to continue the discussion using a concrete example.) http://reviews.llvm.org/D17726 A few questions/todos to start the discussion: 1. What is a good location for these tests? They are in a separate Bitcode directory atm, but using the llvm_multisource. This may change to more closely model the approach for external tests (see next item). 2. There is a single .cpp file testing all operations provided by individual bitcode files. I expect this to change. Instead of using llvm_multisource to have the same test run with specific arguments, each run testing a single operation. 3. The building approach I took is to first link all bitcode files into a single one, then obtain the assembly for it, which cmake knows to take as an input source. A separate discussion is on reading metadata (mcpu and mattr) in llc. I added a script to work around that for now. Looking forward to your feedback! Thanks, Alina On Fri, Feb 19, 2016 at 6:50 AM, Kristof Beyls <kristof.beyls at arm.com> wrote:> > > On 18/02/2016 19:12, Alina Sbirlea via llvm-dev wrote: > > > >> I have more questions for Alina. What kind of tests do you have: >> >> - "the compiler takes the bitcode and generates code without crashing" >> - "the compiled test runs without crashing" >> - "the compiled test will produce an output that be checked against a >> reference" >> - "the compiled test is meaningful as a benchmarks" >> > > We have all 4 kinds of tests in Halide. The bitcode files for the first > category is already available and I'm working on building the ones for the > next 3. We'd like to include all incrementally. > > > It seems to me that the first category ("the compiler takes the bitcode > and generates code without crashing") are tests that should be added to the > "make check-all" tests in the LLVM subproject, rather than the test-suite > subproject? > Or if these tests currently don't crash the compiler anymore, the bugs > must have been fixed, and there should already be equivalent tests? >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160229/1c27123d/attachment.html>
Mehdi Amini via llvm-dev
2016-Feb-29 19:40 UTC
[llvm-dev] RFC: Add bitcode tests to test-suite
> On Feb 29, 2016, at 11:16 AM, Alina Sbirlea via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > All, > > To get the discussion going in a focused manner, here is an initial patch with a running test. The test is from the Halide suite and is checking the correctness of several simd operations. > (Notes: the patch is large due to the number of operations being tested; > I expect a lot of changes before actually landing it, this is simply to continue the discussion using a concrete example.) > http://reviews.llvm.org/D17726 <http://reviews.llvm.org/D17726>I can't figure how to download the patch *with the bitcode files* from Phabricator. Can you push this on github (or somewhere else)? (or if I missed how to proceed...). Thanks, Mehdi> > A few questions/todos to start the discussion: > 1. What is a good location for these tests? They are in a separate Bitcode directory atm, but using the llvm_multisource. This may change to more closely model the approach for external tests (see next item). > 2. There is a single .cpp file testing all operations provided by individual bitcode files. I expect this to change. Instead of using llvm_multisource to have the same test run with specific arguments, each run testing a single operation. > 3. The building approach I took is to first link all bitcode files into a single one, then obtain the assembly for it, which cmake knows to take as an input source. > A separate discussion is on reading metadata (mcpu and mattr) in llc. I added a script to work around that for now. > > Looking forward to your feedback! > > Thanks, > Alina > > > > On Fri, Feb 19, 2016 at 6:50 AM, Kristof Beyls <kristof.beyls at arm.com <mailto:kristof.beyls at arm.com>> wrote: > > > On 18/02/2016 19:12, Alina Sbirlea via llvm-dev wrote: >> >> >> I have more questions for Alina. What kind of tests do you have: >> >> - "the compiler takes the bitcode and generates code without crashing" >> - "the compiled test runs without crashing" >> - "the compiled test will produce an output that be checked against a reference" >> - "the compiled test is meaningful as a benchmarks" >> >> We have all 4 kinds of tests in Halide. The bitcode files for the first category is already available and I'm working on building the ones for the next 3. We'd like to include all incrementally. >> >> > It seems to me that the first category ("the compiler takes the bitcode and generates code without crashing") are tests that should be added to the "make check-all" tests in the LLVM subproject, rather than the test-suite subproject? > Or if these tests currently don't crash the compiler anymore, the bugs must have been fixed, and there should already be equivalent tests? > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160229/57022c8c/attachment.html>
Alina Sbirlea via llvm-dev
2016-Feb-29 19:55 UTC
[llvm-dev] RFC: Add bitcode tests to test-suite
New to arc, I don't know how to help with downloading the patch. Github alternative (the top-level cmake changes are missing, there are just 2 modified lines, please pull that from Phabricator): https://github.com/alinas/Bitcode Thanks, Alina On Mon, Feb 29, 2016 at 11:40 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:> > On Feb 29, 2016, at 11:16 AM, Alina Sbirlea via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > All, > > To get the discussion going in a focused manner, here is an initial patch > with a running test. The test is from the Halide suite and is checking the > correctness of several simd operations. > (Notes: the patch is large due to the number of operations being tested; > I expect a lot of changes before actually landing it, this is simply to > continue the discussion using a concrete example.) > http://reviews.llvm.org/D17726 > > > I can't figure how to download the patch *with the bitcode files* from > Phabricator. Can you push this on github (or somewhere else)? (or if I > missed how to proceed...). > > Thanks, > > Mehdi > > > > > A few questions/todos to start the discussion: > 1. What is a good location for these tests? They are in a separate Bitcode > directory atm, but using the llvm_multisource. This may change to more > closely model the approach for external tests (see next item). > 2. There is a single .cpp file testing all operations provided by > individual bitcode files. I expect this to change. Instead of using > llvm_multisource to have the same test run with specific arguments, each > run testing a single operation. > 3. The building approach I took is to first link all bitcode files into a > single one, then obtain the assembly for it, which cmake knows to take as > an input source. > A separate discussion is on reading metadata (mcpu and mattr) in llc. I > added a script to work around that for now. > > Looking forward to your feedback! > > Thanks, > Alina > > > > On Fri, Feb 19, 2016 at 6:50 AM, Kristof Beyls <kristof.beyls at arm.com> > wrote: > >> >> >> On 18/02/2016 19:12, Alina Sbirlea via llvm-dev wrote: >> >> >> >>> I have more questions for Alina. What kind of tests do you have: >>> >>> - "the compiler takes the bitcode and generates code without crashing" >>> - "the compiled test runs without crashing" >>> - "the compiled test will produce an output that be checked against a >>> reference" >>> - "the compiled test is meaningful as a benchmarks" >>> >> >> We have all 4 kinds of tests in Halide. The bitcode files for the first >> category is already available and I'm working on building the ones for the >> next 3. We'd like to include all incrementally. >> >> >> It seems to me that the first category ("the compiler takes the bitcode >> and generates code without crashing") are tests that should be added to the >> "make check-all" tests in the LLVM subproject, rather than the test-suite >> subproject? >> Or if these tests currently don't crash the compiler anymore, the bugs >> must have been fixed, and there should already be equivalent tests? >> > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160229/e132154c/attachment.html>
Mehdi Amini via llvm-dev
2016-Feb-29 20:18 UTC
[llvm-dev] RFC: Add bitcode tests to test-suite
> On Feb 29, 2016, at 11:40 AM, Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org> wrote: > >> >> On Feb 29, 2016, at 11:16 AM, Alina Sbirlea via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> All, >> >> To get the discussion going in a focused manner, here is an initial patch with a running test. The test is from the Halide suite and is checking the correctness of several simd operations. >> (Notes: the patch is large due to the number of operations being tested; >> I expect a lot of changes before actually landing it, this is simply to continue the discussion using a concrete example.) >> http://reviews.llvm.org/D17726 <http://reviews.llvm.org/D17726> > > I can't figure how to download the patch *with the bitcode files* from Phabricator. Can you push this on github (or somewhere else)? (or if I missed how to proceed...).I was able to figure how get them "one by one", it would still be more convenient to have an archive or a repo to clone somewhere.>> A few questions/todos to start the discussion: >> 1. What is a good location for these tests? They are in a separate Bitcode directory atm, but using the llvm_multisource. This may change to more closely model the approach for external tests (see next item).A good location would be their own external repository IMO :)>> 2. There is a single .cpp file testing all operations provided by individual bitcode files. I expect this to change. Instead of using llvm_multisource to have the same test run with specific arguments, each run testing a single operation. >> 3. The building approach I took is to first link all bitcode files into a single one, then obtain the assembly for it, which cmake knows to take as an input source.Yeah, so I'd rather have a split-build model, with a split execution model. Having a gigantic bitcode file to debug an issue is not friendly. I'd expect to have a .cpp file that contains the main and the logic to run test, and then every test that is linked-in to be executed, a bit like gtests is doing (there are multiple registering mechanisms that would avoid to declare explicitly a test in the header). -> filters.h and filter_headers.h should just go away. Also on the test in general: we should have an idea for each test what it is doing and how. I was expecting your tests to be on the pattern of having an implementation in C++ and an implementation in Halide bitcode of a filters (or whatever) and run both on random data and verifies that the result is matching. Unfortunately from what I can see you are feeding the tests with random data, and the tests are "blackboxes" that set an error flag if they detect an error. This is not super robust: the compiler can mess with the error checking and eliminate it for instance, making any error undetected. Also, just looking quickly at one IR I'm surprised by things like: "assert succeeded165": ; preds = %"assert succeeded146" %buf_host181 = getelementptr inbounds %struct.buffer_t, %struct.buffer_t* %error_op_pcmpeqq_272.buffer, i64 0, i32 1 %23 = bitcast i8** %buf_host181 to double** %error_op_pcmpeqq_272.host226227232 = load double*, double** %23, align 8 %24 = icmp eq %struct.buffer_t* %error_op_pcmpeqq_272.buffer, null br i1 %24, label %"assert failed183", label %"assert succeeded184", !prof !4 Here you have as check for nullptr at %24, but you already loaded %error_op_pcmpeqq_272.host226227232 from this pointer just before!>> A separate discussion is on reading metadata (mcpu and mattr) in llc. I added a script to work around that for now.The generic way of doing it in llvm is (I think) to use function attributes: attributes #0 = { "target-cpu"="x86-64" "target-features"="+avx2" } You shouldn't need it on the command line I think? -- Mehdi>> >> Looking forward to your feedback! >> >> Thanks, >> Alina >> >> >> >> On Fri, Feb 19, 2016 at 6:50 AM, Kristof Beyls <kristof.beyls at arm.com <mailto:kristof.beyls at arm.com>> wrote: >> >> >> On 18/02/2016 19:12, Alina Sbirlea via llvm-dev wrote: >>> >>> >>> I have more questions for Alina. What kind of tests do you have: >>> >>> - "the compiler takes the bitcode and generates code without crashing" >>> - "the compiled test runs without crashing" >>> - "the compiled test will produce an output that be checked against a reference" >>> - "the compiled test is meaningful as a benchmarks" >>> >>> We have all 4 kinds of tests in Halide. The bitcode files for the first category is already available and I'm working on building the ones for the next 3. We'd like to include all incrementally. >>> >>> >> It seems to me that the first category ("the compiler takes the bitcode and generates code without crashing") are tests that should be added to the "make check-all" tests in the LLVM subproject, rather than the test-suite subproject? >> Or if these tests currently don't crash the compiler anymore, the bugs must have been fixed, and there should already be equivalent tests? >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160229/9d99f8af/attachment.html>
Hal Finkel via llvm-dev
2016-Mar-01 06:39 UTC
[llvm-dev] RFC: Add bitcode tests to test-suite
----- Original Message -----> From: "Alina Sbirlea via llvm-dev" <llvm-dev at lists.llvm.org> > To: "llvm-dev" <llvm-dev at lists.llvm.org> > Sent: Monday, February 29, 2016 1:16:54 PM > Subject: Re: [llvm-dev] RFC: Add bitcode tests to test-suite> All,> To get the discussion going in a focused manner, here is an initial > patch with a running test. The test is from the Halide suite and is > checking the correctness of several simd operations. > (Notes: the patch is large due to the number of operations being > tested; > I expect a lot of changes before actually landing it, this is simply > to continue the discussion using a concrete example.)> http://reviews.llvm.org/D17726> A few questions/todos to start the discussion: > 1. What is a good location for these tests? They are in a separate > Bitcode directory atm, but using the llvm_multisource. This may > change to more closely model the approach for external tests (see > next item). > 2. There is a single .cpp file testing all operations provided by > individual bitcode files. I expect this to change. Instead of using > llvm_multisource to have the same test run with specific arguments, > each run testing a single operation.Thanks for continuing to work on this. Having drivers for these test bitcode files makes this significantly more appealing. This allows us to check that a crash fix was actually functionally correct, in addition to generally giving us more functional coverage. -Hal> 3. The building approach I took is to first link all bitcode files > into a single one, then obtain the assembly for it, which cmake > knows to take as an input source. > A separate discussion is on reading metadata (mcpu and mattr) in llc. > I added a script to work around that for now.> Looking forward to your feedback!> Thanks,> Alina> On Fri, Feb 19, 2016 at 6:50 AM, Kristof Beyls < > kristof.beyls at arm.com > wrote:> > On 18/02/2016 19:12, Alina Sbirlea via llvm-dev wrote: >> > > > I have more questions for Alina. What kind of tests do you > > > > have: > > > > > >> > > > - "the compiler takes the bitcode and generates code without > > > > crashing" > > > > > > > > > > - "the compiled test runs without crashing" > > > > > > > > > > - "the compiled test will produce an output that be checked > > > > against > > > > a > > > > reference" > > > > > > > > > > - "the compiled test is meaningful as a benchmarks" > > > > > >> > > We have all 4 kinds of tests in Halide. The bitcode files for the > > > first category is already available and I'm working on building > > > the > > > ones for the next 3. We'd like to include all incrementally. > > >> > It seems to me that the first category ("the compiler takes the > > bitcode and generates code without crashing") are tests that should > > be added to the "make check-all" tests in the LLVM subproject, > > rather than the test-suite subproject? > > > Or if these tests currently don't crash the compiler anymore, the > > bugs must have been fixed, and there should already be equivalent > > tests? >> _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160301/b55a1bb9/attachment.html>