> -----Original Message----- > From: Honnappa Nagarahalli [mailto:[email protected]] > Sent: Wednesday, May 31, 2017 9:03 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> > Cc: Brian Brooks <[email protected]>; [email protected]; Ola > Liljedahl <[email protected]> > Subject: Re: [lng-odp] [API-NEXT PATCH v6 3/6] Add arch/ files > > On 31 May 2017 at 03:02, Savolainen, Petri (Nokia - FI/Espoo) > <[email protected]> wrote: > > > > > >> -----Original Message----- > >> From: Honnappa Nagarahalli [mailto:[email protected]] > >> Sent: Wednesday, May 31, 2017 6:11 AM > >> To: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> > >> Cc: Brian Brooks <[email protected]>; [email protected]; Ola > >> Liljedahl <[email protected]> > >> Subject: Re: [lng-odp] [API-NEXT PATCH v6 3/6] Add arch/ files > >> > >> On 24 May 2017 at 06:55, Savolainen, Petri (Nokia - FI/Espoo) > >> <[email protected]> wrote: > >> > > >> > > >> >> -----Original Message----- > >> >> From: Honnappa Nagarahalli [mailto:[email protected]] > >> >> Sent: Wednesday, May 24, 2017 6:59 AM > >> >> To: Savolainen, Petri (Nokia - FI/Espoo) > <[email protected]> > >> >> Cc: Brian Brooks <[email protected]>; [email protected]; > Ola > >> >> Liljedahl <[email protected]> > >> >> Subject: Re: [lng-odp] [API-NEXT PATCH v6 3/6] Add arch/ files > >> >> > >> >> On 23 May 2017 at 02:10, Savolainen, Petri (Nokia - FI/Espoo) > >> >> <[email protected]> wrote: > >> >> >> diff --git a/platform/linux-generic/arch/powerpc/odp_cpu.h > >> >> >> b/platform/linux-generic/arch/powerpc/odp_cpu.h > >> >> >> new file mode 100644 > >> >> >> index 00000000..e118e709 > >> >> >> --- /dev/null > >> >> >> +++ b/platform/linux-generic/arch/powerpc/odp_cpu.h > >> >> >> @@ -0,0 +1,10 @@ > >> >> >> +/* Copyright (c) 2017, Linaro Limited > >> >> >> + * All rights reserved. > >> >> >> + * > >> >> >> + * SPDX-License-Identifier: BSD-3-Clause > >> >> >> + */ > >> >> >> + > >> >> >> +#ifndef ODP_POWERPC_CPU_H_ > >> >> >> +#define ODP_POWERPC_CPU_H_ > >> >> >> + > >> >> >> +#endif > >> >> > > >> >> > > >> >> > Does this patch break build for all other archs but arm and x86? > >> >> Shouldn't you do the same (dummy) definitions for all architectures, > as > >> >> you do for x86? > >> >> > > >> >> > Odp-linux should be usable in any system that runs Linux. It's not > >> >> practical to test on every arch, but we should always offer the > default > >> >> code path that builds and should work fine on any arch. For example, > I > >> did > >> >> cross compile my latest x86 specific changes for PowerPC to see that > a > >> >> non-x86 path also builds. > >> >> > > >> >> > >> >> We do not have the environment to compile for PowerPC or MIPS. Even > if > >> >> we write dummy functions, we will not be able to compile the code > for > >> >> those targets. During our earlier discussions, there was an > agreement > >> >> that we will not do this for PowerPC or MIPS. Respective arch owners > >> >> have to create those functions. > >> > > >> > ODP dependencies file have some instructions for cross compiling. > With > >> Ubuntu you just need to install some extra packages. E.g. > >> > > >> > sudo apt-get install gcc-powerpc-linux-gnu > >> > > >> > So, you have the environment to build for e.g. PowerPC. Since odp- > linux > >> is for everybody (not only x86 and arm), you must not break the build > for > >> others. You may offer the minimal support, dummy functions, something > that > >> is functionally correct but not optimized - but you must not break the > >> build. > >> > > >> > >> Why would we have code which is not tested? Successful compilation > >> does not mean, the code would work. It is better that compilation > >> fails rather than things not work during run time. > >> > >> Does ODP claim it supports PowerPC? As far as I know, it claims it is > >> supported on Linux. In that case, why not use the 'default' in arch > >> directory for PowerPC? > >> > >> What about MIPS? > >> > >> What about Kalray? > >> > >> What is the version of the gcc compiler that needs to be used? > >> > >> What about support for Clang on PowerPC and MIPS? What is the Clang > >> version we need to support? > >> > >> These builds are in ODP CI. > >> > >> We had agreed that support for PowerPC and MIPS needs to come from > >> respective owners. > > > > > > Odp-linux should build and run where ever you have Linux. Obviously, we > cannot guarantee the quality without testing, but we must not deliberately > break the build. After every commit, makefiles rules and potential arch > files/functions must be present to build for MIPS/PPC/Kalray etc. You > don't need to write assembly for other than ARM, but you need to add > stubs/generic C implementations for others. > > Again, what is the use of code that compiles but not tested and does > not work potentially?
Think it this way, the asm/arch-specific code is an exception. In order to get that exception path approved, you need to provide the default C-only code path. You should develop and test the default path also locally before adding the asm path. You can do it in two commits. First commit, implement only the default path and test it on your favorite system (x86, arm, ...). Second commit, implement and enable asm path for your favorite target. This way both paths go through our CI. True, we should force default path testing in our CI. Anyway, lack of a CI target today is not valid reason to break build for others. While adding arch specific code, you need do everything you can *not* to break build or functionality for others. > > > > > Arch specific code is not commonly added into odp-linux, but when you do > add it you need to try hard not to break the code for others. You can test > the C generic also on ARM/x86, before turning the asm optimized version > the default. So, also without a PPC target you can develop/test the C > generic path (on ARM/x86) and build it for PPC to check that makefiles and > #ifdefs are OK. > > This is how the scope of the work increases unnecessarily. As we had > agreed during our discussions, functions for MIPS and PowerPC code > should be provided by the respective maintainers. It is not possible > to compile for 8 different targets ourselves. > > ODP does not run the CI for these 2 targets. > > Why not fold these two targets into default? You can ask e.g. NXP and Cavium if they want to remove PPC and MIPS directories. Regardless of that you need to always provide the default path. We cannot test the default path on all possible targets, but you need to provide it anyway and test at least on some targets. -Petri
