[compiler-rt] r260041 - Add coverage tests (defaulted constructors/destructor)

Wed Feb 10 00:00:45 PST 2016

On Tue, Feb 9, 2016 at 5:44 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Tue, Feb 9, 2016 at 5:38 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
>>
>>
>> On Tue, Feb 9, 2016 at 5:09 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>>
>>> On Tue, Feb 9, 2016 at 5:06 PM, Alexey Samsonov <vonosmas at gmail.com>
>>> wrote:
>>>>
>>>>
>>>> On Mon, Feb 8, 2016 at 11:08 AM, David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Feb 8, 2016 at 10:54 AM, Xinliang David Li via llvm-commits
>>>>> <llvm-commits at lists.llvm.org> wrote:
>>>>>>
>>>>>> On Mon, Feb 8, 2016 at 10:46 AM, Vedant Kumar <vsk at apple.com> wrote:
>>>>>> > Could this be merged with the test in r260021?
>>>>>>
>>>>>> In theory 260021 can be merged into this one ( end-to-end test)
>>>>>
>>>>>
>>>>> We don't generally do end-to-end tests in the LIT repo. I realize
>>>>> compiler-rt is a bit different, but I think this general attitude still
>>>>> holds.
>>>>>
>>>>> Alexey (for want of any better person to consult - feel free to punt/CC
>>>>> others): what's the general philosophy on tests that go into compiler-rt's
>>>>> lit suite?
>>>>
>>>>
>>>> compiler-rt lit test suite is tailored to running "end-to-end" tests:
>>>> using just-built Clang to build an executable, run this executable,
>>>> and inspect its output somehow (via FileCheck, or other tools).
>>>
>>>
>>> Right - but I'm wondering what sort of things do you generally test
>>> end-to-end in compiler-rt.
>>>
>>> Take this change, for example - that adds profiling counters to another
>>> case in Clang. The IR looks identical to any other function with counters,
>>> so it's not adding more IR diversity to the coverage.
>>>
>>> I'm just not sure how much end-to-end testing you generally do in
>>> compiler-rt's test suite, how the tradeoffs look compared to more targeted
>>> testing in Clang or LLVM, etc.
>>
>>
>> Right... I don't know much about profile test suite, or how much coverage
>> / what features owners want to test there,
>
>
> I was curious about the precedent here in the way of other/prior uses of
> compiler-rt such as the sanitizers to learn from existing experience,
> community norms, etc. My assumption was that we don't really test everything
> end-to-end, at least as far as I recall seeing patches to sanitizers and
> compiler-rt, but quite possibly I've misunderstood the uses here.
>

As Alexey mentioned, it is case by case and driven by the need.  Since
you are curious, I will give it a shot.

On a very high level, end-to-end testing is like integration testing
whose main purpose is to testing interfaces/interactions between
different components of the system and make sure they are working as
expected. The components involved in compiler-rt includes
compiler/assembler, runtime library for the base feature, supporting
libraries like unwinde etcr, linker, and various related host tools
etc. Their interfaces include profile data layout, runtime APIs,
runtime hooks or other runtime overrider mechanisms,  raw data format,
coverage data format, various different data/code related
compiler/linker options (garbage collection, shared lib, dynamically
loaded libs etc). There are lots of other things ..

>>
>> but from a quick glance these test cases seem reasonable:
>> we check that clang -> llvm-profdata -> llvm_cov pipeline works as
>> expected for defaulted functions. We're not checking for IR, or
>> implementation details here, which is IMO
>> nicer that Clang- or LLVM-specific test case. Still, test case in Clang
>> test suite is nice to have as well, because it would work on virtually all
>> targets, preventing people from breaking
>> this code accidentally if they're building on Mac OS w/o compiler-rt
>> checkout.
>
>
> Right, my main worry/thought is that these tests seem mostly redundant
> except fort he frontend part - I would've expected a representative sample
> to be tested end-to-end (each different kind of counter, for example) but
> I'm surprised if the norm is to test end-to-end any change that's made along
> the pipeline.

Using a magnifier, you are bound to find some redundancy in testing.
As I said, we should err on the side of caution in terms of testing
(The cost of regression/bug finding can be huge and greatly outweigh
the cost of the test itself, and good test coverage is the most
important thing to ensure fast development). Instead of saying: we
skip the test because it *seems* to be redundant, we should say the
opposite. If we build up the habit of skipping tests because we assume
they are covered elsewhere (and equivalently), it is not going to end
well.  We are not testing too many, but too few. Ideas on how to
increase end-end testing coverages are welcome.

(we have talked enough about this in this thread. Further discussion
can probably be done in llvm-dev).

thanks,

David

> - David
>
>>
>>
>>>
>>>
>>> Thanks,
>>> - Dave
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> -- but
>>>>>> I think it is better to keep them separate as they are in different
>>>>>> repos.  I will clean up the test as you suggested else where.
>>>>>>
>>>>>> >
>>>>>> > Is this a Linux-specific test?
>>>>>>
>>>>>> It should not be. It is just I don't have a good way to test on other
>>>>>> platforms yet.
>>>>>>
>>>>>> David
>>>>>> >
>>>>>> > vedant
>>>>>> >
>>>>>> >> On Feb 7, 2016, at 8:31 AM, Xinliang David Li via llvm-commits
>>>>>> >> <llvm-commits at lists.llvm.org> wrote:
>>>>>> >>
>>>>>> >> Author: davidxl
>>>>>> >> Date: Sun Feb  7 10:31:13 2016
>>>>>> >> New Revision: 260041
>>>>>> >>
>>>>>> >> URL: http://llvm.org/viewvc/llvm-project?rev=260041&view=rev
>>>>>> >> Log:
>>>>>> >> Add coverage tests (defaulted constructors/destructor)
>>>>>> >>
>>>>>> >> Added:
>>>>>> >>    compiler-rt/trunk/test/profile/Linux/coverage_ctors.cpp
>>>>>> >>    compiler-rt/trunk/test/profile/Linux/coverage_dtor.cpp
>>>>>> >>
>>>>>> >> Added: compiler-rt/trunk/test/profile/Linux/coverage_ctors.cpp
>>>>>> >> URL:
>>>>>> >> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/Linux/coverage_ctors.cpp?rev=260041&view=auto
>>>>>> >>
>>>>>> >> ==============================================================================
>>>>>> >> --- compiler-rt/trunk/test/profile/Linux/coverage_ctors.cpp (added)
>>>>>> >> +++ compiler-rt/trunk/test/profile/Linux/coverage_ctors.cpp Sun Feb
>>>>>> >> 7 10:31:13 2016
>>>>>> >> @@ -0,0 +1,33 @@
>>>>>> >> +// RUN: %clang_profgen -x c++  -std=c++11 -fuse-ld=gold
>>>>>> >> -fcoverage-mapping -o %t %s
>>>>>> >> +// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
>>>>>> >> +// RUN: llvm-profdata merge -o %t.profdata %t.profraw
>>>>>> >> +// RUN: llvm-cov show %t -instr-profile %t.profdata
>>>>>> >> -filename-equivalence 2>&1 | FileCheck %s
>>>>>> >> +
>>>>>> >> +struct Base {
>>>>>> >> +  int B;
>>>>>> >> +  Base() : B(2) {}
>>>>>> >> +  Base(const struct Base &b2) {
>>>>>> >> +    if (b2.B == 0) {
>>>>>> >> +      B = b2.B + 1;
>>>>>> >> +    } else
>>>>>> >> +      B = b2.B;
>>>>>> >> +  }
>>>>>> >> +};
>>>>>> >> +
>>>>>> >> +struct Derived : public Base {
>>>>>> >> +  Derived(const Derived &) = default; // CHECK:  2| [[@LINE]]|
>>>>>> >> Derived
>>>>>> >> +  Derived() = default;                // CHECK:  1| [[@LINE]]|
>>>>>> >> Derived
>>>>>> >> +  int I;
>>>>>> >> +  int J;
>>>>>> >> +  int getI() { return I; }
>>>>>> >> +};
>>>>>> >> +
>>>>>> >> +Derived dd;
>>>>>> >> +int g;
>>>>>> >> +int main() {
>>>>>> >> +  Derived dd2(dd);
>>>>>> >> +  Derived dd3(dd);
>>>>>> >> +
>>>>>> >> +  g = dd2.getI() + dd3.getI();
>>>>>> >> +  return 0;
>>>>>> >> +}
>>>>>> >>
>>>>>> >> Added: compiler-rt/trunk/test/profile/Linux/coverage_dtor.cpp
>>>>>> >> URL:
>>>>>> >> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/Linux/coverage_dtor.cpp?rev=260041&view=auto
>>>>>> >>
>>>>>> >> ==============================================================================
>>>>>> >> --- compiler-rt/trunk/test/profile/Linux/coverage_dtor.cpp (added)
>>>>>> >> +++ compiler-rt/trunk/test/profile/Linux/coverage_dtor.cpp Sun Feb
>>>>>> >> 7 10:31:13 2016
>>>>>> >> @@ -0,0 +1,26 @@
>>>>>> >> +// RUN: %clang -x c++ -fno-exceptions  -std=c++11 -fuse-ld=gold
>>>>>> >> -fprofile-instr-generate -fcoverage-mapping -o %t %s
>>>>>> >> +// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
>>>>>> >> +// RUN: llvm-profdata merge -o %t.profdata %t.profraw
>>>>>> >> +// RUN: llvm-cov show %t -instr-profile %t.profdata
>>>>>> >> -filename-equivalence 2>&1 | FileCheck %s
>>>>>> >> +
>>>>>> >> +struct Base {
>>>>>> >> +  int B;
>>>>>> >> +  Base(int B_) : B(B_) {}
>>>>>> >> +  ~Base() {}
>>>>>> >> +};
>>>>>> >> +
>>>>>> >> +struct Derived : public Base {
>>>>>> >> +  Derived(int K) : Base(K), I(K), J(K) {}
>>>>>> >> +  ~Derived() = default; // CHECK:  2| [[@LINE]]|  ~Derived
>>>>>> >> +  int I;
>>>>>> >> +  int J;
>>>>>> >> +  int getI() { return I; }
>>>>>> >> +};
>>>>>> >> +
>>>>>> >> +int g;
>>>>>> >> +int main() {
>>>>>> >> +  Derived dd(10);
>>>>>> >> +  Derived dd2(120);
>>>>>> >> +  g = dd2.getI() + dd.getI();
>>>>>> >> +  return 0;
>>>>>> >> +}
>>>>>> >>
>>>>>> >>
>>>>>> >> _______________________________________________
>>>>>> >> llvm-commits mailing list
>>>>>> >> llvm-commits at lists.llvm.org
>>>>>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>> >
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Alexey Samsonov
>>>> vonosmas at gmail.com
>>>
>>>
>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>
>