On 1 June 2017 at 02:43, Savolainen, Petri (Nokia - FI/Espoo) <[email protected]> wrote: > > >> -----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. > This does not help in solving the problem we are facing today, compiling for 8 different targets.
> 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. Just curious, did you compile the code for PPC and MIPS or are you saying because you did not find the code under these target specific directories? > > >> >> > >> > 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 > >
