linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Oberparleiter <oberpar@linux.ibm.com>
To: Tri Vo <trong@android.com>
Cc: ghackmann@android.com, ndesaulniers@google.com,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	yamada.masahiro@socionext.com, Trilok Soni <tsoni@quicinc.com>,
	Prasad Sodagudi <psodagud@quicinc.com>,
	Daniel Mentz <danielmentz@google.com>,
	Petri Gynther <pgynther@google.com>
Subject: Re: [PATCH v3 2/3] gcov: Clang support
Date: Tue, 5 Mar 2019 14:57:55 +0100	[thread overview]
Message-ID: <faa5a621-1f21-3136-58e7-9215c4f4d079@linux.ibm.com> (raw)
In-Reply-To: <20190122233749.42220-3-trong@android.com>

On 23.01.2019 00:37, Tri Vo wrote:
> From: Greg Hackmann <ghackmann@android.com>
> 
> LLVM uses profiling data that's deliberately similar to GCC, but has a very
> different way of exporting that data.  LLVM calls llvm_gcov_init() once per
> module, and provides a couple of callbacks that we can use to ask for more
> data.
> 
> We care about the "writeout" callback, which in turn calls back into
> compiler-rt/this module to dump all the gathered coverage data to disk:
> 
>    llvm_gcda_start_file()
>      llvm_gcda_emit_function()
>      llvm_gcda_emit_arcs()
>      llvm_gcda_emit_function()
>      llvm_gcda_emit_arcs()
>      [... repeats for each function ...]
>    llvm_gcda_summary_info()
>    llvm_gcda_end_file()
> 
> This design is much more stateless and unstructured than gcc's, and is
> intended to run at process exit.  This forces us to keep some local state
> about which module we're dealing with at the moment.  On the other hand, it
> also means we don't depend as much on how LLVM represents profiling data
> internally.
> 
> See LLVM's lib/Transforms/Instrumentation/GCOVProfiling.cpp for more
> details on how this works, particularly GCOVProfiler::emitProfileArcs(),
> GCOVProfiler::insertCounterWriteout(), and GCOVProfiler::insertFlush().
> 
> Co-authored-by: Nick Desaulniers <ndesaulniers@google.com>
> Co-authored-by: Tri Vo <trong@android.com>
> Signed-off-by: Greg Hackmann <ghackmann@android.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Tri Vo <trong@android.com>
> Tested-by: Trilok Soni <tsoni@quicinc.com>
> Tested-by: Prasad Sodagudi <psodagud@quicinc.com>
> Tested-by: Tri Vo <trong@android.com>
> Tested-by: Daniel Mentz <danielmentz@google.com>
> Tested-by: Petri Gynther <pgynther@google.com>
> ---
>  kernel/gcov/Kconfig  |   3 +-
>  kernel/gcov/Makefile |   1 +
>  kernel/gcov/clang.c  | 555 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 558 insertions(+), 1 deletion(-)
>  create mode 100644 kernel/gcov/clang.c
> 

[...]

> diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
> new file mode 100644
> index 000000000000..f9acdee3d578
> --- /dev/null
> +++ b/kernel/gcov/clang.c
> @@ -0,0 +1,555 @@

[...]

> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/printk.h>
> +#include <linux/ratelimit.h>
> +#include <linux/seq_file.h>
> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +#include "gcov.h"
> +
> +#define GCOV_TAG_FUNCTION_LENGTH	3

This define seems to be unused and should be removed.

[...]

> +/**
> + * gcov_info_unlink - unlink/remove profiling data set from the list
> + * @prev: previous profiling data set
> + * @info: profiling data set
> + */
> +void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
> +{
> +	if (info)
> +		list_del(&info->head);

The if-clause can be removed since @info is always non-NULL.

[...]

> +static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
> +{
> +	size_t cv_size; /* counter values size */
> +

This empty line should be removed.

[...]

Rest looks good! With these minor changes applied:

Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany


  reply	other threads:[~2019-03-05 13:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 23:37 [PATCH v3 0/3] gcov: add Clang support Tri Vo
2019-01-22 23:37 ` [PATCH v3 1/3] gcov: Clang: move common GCC code into gcc_base.c Tri Vo
2019-03-05 13:40   ` Peter Oberparleiter
2019-01-22 23:37 ` [PATCH v3 2/3] gcov: Clang support Tri Vo
2019-03-05 13:57   ` Peter Oberparleiter [this message]
2019-01-22 23:37 ` [PATCH v3 3/3] gcov: docs: add a note on GCC vs Clang differences Tri Vo
2019-01-22 23:50   ` Nick Desaulniers
2019-03-05 14:07   ` Peter Oberparleiter
2019-02-07  0:52 ` [PATCH v3 0/3] gcov: add Clang support Nick Desaulniers
2019-02-22  0:57   ` Nick Desaulniers
2019-03-05 14:29 ` Peter Oberparleiter
2019-03-12  4:57   ` Tri Vo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=faa5a621-1f21-3136-58e7-9215c4f4d079@linux.ibm.com \
    --to=oberpar@linux.ibm.com \
    --cc=danielmentz@google.com \
    --cc=ghackmann@android.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=pgynther@google.com \
    --cc=psodagud@quicinc.com \
    --cc=trong@android.com \
    --cc=tsoni@quicinc.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).