On 15 December 2014 at 06:40, Jerin Jacob
<jerin.ja...@caviumnetworks.com> wrote:
> On Fri, Dec 12, 2014 at 10:56:26PM +0100, Ola Liljedahl wrote:
>> On 12 December 2014 at 16:59, Mike Holmes <mike.hol...@linaro.org> wrote:
>> >
>> >
>> > On 12 December 2014 at 10:51, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
>> >>
>> >> On 12/12/2014 06:47 PM, Taras Kondratiuk wrote:
>> >>>
>> >>> On 12/12/2014 05:03 PM, Maxim Uvarov wrote:
>> >>>>
>> >>>> Odp atomic operations based on compiler build-ins. Make
>> >>>> sure that compiler supports such operation at configure
>> >>>> stage.
>> >>>
>> >>> This check should be limited to platforms that use gcc atomics.
>> >>
>> >> Do we have such platforms?
>> >
>> >
>> > __OCTEON__ directly swappes out gcc, infact that pre processor switch
>> > affects other files in linux-generic too.
>> >
>> > 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
>> > }
>> This is an OCTEON-special for just one function. I didn't want to
>> remove it. But I am not sure it is actually needed. Shouldn't the
>> compiler be able to generate the appropriate OCTEON-specific
>> instructions (e.g. laa(d), saa(d), lai(d), lad(d) etc), gcc has
>> supported OCTEON since version 4.4? Also I don't understand the reason
>> for the syncw *before* the lai (load-atomic-increment) but perhaps the
>> load-atomic instructions (which shouldn't linger in the write buffer
>> like stores) need to be treated differently.
>
> I generated the dis-assembly of octeon gcc generated output,
> Compiler does generate appropriate instruction for __atomic_fetch*
> instructions.
So no need for any OCTEON-specific inline assembler code then?
> and if the contract is "__ATOMIC_RELAXED" then "syncws" can be removed.
The ordering contract here is now relaxed. The public use case for
odp_atomics.h is for software counters, no need to waste performance
with unnecessary ordering requirements.
Have you checked the code generation for the _odp_atomic_internal.h
functions? E.g. for acquire and release memory orderings. I assume
that GCC will generate correct code but don't know if it actually
generates optimal code.
>
> Typical use case for putting "syncw" before the lai operation to make sure
> cpu does not change the order of "lai" and "instructions before it"
> as octeon is weakly ordered cpu for better performance.
This would be release ordering which is often associated with unlock
type of operations. But the type of ordering you need depends on the
usage of the atomic operation which is unknown to the atomic operation
itself.
Is there any other case where "syncw" should still be used, e.g. for
performance if not for correctness reasons? Flush the write buffer on
release type of operations. We need a better name for this than
odp_sync_stores().
I know MIPS is weakly ordered but I was under the impression that the
OCTEON micro-architectures are more strongly ordered (at least the
first OCTEON implementations were in-order).
>
>>
>> I think this OCTEON-fix should either be removed or there should be
>> specials for most if not all of the functions in odp_atomic.h
>
> Yes, It can be removed if the contract is "__ATOMIC_RELAXED".
Great. I can post a patch.
>
> Do we have any specific reason to keep the API as odp_atomic_*, if its
> an "__ATOMIC_RELAXED" memory model then its better to rename as counters.
This was my original idea as well.
>
>
>> >
>> >
>> >>
>> >>
>> >>
>> >> Maxim.
>> >>
>> >>
>> >> _______________________________________________
>> >> lng-odp mailing list
>> >> lng-odp@lists.linaro.org
>> >> http://lists.linaro.org/mailman/listinfo/lng-odp
>> >
>> >
>> >
>> > --
>> > Mike Holmes
>> > Linaro Sr Technical Manager
>> > LNG - ODP
>> >
>> > _______________________________________________
>> > lng-odp mailing list
>> > lng-odp@lists.linaro.org
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
>> >
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
