This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix target clones (PR gcov-profile/85370).


On Thu, Jul 26, 2018 at 10:44 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 07/25/2018 03:50 PM, Richard Biener wrote:
> > On Wed, Jul 25, 2018 at 3:38 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> Hi.
> >>
> >> Target clones have DECL_ARTIFICIAL set to 1, but we want to
> >> provide --coverage for that. With patched GCC on can see:
> >>
> >>         -:    0:Source:pr85370.c
> >>         -:    0:Graph:pr85370.gcno
> >>         -:    0:Data:pr85370.gcda
> >>         -:    0:Runs:1
> >>         -:    0:Programs:1
> >>         -:    1:__attribute__((target_clones("arch=slm","default")))
> >>         1:    2:int foo1 (int a, int b) { // executed #### wrongly
> >>         1:    3:  return a + b;
> >>         -:    4:}
> >> ------------------
> >> foo1.arch_slm.0:
> >>         0:    2:int foo1 (int a, int b) { // executed #### wrongly
> >>         0:    3:  return a + b;
> >>         -:    4:}
> >> ------------------
> >> foo1.default.1:
> >>         1:    2:int foo1 (int a, int b) { // executed #### wrongly
> >>         1:    3:  return a + b;
> >>         -:    4:}
> >> ------------------
> >>         -:    5:
> >>         1:    6:int foo2 (int a, int b) {
> >>         1:    7:  return a + b;
> >>         -:    8:}
> >>         -:    9:
> >>         1:   10:int main() {
> >>         1:   11:  foo1(1, 1);
> >>         1:   12:  foo2(1, 1);
> >>         1:   13:  return 1;
> >>         -:   14:}
> >>
> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> >> Will install in couple of days if no objection.
> >
> > I wonder if representing the clones as artificial but have their body be
> > marked as inline instance of the original function works for gcov?
>
> Do you mean an inlined functions? If so, these are fine as gimple statements
> that were inlined still point to original source code.
>
>  I think
> > it should for debuggers.  A similar case is probably the
> > static_constructors_and_destructors
>
> Actually static_c_a_d were motivation for exclusion. They have location set
> to last line in source code and it's not intentional. Similarly implicit
> ctors/dtors (e.g. Centering<3>::Centering(Centering<3> const&)) are ignored
> as they don't have any real line of code in a source file.
>
> > function which has all ctors/dtors of static objects inlined into but itself is
> > of course artificial.  Is that handled correctly?
>
> Hope I explained enough?

The question is what you like to see - looking at your figure above it
looks like
you want to see separate coverage for the different clones even when they
are auto-generated by GCC?  Isn't that inconsistent with for example
IPA-CP generated clones or inline instances?  Manual multiversions
in source are already reported separately?

Richard.

> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >> 2018-07-25  Martin Liska  <mliska@suse.cz>
> >>
> >>         PR gcov-profile/85370
> >>         * coverage.c (coverage_begin_function): Do not mark target
> >>         clones as artificial functions.
> >> ---
> >>  gcc/coverage.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]