On 04/24 07:21:09, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > -----Original Message----- > > From: Brian Brooks [mailto:brian.bro...@arm.com] > > Sent: Friday, April 21, 2017 7:51 PM > > To: Petri Savolainen <petri.savolai...@linaro.org> > > Cc: lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] [API-NEXT PATCH 2/8] linux-gen: cpu_flags: added > > x86 cpu flag read functions > > > > On 04/21 16:11:28, Petri Savolainen wrote: > > > When building on x86 CPU flags can be used to determine which > > > CPU features are supported. CPU flag definitions and the code > > > to read the flags is from DPDK. > > > > > > Signed-off-by: Petri Savolainen <petri.savolai...@linaro.org> > > > --- > > > configure.ac | 1 + > > > platform/linux-generic/Makefile.am | 4 + > > > platform/linux-generic/arch/x86/cpu_flags.c | 349 > > ++++++++++++++++++++++++++++ > > > platform/linux-generic/arch/x86/cpu_flags.h | 20 ++ > > > 4 files changed, 374 insertions(+) > > > create mode 100644 platform/linux-generic/arch/x86/cpu_flags.c > > > create mode 100644 platform/linux-generic/arch/x86/cpu_flags.h > > > > > > diff --git a/configure.ac b/configure.ac > > > index e86e2dca..38129030 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -224,6 +224,7 @@ AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = > > "xdoxygen"]) > > > AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ]) > > > AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"]) > > > AM_CONDITIONAL([helper_linux], [test x$helper_linux = xyes ]) > > > +AM_CONDITIONAL([ARCH_IS_X86], [test "x${ARCH_DIR}" = "xx86"]) > > > > > > > > ########################################################################## > > > # Setup doxygen documentation > > > diff --git a/platform/linux-generic/Makefile.am b/platform/linux- > > generic/Makefile.am > > > index 81a19011..60b7f849 100644 > > > --- a/platform/linux-generic/Makefile.am > > > +++ b/platform/linux-generic/Makefile.am > > > @@ -252,6 +252,10 @@ __LIB__libodp_linux_la_SOURCES = \ > > > arch/@ARCH_DIR@/odp_cpu_arch.c \ > > > arch/@ARCH_DIR@/odp_sysinfo_parse.c > > > > > > +if ARCH_IS_X86 > > > +__LIB__libodp_linux_la_SOURCES += arch/@ARCH_DIR@/cpu_flags.c > > > +endif > > > > The pre-existing condition is that all source files are compiled and > > this method of conditional compilation is rejected, as is the case > > for schedulers. How do we want to proceed? > > This file (arch/x86/cpu_flags.c) contains x86 assembly and is built only for > x86. As you can see from the lines above this line, arch/@ARCH_DIR@/ is used > to select the arch files. > > This is how I have instructed you to split out ARM assembly from your > scheduler code. Place ASM code into a new file (e.g. arch/arm/ll_sc.c or > whatever) and select that file when building for ARM. No ifdefs in C files. > > Current makefile ... > > arch/@ARCH_DIR@/odp_cpu_arch.c \ > arch/@ARCH_DIR@/odp_sysinfo_parse.c > > ... expects that all archs have those files implemented. That part should be > actually changed to use arch specific files only for those arch that do not > use the default implementation (x86/mips today). But that's not a job of this > patch - it just adds cpu_flags for x86.
This same argument can be applied to the scheduler i.e. it conditionally compiles code as needed, and anything else is not a job of that patch. Here, you would need to actually add those files for other archs and find suitible arch-agnostic names for identifiers. Which way do you want to go? > -Petri >
