linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] gcov: add Clang support
@ 2019-01-15 18:36 Tri Vo
  2019-01-15 18:36 ` [PATCH v2 1/4] gcov: clang: move common gcc code into gcc_base.c Tri Vo
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Tri Vo @ 2019-01-15 18:36 UTC (permalink / raw)
  To: oberpar; +Cc: ghackmann, ndesaulniers, linux-kernel, kernel-team, Tri Vo

From: Tri Vo <trong@android.com>

This patch series adds Clang supoprt for gcov.

Patch 1 refactors existing code in preparation for Clang support. Patches
2-3 implement necessary LLVM runtime hooks and gcov kernel interfaces.
Patch 4 simplifies config selection.

Greg Hackmann (2):
  gcov: clang: move common gcc code into gcc_base.c
  gcov: clang support

Nick Desaulniers (1):
  gcov: clang: link/unlink profiling data set.

Tri Vo (1):
  gcov: clang: pick GCC vs Clang format depending on compiler

 kernel/gcov/Kconfig    |   3 +-
 kernel/gcov/Makefile   |   5 +-
 kernel/gcov/base.c     |  78 +-----
 kernel/gcov/clang.c    | 554 +++++++++++++++++++++++++++++++++++++++++
 kernel/gcov/gcc_base.c |  79 ++++++
 kernel/gcov/gcov.h     |   3 +
 6 files changed, 643 insertions(+), 79 deletions(-)
 create mode 100644 kernel/gcov/clang.c
 create mode 100644 kernel/gcov/gcc_base.c

v2:
- Reorganized config dependencies, as per Masahiro.

--
2.20.1.97.g81188d93c3-goog


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] gcov: clang: move common gcc code into gcc_base.c
  2019-01-15 18:36 [PATCH v2 0/4] gcov: add Clang support Tri Vo
@ 2019-01-15 18:36 ` Tri Vo
  2019-01-15 18:36 ` [PATCH v2 2/4] gcov: clang support Tri Vo
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Tri Vo @ 2019-01-15 18:36 UTC (permalink / raw)
  To: oberpar
  Cc: ghackmann, ndesaulniers, linux-kernel, kernel-team,
	Greg Hackmann, Tri Vo, Trilok Soni, Prasad Sodagudi

From: Greg Hackmann <ghackmann@google.com>

base.c contains a few callbacks specific to GCC's gcov implementation.
Move these into their own module in preparation for clang support.

Signed-off-by: Greg Hackmann <ghackmann@google.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>
---
 kernel/gcov/Makefile   |  4 +--
 kernel/gcov/base.c     | 78 ++---------------------------------------
 kernel/gcov/gcc_base.c | 79 ++++++++++++++++++++++++++++++++++++++++++
 kernel/gcov/gcov.h     |  3 ++
 4 files changed, 86 insertions(+), 78 deletions(-)
 create mode 100644 kernel/gcov/gcc_base.c

diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
index ff06d64df397..45431ed679d1 100644
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -2,5 +2,5 @@
 ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
 
 obj-y := base.o fs.o
-obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o
-obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o
+obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
+obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
diff --git a/kernel/gcov/base.c b/kernel/gcov/base.c
index 9c7c8d5c18f2..5262bb784597 100644
--- a/kernel/gcov/base.c
+++ b/kernel/gcov/base.c
@@ -22,82 +22,8 @@
 #include <linux/sched.h>
 #include "gcov.h"
 
-static int gcov_events_enabled;
-static DEFINE_MUTEX(gcov_lock);
-
-/*
- * __gcov_init is called by gcc-generated constructor code for each object
- * file compiled with -fprofile-arcs.
- */
-void __gcov_init(struct gcov_info *info)
-{
-	static unsigned int gcov_version;
-
-	mutex_lock(&gcov_lock);
-	if (gcov_version == 0) {
-		gcov_version = gcov_info_version(info);
-		/*
-		 * Printing gcc's version magic may prove useful for debugging
-		 * incompatibility reports.
-		 */
-		pr_info("version magic: 0x%x\n", gcov_version);
-	}
-	/*
-	 * Add new profiling data structure to list and inform event
-	 * listener.
-	 */
-	gcov_info_link(info);
-	if (gcov_events_enabled)
-		gcov_event(GCOV_ADD, info);
-	mutex_unlock(&gcov_lock);
-}
-EXPORT_SYMBOL(__gcov_init);
-
-/*
- * These functions may be referenced by gcc-generated profiling code but serve
- * no function for kernel profiling.
- */
-void __gcov_flush(void)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_flush);
-
-void __gcov_merge_add(gcov_type *counters, unsigned int n_counters)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_merge_add);
-
-void __gcov_merge_single(gcov_type *counters, unsigned int n_counters)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_merge_single);
-
-void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_merge_delta);
-
-void __gcov_merge_ior(gcov_type *counters, unsigned int n_counters)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_merge_ior);
-
-void __gcov_merge_time_profile(gcov_type *counters, unsigned int n_counters)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_merge_time_profile);
-
-void __gcov_merge_icall_topn(gcov_type *counters, unsigned int n_counters)
-{
-	/* Unused. */
-}
-EXPORT_SYMBOL(__gcov_merge_icall_topn);
+int gcov_events_enabled;
+DEFINE_MUTEX(gcov_lock);
 
 void __gcov_exit(void)
 {
diff --git a/kernel/gcov/gcc_base.c b/kernel/gcov/gcc_base.c
new file mode 100644
index 000000000000..823565bcf9bf
--- /dev/null
+++ b/kernel/gcov/gcc_base.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include "gcov.h"
+
+/*
+ * __gcov_init is called by gcc-generated constructor code for each object
+ * file compiled with -fprofile-arcs.
+ */
+void __gcov_init(struct gcov_info *info)
+{
+	static unsigned int gcov_version;
+
+	mutex_lock(&gcov_lock);
+	if (gcov_version == 0) {
+		gcov_version = gcov_info_version(info);
+		/*
+		 * Printing gcc's version magic may prove useful for debugging
+		 * incompatibility reports.
+		 */
+		pr_info("version magic: 0x%x\n", gcov_version);
+	}
+	/*
+	 * Add new profiling data structure to list and inform event
+	 * listener.
+	 */
+	gcov_info_link(info);
+	if (gcov_events_enabled)
+		gcov_event(GCOV_ADD, info);
+	mutex_unlock(&gcov_lock);
+}
+EXPORT_SYMBOL(__gcov_init);
+
+/*
+ * These functions may be referenced by gcc-generated profiling code but serve
+ * no function for kernel profiling.
+ */
+void __gcov_flush(void)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_flush);
+
+void __gcov_merge_add(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_add);
+
+void __gcov_merge_single(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_single);
+
+void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_delta);
+
+void __gcov_merge_ior(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_ior);
+
+void __gcov_merge_time_profile(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_time_profile);
+
+void __gcov_merge_icall_topn(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_icall_topn);
diff --git a/kernel/gcov/gcov.h b/kernel/gcov/gcov.h
index de118ad4a024..0ecf1d664ec3 100644
--- a/kernel/gcov/gcov.h
+++ b/kernel/gcov/gcov.h
@@ -83,4 +83,7 @@ struct gcov_link {
 };
 extern const struct gcov_link gcov_link[];
 
+extern int gcov_events_enabled;
+extern struct mutex gcov_lock;
+
 #endif /* GCOV_H */
-- 
2.20.1.97.g81188d93c3-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] gcov: clang support
  2019-01-15 18:36 [PATCH v2 0/4] gcov: add Clang support Tri Vo
  2019-01-15 18:36 ` [PATCH v2 1/4] gcov: clang: move common gcc code into gcc_base.c Tri Vo
@ 2019-01-15 18:36 ` Tri Vo
  2019-01-15 18:36 ` [PATCH v2 3/4] gcov: clang: link/unlink profiling data set Tri Vo
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Tri Vo @ 2019-01-15 18:36 UTC (permalink / raw)
  To: oberpar
  Cc: ghackmann, ndesaulniers, linux-kernel, kernel-team,
	Greg Hackmann, Tri Vo, Trilok Soni, Prasad Sodagudi

From: Greg Hackmann <ghackmann@google.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().

Signed-off-by: Greg Hackmann <ghackmann@google.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>
---
 kernel/gcov/Kconfig  |   5 +
 kernel/gcov/Makefile |   1 +
 kernel/gcov/clang.c  | 531 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 537 insertions(+)
 create mode 100644 kernel/gcov/clang.c

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 1e3823fa799b..eb428e570923 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -71,6 +71,11 @@ config GCOV_FORMAT_4_7
 	---help---
 	Select this option to use the format defined by GCC 4.7.
 
+config GCOV_FORMAT_CLANG
+	bool "Clang format"
+	---help---
+	Select this option to use the format defined by Clang.
+
 endchoice
 
 endmenu
diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
index 45431ed679d1..83da4765c18d 100644
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -4,3 +4,4 @@ ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
 obj-y := base.o fs.o
 obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
 obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
+obj-$(CONFIG_GCOV_FORMAT_CLANG) += clang.o
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
new file mode 100644
index 000000000000..b00795d28137
--- /dev/null
+++ b/kernel/gcov/clang.c
@@ -0,0 +1,531 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 Google, Inc.
+ * modified from kernel/gcov/gcc_4_7.c
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *
+ * 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().
+ */
+
+#define pr_fmt(fmt)	"gcov: " fmt
+
+#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
+
+typedef void (*llvm_gcov_callback)(void);
+
+struct gcov_info {
+	struct list_head head;
+
+	const char *filename;
+	unsigned int version;
+	u32 checksum;
+
+	struct list_head functions;
+};
+
+struct gcov_fn_info {
+	struct list_head head;
+
+	u32 ident;
+	u32 checksum;
+	u8 use_extra_checksum;
+	u32 cfg_checksum;
+
+	u32 num_counters;
+	u64 *counters;
+};
+
+static struct gcov_info *current_info;
+
+static LIST_HEAD(clang_gcov_list);
+
+void llvm_gcov_init(llvm_gcov_callback writeout, llvm_gcov_callback flush)
+{
+	struct gcov_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+	if (!info) {
+		pr_warn_ratelimited("failed to allocate gcov info\n");
+		return;
+	}
+
+	INIT_LIST_HEAD(&info->head);
+	INIT_LIST_HEAD(&info->functions);
+
+	mutex_lock(&gcov_lock);
+
+	list_add_tail(&info->head, &clang_gcov_list);
+	current_info = info;
+	writeout();
+	current_info = NULL;
+	if (gcov_events_enabled)
+		gcov_event(GCOV_ADD, info);
+
+	mutex_unlock(&gcov_lock);
+}
+EXPORT_SYMBOL(llvm_gcov_init);
+
+void llvm_gcda_start_file(const char *orig_filename, const char version[4],
+		u32 checksum)
+{
+	current_info->filename = orig_filename;
+	memcpy(&current_info->version, version, sizeof(current_info->version));
+	current_info->checksum = checksum;
+}
+EXPORT_SYMBOL(llvm_gcda_start_file);
+
+void llvm_gcda_emit_function(u32 ident, const char *function_name,
+		u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
+{
+	struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+	if (!info) {
+		pr_warn_ratelimited("failed to allocate gcov function info for %s\n",
+				function_name);
+		return;
+	}
+
+	INIT_LIST_HEAD(&info->head);
+	info->ident = ident;
+	info->checksum = func_checksum;
+	info->use_extra_checksum = use_extra_checksum;
+	info->cfg_checksum = cfg_checksum;
+
+	list_add_tail(&info->head, &current_info->functions);
+}
+EXPORT_SYMBOL(llvm_gcda_emit_function);
+
+void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
+{
+	struct gcov_fn_info *info = list_last_entry(&current_info->functions,
+			struct gcov_fn_info, head);
+
+	info->num_counters = num_counters;
+	info->counters = counters;
+}
+EXPORT_SYMBOL(llvm_gcda_emit_arcs);
+
+void llvm_gcda_summary_info(void)
+{
+}
+EXPORT_SYMBOL(llvm_gcda_summary_info);
+
+void llvm_gcda_end_file(void)
+{
+}
+EXPORT_SYMBOL(llvm_gcda_end_file);
+
+/**
+ * gcov_info_filename - return info filename
+ * @info: profiling data set
+ */
+const char *gcov_info_filename(struct gcov_info *info)
+{
+	return info->filename;
+}
+
+/**
+ * gcov_info_version - return info version
+ * @info: profiling data set
+ */
+unsigned int gcov_info_version(struct gcov_info *info)
+{
+	return info->version;
+}
+
+/**
+ * gcov_info_next - return next profiling data set
+ * @info: profiling data set
+ *
+ * Returns next gcov_info following @info or first gcov_info in the chain if
+ * @info is %NULL.
+ */
+struct gcov_info *gcov_info_next(struct gcov_info *info)
+{
+	if (!info)
+		return list_first_entry_or_null(&clang_gcov_list,
+				struct gcov_info, head);
+	if (list_is_last(&info->head, &clang_gcov_list))
+		return NULL;
+	return list_next_entry(info, head);
+}
+
+/* Symbolic links to be created for each profiling data file. */
+const struct gcov_link gcov_link[] = {
+	{ OBJ_TREE, "gcno" },	/* Link to .gcno file in $(objtree). */
+	{ 0, NULL},
+};
+
+/**
+ * gcov_info_reset - reset profiling data to zero
+ * @info: profiling data set
+ */
+void gcov_info_reset(struct gcov_info *info)
+{
+	struct gcov_fn_info *fn;
+
+	list_for_each_entry(fn, &info->functions, head)
+		memset(fn->counters, 0,
+				sizeof(fn->counters[0]) * fn->num_counters);
+}
+
+/**
+ * gcov_info_is_compatible - check if profiling data can be added
+ * @info1: first profiling data set
+ * @info2: second profiling data set
+ *
+ * Returns non-zero if profiling data can be added, zero otherwise.
+ */
+int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
+{
+	return (info1->checksum == info2->checksum);
+}
+
+/**
+ * gcov_info_add - add up profiling data
+ * @dest: profiling data set to which data is added
+ * @source: profiling data set which is added
+ *
+ * Adds profiling counts of @source to @dest.
+ */
+void gcov_info_add(struct gcov_info *dst, struct gcov_info *src)
+{
+	struct gcov_fn_info *dfn_ptr;
+	struct gcov_fn_info *sfn_ptr = list_first_entry_or_null(&src->functions,
+			struct gcov_fn_info, head);
+
+	list_for_each_entry(dfn_ptr, &dst->functions, head) {
+		u32 i;
+
+		for (i = 0; i < sfn_ptr->num_counters; i++)
+			dfn_ptr->counters[i] += sfn_ptr->counters[i];
+
+		if (!list_is_last(&sfn_ptr->head, &src->functions))
+			sfn_ptr = list_next_entry(sfn_ptr, head);
+	}
+}
+
+static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
+{
+	struct gcov_fn_info *fn_dup = kmemdup(fn, sizeof(*fn),
+			GFP_KERNEL);
+	if (!fn_dup)
+		return NULL;
+	INIT_LIST_HEAD(&fn_dup->head);
+
+	fn_dup->name = kstrdup(fn->name, GFP_KERNEL);
+	if (!fn_dup->name)
+		goto err_name;
+
+	fn_dup->counters = kmemdup(fn->counters,
+			fn->num_counters * sizeof(fn->counters[0]),
+			GFP_KERNEL);
+	if (!fn_dup->counters)
+		goto err_counters;
+
+	return fn_dup;
+
+err_counters:
+	kfree(fn_dup->name);
+err_name:
+	kfree(fn_dup);
+	return NULL;
+}
+
+/**
+ * gcov_info_dup - duplicate profiling data set
+ * @info: profiling data set to duplicate
+ *
+ * Return newly allocated duplicate on success, %NULL on error.
+ */
+struct gcov_info *gcov_info_dup(struct gcov_info *info)
+{
+	struct gcov_info *dup;
+	struct gcov_fn_info *fn, *tmp;
+
+	dup = kmemdup(info, sizeof(*dup), GFP_KERNEL);
+	if (!dup)
+		return NULL;
+	INIT_LIST_HEAD(&dup->head);
+	INIT_LIST_HEAD(&dup->functions);
+	dup->filename = kstrdup(info->filename, GFP_KERNEL);
+
+	list_for_each_entry_safe(fn, tmp, &info->functions, head) {
+		struct gcov_fn_info *fn_dup = gcov_fn_info_dup(fn);
+
+		if (!fn_dup)
+			goto err;
+		list_add_tail(&fn_dup->head, &dup->functions);
+	}
+
+	return dup;
+
+err:
+	gcov_info_free(dup);
+	return NULL;
+}
+
+/**
+ * gcov_info_free - release memory for profiling data set duplicate
+ * @info: profiling data set duplicate to free
+ */
+void gcov_info_free(struct gcov_info *info)
+{
+	struct gcov_fn_info *fn, *tmp;
+
+	list_for_each_entry_safe(fn, tmp, &info->functions, head) {
+		kfree(fn->name);
+		kfree(fn->counters);
+		list_del(&fn->head);
+		kfree(fn);
+	}
+	kfree(info->filename);
+	kfree(info);
+}
+
+#define ITER_STRIDE	PAGE_SIZE
+
+/**
+ * struct gcov_iterator - specifies current file position in logical records
+ * @info: associated profiling data
+ * @buffer: buffer containing file data
+ * @size: size of buffer
+ * @pos: current position in file
+ */
+struct gcov_iterator {
+	struct gcov_info *info;
+	void *buffer;
+	size_t size;
+	loff_t pos;
+};
+
+/**
+ * store_gcov_u32 - store 32 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
+ * store anything.
+ */
+static size_t store_gcov_u32(void *buffer, size_t off, u32 v)
+{
+	u32 *data;
+
+	if (buffer) {
+		data = buffer + off;
+		*data = v;
+	}
+
+	return sizeof(*data);
+}
+
+/**
+ * store_gcov_u64 - store 64 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. 64 bit numbers are stored as two 32 bit numbers, the low part
+ * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
+ * anything.
+ */
+static size_t store_gcov_u64(void *buffer, size_t off, u64 v)
+{
+	u32 *data;
+
+	if (buffer) {
+		data = buffer + off;
+
+		data[0] = (v & 0xffffffffUL);
+		data[1] = (v >> 32);
+	}
+
+	return sizeof(*data) * 2;
+}
+
+/**
+ * convert_to_gcda - convert profiling data set to gcda file format
+ * @buffer: the buffer to store file data or %NULL if no data should be stored
+ * @info: profiling data set to be converted
+ *
+ * Returns the number of bytes that were/would have been stored into the buffer.
+ */
+static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
+{
+	struct gcov_fn_info *fi_ptr;
+	size_t pos = 0;
+
+	/* File header. */
+	pos += store_gcov_u32(buffer, pos, GCOV_DATA_MAGIC);
+	pos += store_gcov_u32(buffer, pos, info->version);
+	pos += store_gcov_u32(buffer, pos, info->checksum);
+
+	list_for_each_entry(fi_ptr, &info->functions, head) {
+		u32 i;
+		u32 len = 2;
+
+		if (fi_ptr->use_extra_checksum)
+			len++;
+
+		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
+		pos += store_gcov_u32(buffer, pos, len);
+		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
+		pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
+		if (fi_ptr->use_extra_checksum)
+			pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+
+		pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
+		pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
+		for (i = 0; i < fi_ptr->num_counters; i++)
+			pos += store_gcov_u64(buffer, pos, fi_ptr->counters[i]);
+	}
+
+	return pos;
+}
+
+/**
+ * gcov_iter_new - allocate and initialize profiling data iterator
+ * @info: profiling data set to be iterated
+ *
+ * Return file iterator on success, %NULL otherwise.
+ */
+struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
+{
+	struct gcov_iterator *iter;
+
+	iter = kzalloc(sizeof(struct gcov_iterator), GFP_KERNEL);
+	if (!iter)
+		goto err_free;
+
+	iter->info = info;
+	/* Dry-run to get the actual buffer size. */
+	iter->size = convert_to_gcda(NULL, info);
+	iter->buffer = vmalloc(iter->size);
+	if (!iter->buffer)
+		goto err_free;
+
+	convert_to_gcda(iter->buffer, info);
+
+	return iter;
+
+err_free:
+	kfree(iter);
+	return NULL;
+}
+
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+void gcov_iter_free(struct gcov_iterator *iter)
+{
+	vfree(iter->buffer);
+	kfree(iter);
+}
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
+{
+	return iter->info;
+}
+
+/**
+ * gcov_iter_start - reset file iterator to starting position
+ * @iter: file iterator
+ */
+void gcov_iter_start(struct gcov_iterator *iter)
+{
+	iter->pos = 0;
+}
+
+/**
+ * gcov_iter_next - advance file iterator to next logical record
+ * @iter: file iterator
+ *
+ * Return zero if new position is valid, non-zero if iterator has reached end.
+ */
+int gcov_iter_next(struct gcov_iterator *iter)
+{
+	if (iter->pos < iter->size)
+		iter->pos += ITER_STRIDE;
+
+	if (iter->pos >= iter->size)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * gcov_iter_write - write data for current pos to seq_file
+ * @iter: file iterator
+ * @seq: seq_file handle
+ *
+ * Return zero on success, non-zero otherwise.
+ */
+int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
+{
+	size_t len;
+
+	if (iter->pos >= iter->size)
+		return -EINVAL;
+
+	len = ITER_STRIDE;
+	if (iter->pos + len > iter->size)
+		len = iter->size - iter->pos;
+
+	seq_write(seq, iter->buffer + iter->pos, len);
+
+	return 0;
+}
-- 
2.20.1.97.g81188d93c3-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] gcov: clang: link/unlink profiling data set.
  2019-01-15 18:36 [PATCH v2 0/4] gcov: add Clang support Tri Vo
  2019-01-15 18:36 ` [PATCH v2 1/4] gcov: clang: move common gcc code into gcc_base.c Tri Vo
  2019-01-15 18:36 ` [PATCH v2 2/4] gcov: clang support Tri Vo
@ 2019-01-15 18:36 ` Tri Vo
  2019-01-15 18:36 ` [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler Tri Vo
  2019-01-16 15:17 ` [PATCH v2 0/4] gcov: add Clang support Peter Oberparleiter
  4 siblings, 0 replies; 11+ messages in thread
From: Tri Vo @ 2019-01-15 18:36 UTC (permalink / raw)
  To: oberpar
  Cc: ghackmann, ndesaulniers, linux-kernel, kernel-team, Tri Vo,
	Trilok Soni, Prasad Sodagudi

From: Nick Desaulniers <ndesaulniers@google.com>

gcov.h defines an interface to access gcov_info data.

Add Clang implementation of gcov_info_link/gcov_info_unlink interfaces.

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>
---
 kernel/gcov/clang.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index b00795d28137..ea42deb852f7 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -77,6 +77,7 @@ struct gcov_fn_info {
 
 	u32 num_counters;
 	u64 *counters;
+	const char *function_name;
 };
 
 static struct gcov_info *current_info;
@@ -124,7 +125,7 @@ void llvm_gcda_emit_function(u32 ident, const char *function_name,
 
 	if (!info) {
 		pr_warn_ratelimited("failed to allocate gcov function info for %s\n",
-				function_name);
+				function_name ?: "UNKNOWN");
 		return;
 	}
 
@@ -133,6 +134,8 @@ void llvm_gcda_emit_function(u32 ident, const char *function_name,
 	info->checksum = func_checksum;
 	info->use_extra_checksum = use_extra_checksum;
 	info->cfg_checksum = cfg_checksum;
+	if (function_name)
+		info->function_name = kstrdup(function_name, GFP_KERNEL);
 
 	list_add_tail(&info->head, &current_info->functions);
 }
@@ -193,6 +196,26 @@ struct gcov_info *gcov_info_next(struct gcov_info *info)
 	return list_next_entry(info, head);
 }
 
+/**
+ * gcov_info_link - link/add profiling data set to the list
+ * @info: profiling data set
+ */
+void gcov_info_link(struct gcov_info *info)
+{
+	list_add_tail(&info->head, &clang_gcov_list);
+}
+
+/**
+ * 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 (prev)
+		list_del(&prev->head);
+}
+
 /* Symbolic links to be created for each profiling data file. */
 const struct gcov_link gcov_link[] = {
 	{ OBJ_TREE, "gcno" },	/* Link to .gcno file in $(objtree). */
@@ -256,8 +279,8 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
 		return NULL;
 	INIT_LIST_HEAD(&fn_dup->head);
 
-	fn_dup->name = kstrdup(fn->name, GFP_KERNEL);
-	if (!fn_dup->name)
+	fn_dup->function_name = kstrdup(fn->function_name, GFP_KERNEL);
+	if (!fn_dup->function_name)
 		goto err_name;
 
 	fn_dup->counters = kmemdup(fn->counters,
@@ -269,7 +292,7 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
 	return fn_dup;
 
 err_counters:
-	kfree(fn_dup->name);
+	kfree(fn_dup->function_name);
 err_name:
 	kfree(fn_dup);
 	return NULL;
@@ -317,7 +340,7 @@ void gcov_info_free(struct gcov_info *info)
 	struct gcov_fn_info *fn, *tmp;
 
 	list_for_each_entry_safe(fn, tmp, &info->functions, head) {
-		kfree(fn->name);
+		kfree(fn->function_name);
 		kfree(fn->counters);
 		list_del(&fn->head);
 		kfree(fn);
-- 
2.20.1.97.g81188d93c3-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler
  2019-01-15 18:36 [PATCH v2 0/4] gcov: add Clang support Tri Vo
                   ` (2 preceding siblings ...)
  2019-01-15 18:36 ` [PATCH v2 3/4] gcov: clang: link/unlink profiling data set Tri Vo
@ 2019-01-15 18:36 ` Tri Vo
  2019-01-15 19:25   ` Nick Desaulniers
  2019-01-16 15:17 ` [PATCH v2 0/4] gcov: add Clang support Peter Oberparleiter
  4 siblings, 1 reply; 11+ messages in thread
From: Tri Vo @ 2019-01-15 18:36 UTC (permalink / raw)
  To: oberpar; +Cc: ghackmann, ndesaulniers, linux-kernel, kernel-team, Tri Vo

From: Tri Vo <trong@android.com>

Clang gcov format is only supported by Clang compiler, and Clang
compiler only supports Clang format.

We set gcov format to depend on which compiler (GCC or Clang) is used.

Automatic format detection behavior is preserved because:
If GCC is used, one of the GCC gcov formats is selected.
If Clang is used, Clang gcov format is selected.

Signed-off-by: Tri Vo <trong@android.com>
---
 kernel/gcov/Kconfig  | 8 ++------
 kernel/gcov/Makefile | 2 +-
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index eb428e570923..f71c1adcff31 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -53,6 +53,7 @@ config GCOV_PROFILE_ALL
 choice
 	prompt "Specify GCOV format"
 	depends on GCOV_KERNEL
+	depends on CC_IS_GCC
 	---help---
 	The gcov format is usually determined by the GCC version, and the
 	default is chosen according to your GCC version. However, there are
@@ -62,7 +63,7 @@ choice
 
 config GCOV_FORMAT_3_4
 	bool "GCC 3.4 format"
-	depends on CC_IS_GCC && GCC_VERSION < 40700
+	depends on GCC_VERSION < 40700
 	---help---
 	Select this option to use the format defined by GCC 3.4.
 
@@ -71,11 +72,6 @@ config GCOV_FORMAT_4_7
 	---help---
 	Select this option to use the format defined by GCC 4.7.
 
-config GCOV_FORMAT_CLANG
-	bool "Clang format"
-	---help---
-	Select this option to use the format defined by Clang.
-
 endchoice
 
 endmenu
diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
index 83da4765c18d..d66a74b0f100 100644
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -4,4 +4,4 @@ ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
 obj-y := base.o fs.o
 obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
 obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
-obj-$(CONFIG_GCOV_FORMAT_CLANG) += clang.o
+obj-$(CONFIG_CC_IS_CLANG) += clang.o
-- 
2.20.1.97.g81188d93c3-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler
  2019-01-15 18:36 ` [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler Tri Vo
@ 2019-01-15 19:25   ` Nick Desaulniers
  2019-01-16 10:18     ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-01-15 19:25 UTC (permalink / raw)
  To: Tri Vo; +Cc: oberpar, Greg Hackmann, LKML, kernel-team, Masahiro Yamada

+ Masahiro

On Tue, Jan 15, 2019 at 10:37 AM Tri Vo <trong@android.com> wrote:
>
> From: Tri Vo <trong@android.com>
>
> Clang gcov format is only supported by Clang compiler, and Clang
> compiler only supports Clang format.
>
> We set gcov format to depend on which compiler (GCC or Clang) is used.
>
> Automatic format detection behavior is preserved because:
> If GCC is used, one of the GCC gcov formats is selected.
> If Clang is used, Clang gcov format is selected.
>
> Signed-off-by: Tri Vo <trong@android.com>
> ---
>  kernel/gcov/Kconfig  | 8 ++------
>  kernel/gcov/Makefile | 2 +-
>  2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index eb428e570923..f71c1adcff31 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -53,6 +53,7 @@ config GCOV_PROFILE_ALL
>  choice
>         prompt "Specify GCOV format"
>         depends on GCOV_KERNEL
> +       depends on CC_IS_GCC
>         ---help---
>         The gcov format is usually determined by the GCC version, and the
>         default is chosen according to your GCC version. However, there are
> @@ -62,7 +63,7 @@ choice
>
>  config GCOV_FORMAT_3_4
>         bool "GCC 3.4 format"
> -       depends on CC_IS_GCC && GCC_VERSION < 40700
> +       depends on GCC_VERSION < 40700
>         ---help---
>         Select this option to use the format defined by GCC 3.4.
>
> @@ -71,11 +72,6 @@ config GCOV_FORMAT_4_7
>         ---help---
>         Select this option to use the format defined by GCC 4.7.
>
> -config GCOV_FORMAT_CLANG
> -       bool "Clang format"
> -       ---help---
> -       Select this option to use the format defined by Clang.
> -
>  endchoice
>
>  endmenu
> diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
> index 83da4765c18d..d66a74b0f100 100644
> --- a/kernel/gcov/Makefile
> +++ b/kernel/gcov/Makefile
> @@ -4,4 +4,4 @@ ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
>  obj-y := base.o fs.o
>  obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
>  obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
> -obj-$(CONFIG_GCOV_FORMAT_CLANG) += clang.o
> +obj-$(CONFIG_CC_IS_CLANG) += clang.o

I think this was the suggestion from Masahiro from v1?
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler
  2019-01-15 19:25   ` Nick Desaulniers
@ 2019-01-16 10:18     ` Masahiro Yamada
  2019-01-17 21:48       ` Nick Desaulniers
  0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2019-01-16 10:18 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: Tri Vo, oberpar, Greg Hackmann, LKML, kernel-team

On Wed, Jan 16, 2019 at 7:00 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> + Masahiro
>
> On Tue, Jan 15, 2019 at 10:37 AM Tri Vo <trong@android.com> wrote:
> >
> > From: Tri Vo <trong@android.com>
> >
> > Clang gcov format is only supported by Clang compiler, and Clang
> > compiler only supports Clang format.
> >
> > We set gcov format to depend on which compiler (GCC or Clang) is used.
> >
> > Automatic format detection behavior is preserved because:
> > If GCC is used, one of the GCC gcov formats is selected.
> > If Clang is used, Clang gcov format is selected.
> >
> > Signed-off-by: Tri Vo <trong@android.com>
> > ---
> >  kernel/gcov/Kconfig  | 8 ++------
> >  kernel/gcov/Makefile | 2 +-
> >  2 files changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> > index eb428e570923..f71c1adcff31 100644
> > --- a/kernel/gcov/Kconfig
> > +++ b/kernel/gcov/Kconfig
> > @@ -53,6 +53,7 @@ config GCOV_PROFILE_ALL
> >  choice
> >         prompt "Specify GCOV format"
> >         depends on GCOV_KERNEL
> > +       depends on CC_IS_GCC
> >         ---help---
> >         The gcov format is usually determined by the GCC version, and the
> >         default is chosen according to your GCC version. However, there are
> > @@ -62,7 +63,7 @@ choice
> >
> >  config GCOV_FORMAT_3_4
> >         bool "GCC 3.4 format"
> > -       depends on CC_IS_GCC && GCC_VERSION < 40700
> > +       depends on GCC_VERSION < 40700
> >         ---help---
> >         Select this option to use the format defined by GCC 3.4.
> >
> > @@ -71,11 +72,6 @@ config GCOV_FORMAT_4_7
> >         ---help---
> >         Select this option to use the format defined by GCC 4.7.
> >
> > -config GCOV_FORMAT_CLANG
> > -       bool "Clang format"
> > -       ---help---
> > -       Select this option to use the format defined by Clang.
> > -
> >  endchoice
> >
> >  endmenu
> > diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
> > index 83da4765c18d..d66a74b0f100 100644
> > --- a/kernel/gcov/Makefile
> > +++ b/kernel/gcov/Makefile
> > @@ -4,4 +4,4 @@ ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
> >  obj-y := base.o fs.o
> >  obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_base.o gcc_3_4.o
> >  obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_base.o gcc_4_7.o
> > -obj-$(CONFIG_GCOV_FORMAT_CLANG) += clang.o
> > +obj-$(CONFIG_CC_IS_CLANG) += clang.o
>
> I think this was the suggestion from Masahiro from v1?

Yes.


What is strange is, 2/4 added GCOV_FORMAT_CLANG,
then 4/4 deletes it.


Probably, changing the patch order,
or squashing 4/4 into 2/4 would be cleaner.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] gcov: add Clang support
  2019-01-15 18:36 [PATCH v2 0/4] gcov: add Clang support Tri Vo
                   ` (3 preceding siblings ...)
  2019-01-15 18:36 ` [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler Tri Vo
@ 2019-01-16 15:17 ` Peter Oberparleiter
  2019-01-17 21:30   ` Nick Desaulniers
  4 siblings, 1 reply; 11+ messages in thread
From: Peter Oberparleiter @ 2019-01-16 15:17 UTC (permalink / raw)
  To: Tri Vo; +Cc: ghackmann, ndesaulniers, linux-kernel, kernel-team

On 15.01.2019 19:36, Tri Vo wrote:
> From: Tri Vo <trong@android.com>
> 
> This patch series adds Clang supoprt for gcov.
> 
> Patch 1 refactors existing code in preparation for Clang support. Patches
> 2-3 implement necessary LLVM runtime hooks and gcov kernel interfaces.
> Patch 4 simplifies config selection.
> 
> Greg Hackmann (2):
>   gcov: clang: move common gcc code into gcc_base.c
>   gcov: clang support
> 
> Nick Desaulniers (1):
>   gcov: clang: link/unlink profiling data set.
> 
> Tri Vo (1):
>   gcov: clang: pick GCC vs Clang format depending on compiler

Overall this patch set looks good to me. It introduces a clean way for
GCC and Clang specific implementation details to coexist.

A quick test compile using GCC on s390 with the patch set applied was
successful, so it seems like the existing support is unaffected.

There are some detail suggestions on patch improvements that I'll post
in separate e-mails.

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] gcov: add Clang support
  2019-01-16 15:17 ` [PATCH v2 0/4] gcov: add Clang support Peter Oberparleiter
@ 2019-01-17 21:30   ` Nick Desaulniers
  2019-01-17 22:01     ` Tri Vo
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-01-17 21:30 UTC (permalink / raw)
  To: Peter Oberparleiter, Tri Vo; +Cc: Greg Hackmann, LKML, kernel-team

On Wed, Jan 16, 2019 at 7:17 AM Peter Oberparleiter
<oberpar@linux.ibm.com> wrote:
>
> On 15.01.2019 19:36, Tri Vo wrote:
> > From: Tri Vo <trong@android.com>
> >
> > This patch series adds Clang supoprt for gcov.
> >
> > Patch 1 refactors existing code in preparation for Clang support. Patches
> > 2-3 implement necessary LLVM runtime hooks and gcov kernel interfaces.
> > Patch 4 simplifies config selection.
> >
> > Greg Hackmann (2):
> >   gcov: clang: move common gcc code into gcc_base.c
> >   gcov: clang support
> >
> > Nick Desaulniers (1):
> >   gcov: clang: link/unlink profiling data set.
> >
> > Tri Vo (1):
> >   gcov: clang: pick GCC vs Clang format depending on compiler
>
> Overall this patch set looks good to me. It introduces a clean way for
> GCC and Clang specific implementation details to coexist.
>
> A quick test compile using GCC on s390 with the patch set applied was
> successful, so it seems like the existing support is unaffected.
>
> There are some detail suggestions on patch improvements that I'll post
> in separate e-mails.

Peter,
Thank you so much for the valuable code review and feedback.  We'll prep a v3.

Tri,
I think you should change the authorship for the patches to yourself,
and add a "Co-authored-by: " tag to the commit messages.  Can probably
squash this series into 2 patches; the one that separates out the
shared code; and the new llvm additions.  WDYT?
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler
  2019-01-16 10:18     ` Masahiro Yamada
@ 2019-01-17 21:48       ` Nick Desaulniers
  0 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2019-01-17 21:48 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Tri Vo, Peter Oberparleiter, Greg Hackmann, LKML, kernel-team

On Wed, Jan 16, 2019 at 2:19 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> > I think this was the suggestion from Masahiro from v1?
>
> Yes.
>
>
> What is strange is, 2/4 added GCOV_FORMAT_CLANG,
> then 4/4 deletes it.
>
>
> Probably, changing the patch order,
> or squashing 4/4 into 2/4 would be cleaner.

Sure; Tri, I think it's ok for you to become author of 2/4 and put a
"Co-authored-by:" tag for Greg.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] gcov: add Clang support
  2019-01-17 21:30   ` Nick Desaulniers
@ 2019-01-17 22:01     ` Tri Vo
  0 siblings, 0 replies; 11+ messages in thread
From: Tri Vo @ 2019-01-17 22:01 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: Peter Oberparleiter, Greg Hackmann, LKML, kernel-team

On Thu, Jan 17, 2019 at 1:30 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Jan 16, 2019 at 7:17 AM Peter Oberparleiter
> <oberpar@linux.ibm.com> wrote:
> >
> > On 15.01.2019 19:36, Tri Vo wrote:
> > > From: Tri Vo <trong@android.com>
> > >
> > > This patch series adds Clang supoprt for gcov.
> > >
> > > Patch 1 refactors existing code in preparation for Clang support. Patches
> > > 2-3 implement necessary LLVM runtime hooks and gcov kernel interfaces.
> > > Patch 4 simplifies config selection.
> > >
> > > Greg Hackmann (2):
> > >   gcov: clang: move common gcc code into gcc_base.c
> > >   gcov: clang support
> > >
> > > Nick Desaulniers (1):
> > >   gcov: clang: link/unlink profiling data set.
> > >
> > > Tri Vo (1):
> > >   gcov: clang: pick GCC vs Clang format depending on compiler
> >
> > Overall this patch set looks good to me. It introduces a clean way for
> > GCC and Clang specific implementation details to coexist.
> >
> > A quick test compile using GCC on s390 with the patch set applied was
> > successful, so it seems like the existing support is unaffected.
> >
> > There are some detail suggestions on patch improvements that I'll post
> > in separate e-mails.
>
> Peter,
> Thank you so much for the valuable code review and feedback.  We'll prep a v3.
>
> Tri,
> I think you should change the authorship for the patches to yourself,
> and add a "Co-authored-by: " tag to the commit messages.  Can probably
> squash this series into 2 patches; the one that separates out the
> shared code; and the new llvm additions.  WDYT?

Squashing into 2 patches sounds good. I'll leave Greg's authorship in
there though.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-17 22:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 18:36 [PATCH v2 0/4] gcov: add Clang support Tri Vo
2019-01-15 18:36 ` [PATCH v2 1/4] gcov: clang: move common gcc code into gcc_base.c Tri Vo
2019-01-15 18:36 ` [PATCH v2 2/4] gcov: clang support Tri Vo
2019-01-15 18:36 ` [PATCH v2 3/4] gcov: clang: link/unlink profiling data set Tri Vo
2019-01-15 18:36 ` [PATCH v2 4/4] gcov: clang: pick GCC vs Clang format depending on compiler Tri Vo
2019-01-15 19:25   ` Nick Desaulniers
2019-01-16 10:18     ` Masahiro Yamada
2019-01-17 21:48       ` Nick Desaulniers
2019-01-16 15:17 ` [PATCH v2 0/4] gcov: add Clang support Peter Oberparleiter
2019-01-17 21:30   ` Nick Desaulniers
2019-01-17 22:01     ` Tri Vo

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).