Re: [PATCH v5 3/5] perf callchain: Add support for cross-platform unwind

From: Hekuang
Date: Fri May 27 2016 - 04:06:03 EST




å 2016/5/27 15:38, Jiri Olsa åé:
On Fri, May 27, 2016 at 03:13:04PM +0800, Hekuang wrote:

SNIP

- I understand we need to compile 3 objects from unwind-libunwind.c,
how about we create 3 files like:

util/unwind-libunwind-local.c
util/unwind-libunwind-x86_32.c
util/unwind-libunwind-arm64.c

which would setup all necessary defines and include unwind-libunwind.c like:

---
/* comments explaining every define ;-) */
...
#define LOCAL... REMOTE..
...
#include <util/unwind-libunwind-local.c>
...
----

this way we will keep all the special setup for given unwind object
in one place and you can also use simple rule in the Build file like
without defining special rule:

libperf-$(CONFIG_LIBUNWIND_X86) += unwind-libunwind_x86_32.o
libperf-$(CONFIG_LIBUNWIND_AARCH64) += unwind-libunwind_arm64.o

the same way for the arch object:

arch/x86/util/unwind-libunwind-local.c
arch/x86/util/unwind-libunwind-x86_32.c


Not sure I thought everything through, but I think this way
we'll keep it more maintainable and readable..

let me know what you think
The only concern is that, if later we support more platforms,
there will be too much files named as 'tools/perf/util/unwind-libunwind*.c'
Is it acceptable or not?

And I thought all files belongs to specific archs should
go to folder under 'tools/perf/arch/xxx', is that right?
hum, I wouldn't worry about that.. but you're right,
let's put them under arch

But only 'tools/perf/arch/$(host platform)' will be built, in our case,
we should built the unwind-libunwind-$(arch) as long as we have
the remote libunwind libraries. So, I think there's a conflict in the
existing build script and not easy to 'put them under arch'. That's why
I choose a complex way in my previous patch.

Do you have some suggestions?

thanks,
jirka