This is getting very confusing now: why helper/<OS>?  do we have
odp/<OS>?: shouldn't  these be symetric?

My view is getting clearer and clearer:
On the test side, was have made tests for {OS, HW}= {linux/PC} and
given the possibility to diverge from these (for any  reason, e.g.
difference of HW or OS) with the use of "platform":
If a test defined on the common validation test is inappropriate for a
ODP developer (whatever the reason), that developer can overwrite this
test (or part of it, such as its setup) with an appropriate one on the
platform side.
Kallray, AFAIK, if using the platform trick for both OS and HW divergence.

Why not the same approach on the helper side? We give helpers for
{linux/PC} and provide a way to redefine these function for those who
need to,  on the platform side (e.g. using a weak symbol on the common
helpers and letting those who need define the same function as
"strong" on the helper platform side?... or any other better trick).

Then, any implementer knows where to place their hacks: on the
"platform side". Maybe not best, but at least consistent and clear.

That would just give helpers, with different platform (for any reason)
implementations.

The linux-specific helper would of course not fit on the common helper
part as they are linux specific and cannot be implementation on
different OS (platform). Once again, I don't understand why we needed
to do wrappers around well known Linux system calls: IMHO, helpers
should contains functions that we expect to be delivered on any
implementation (OS/HW) helpers, so these linux wrappers don't fit
there.
If we really want to keep that (I'd vote against), let's create
another directory name for what it is: "linux-wrappers"... and
separate them from function we'd expect to see everywhere (the rest of
the helpers, even if different platform dependent implementations for
those might exist)

Christophe

On 13 February 2017 at 16:09, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolai...@nokia-bell-labs.com> wrote:
>
>
> From: Mike Holmes [mailto:mike.hol...@linaro.org]
> Sent: Monday, February 13, 2017 5:02 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) 
> <petri.savolai...@nokia-bell-labs.com>
> Cc: lng-odp <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to 
> linux helpers
>
>
>
> On 13 February 2017 at 09:41, Savolainen, Petri (Nokia - FI/Espoo) 
> <mailto:petri.savolai...@nokia-bell-labs.com> wrote:
>
>
> From: Mike Holmes [mailto:mailto:mike.hol...@linaro.org]
> Sent: Friday, February 10, 2017 5:02 PM
> To: Petri Savolainen <mailto:petri.savolai...@linaro.org>
> Cc: lng-odp <mailto:lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to 
> linux helpers
>
>
>
> On 3 February 2017 at 06:23, Petri Savolainen 
> <mailto:mailto:petri.savolai...@linaro.org> wrote:
> There's no platform specific helpers. Helpers may depend on
> Linux and make it easier to do common series of Linux system
> calls. These kind of helpers are grouped into helper/linux
> directory.
>
> Use --enable-helper-linux configuration option to enable
> support for Linux helpers.
>
> Signed-off-by: Petri Savolainen <mailto:mailto:petri.savolai...@linaro.org>
>
> Reviewed-by: Mike Holmes <mailto:mailto:mike.hol...@linaro.org>
>
> required 3 way apply
>
>
>
> This should be merged. It moves linux dependent helper code under linux 
> folder. If there would be helper code dependent on some other OS, that would 
> go to their own helper/OS_foo/ folder. All Linux based ODP implementations 
> can run this helper code - there is no need to have N different linux helpers 
> for N different Linux based implementations.
>
>
> Also, the build is currently broken in master for --enable-helper-extn. E.g. 
> OFP cannot be built against it.
>
> This is not broken IMHO, it is by design, OFP depends on the helpers, we need 
> helpers to either be support for apps that we maintain and don't use (current 
> case) or we enforce  OFP and others to use the abstract API that all our own 
> executable use. Maintaining code we don't use is dangerous, you should have 
> to choose to do it, but I like the rename, that is fine and is progress, 
> hence the review.
>
> I think splitting out all of all the executables  will make this much cleaner.
> All ODP upstream executables will use the abstract API and be portable to any 
> platform and OS. Meanwhile the odp-Linux repo can have a Linux specific apis 
> such as the legacy one OFP is using and that is just that implementations 
> choice, as we have said there is no requirement to use helpers, or linux, and 
> OFP might want to attach itself to that,  but the tests and examples must 
> remain agnostic until we state we are dropping that goal.
>
>
> Please try:
> ./configure --enable-helper-extn
> make
>
> It does not compile, due to:
>
> platform/linux-generic/thread.c:282:12: error: 
> \u2018odph_linux_thread_create\u2019 defined but not used 
> [-Werror=unused-function]
>  static int odph_linux_thread_create(odph_odpthread_t *thread_tbl,
>
>
> My patch fixes this also.
>
>
> -Petri
>
>

Reply via email to