On 12/23/2014 10:40 PM, Mike Holmes wrote:
On 23 December 2014 at 13:49, Bill Fischofer
<[email protected] <mailto:[email protected]>> wrote:
That's good to know and certainly the code is cleaner that way. I
think we should adopt the practice that any architecture-specific
changes to linux-generic code (adds, updates, or removals) must
either originate from or have a Reviewed-by a representative of
that architecture (in this case, Bala). That way there's no
confusion as to whether the known experts agree with the change,
since these sort of things can have subtle implications that are
not obvious to the layman.
I think linux-generic should be devoid of any per core optimizations
in its default state.
There could be in an arch directory and it would be supported as you
say Bill by someone who will care for that that variant, this also
avoids filling linux-generic with a possibly large list of
preprocessor flags that we can't test.
In this case we don't have public access to the octeon to even know if
this code works so separating or deletion feels right to me, although
Cavium are adding their board in this case.
To have optimizations we should understand which per arch optimization
we need. I see that if newer gcc supports good optimizations then we
need to use that gcc. Most of people use Open Embedded and Linaro gcc
which is almost latest version. So if there is no optimization in
standard environment (gcc, kernel, libc) then arch implementation should
go to platform specific git variant. If it's covered by standard
environment then we simple updating minimal version requirement for
required component.
Maxim.
On Tue, Dec 23, 2014 at 12:38 PM, Ola Liljedahl
<[email protected] <mailto:[email protected]>> wrote:
I had some discussions with Bala and we think the GCC
generates the
appropriate code for OCTEON so there should be no need for any
OCTEON-specific inline assembler in odp_atomic.h. I just
didn't want
to remove this piece of code when I redesigned the atomic support
because I didn't understand why there is a SYNCW instruction
*before*
the load. Load-acquire type of loads may need sync or barrier
instructions but they come after the load, not before.
I think we should trust GCC to generate better code for OCTEON
than us
mere humans, at least until proven wrong. So please remove this
OCTEON-specific snippet.
-- Ola
On 23 December 2014 at 18:01, Mike Holmes
<[email protected] <mailto:[email protected]>> wrote:
>
>
> On 23 December 2014 at 11:15, Taras Kondratiuk
<[email protected] <mailto:[email protected]>>
> wrote:
>>
>> On 12/23/2014 05:28 PM, Mike Holmes wrote:
>> > how will this work, Octeon does not care about
__atomic_fetch_add(&v,
>> > 1, __ATOMIC_RELAXED); and yet you will fail configure I
assume
>> >
>> > static inline uint32_t
odp_atomic_fetch_inc_u32(odp_atomic_u32_t *atom)
>> > {
>> > #if defined __OCTEON__
>> >>-------uint32_t ret;
>> >>-------__asm__ __volatile__ ("syncws");
>> >>-------__asm__ __volatile__ ("lai %0,(%2)" : "=r" (ret),
"+m" (atom) :
>> >>------->------->------- "r" (atom));
>> >>-------return ret;
>> > #else
>> >>-------return __atomic_fetch_add(&atom->v, 1,
__ATOMIC_RELAXED);
>> > #endif
>> > }
>>
>> Linux-generic shouldn't have arch specific parts, because it is
>> *generic*. Instead it can fall back to strong __sync
functions if
>> __atomic are not available.
>>
>
> Linux kernel is generic but has core specifics - is this not
the same ?
>
> What about these cases ?
>
> platform/linux-generic/odp_system_info.c:static int
cpuinfo_octeon(FILE
> *file, odp_system_info_t *sysinfo)
> platform/linux-generic/odp_system_info.c: .cpu_arch_str =
"octeon",
> platform/linux-generic/odp_system_info.c: .cpuinfo_parser =
> cpuinfo_octeon
> platform/linux-generic/include/api/odp_align.h:#if defined
__x86_64__ ||
> defined __i386__
> platform/linux-generic/include/api/odp_sync.h:#if defined
__x86_64__ ||
> defined __i386__
> platform/linux-generic/include/odp_spin_internal.h:#if
defined __x86_64__ ||
> defined __i386__
> platform/linux-generic/odp_system_info.c:#if defined
__x86_64__ || defined
> __i386__ || defined __OCTEON__ || \
> platform/linux-generic/odp_system_info.c:#if defined
__x86_64__ || defined
> __i386__
> platform/linux-generic/odp_system_info.c:static int
cpuinfo_x86(FILE *file,
> odp_system_info_t *sysinfo)
> platform/linux-generic/odp_system_info.c: #if defined
__x86_64__ ||
> defined __i386__
> platform/linux-generic/odp_system_info.c: .cpu_arch_str =
"x86",
> platform/linux-generic/odp_system_info.c: .cpuinfo_parser =
> cpuinfo_x86
> platform/linux-generic/odp_system_info.c:#if defined
__x86_64__ || defined
> __i386__ || defined __OCTEON__ || \
> platform/linux-generic/odp_time.c:#if defined __x86_64__ ||
defined __i386__
> platform/linux-generic/include/api/odp_align.h:#elif defined
__arm__ ||
> defined __aarch64__
> platform/linux-generic/include/api/odp_sync.h:#elif
defined(__arm__)
> platform/linux-generic/include/odp_spin_internal.h:#elif
defined __arm__
> platform/linux-generic/odp_system_info.c:#elif defined
__arm__ || defined
> __aarch64__
> platform/linux-generic/odp_system_info.c:static int
cpuinfo_arm(FILE *file
> ODP_UNUSED,
> platform/linux-generic/odp_system_info.c: #elif defined
__arm__ ||
> defined __aarch64__
> platform/linux-generic/odp_system_info.c: .cpu_arch_str =
"arm",
> platform/linux-generic/odp_system_info.c: .cpuinfo_parser =
> cpuinfo_arm
>
>
>>
>>
>> Taras Kondratiuk
>
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
http://lists.linaro.org/mailman/listinfo/lng-odp
--
*Mike Holmes*
Linaro Sr Technical Manager
LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
