Re: [PATCH v2] Remove atomic_compare_and_exchange_bool_rel.

["Tulio Magno Quites Machado Filho" <tuliom at linux dot vnet dot ibm dot com>]

Torvald Riegel <triegel@redhat.com> writes:

> On Fri, 2016-06-17 at 14:33 -0300, Tulio Magno Quites Machado Filho
> wrote:
>> Torvald Riegel <triegel@redhat.com> writes:
>> 
>> > Removing this operation and the matching (unused) catomic_ operation
>> > seemed to be easier than fixing powerpc's definition of it, only for it
>> > to be removed anyway in the future.  There were just three call sites of
>> > it.
>> 
>> OK, but why can't we remove __arch_compare_and_exchange_bool_*_rel from
>> powerpc[32|64] right now?
>> AFAICS, they're all unused.
>
> Right. I've removed them in the attached revised patch.
>
>> For the record, microblaze seems to have a few unused lines as well.
>
> Likewise.
>
> Note that for powerpc specifically, use of the new C11-like
> atomic_compare_exchange_weak_release will have acquire semantics too,
> unnecessarily.

I'm probably missing something here.

When atomic_compare_exchange_weak_release isn't available and
USE_ATOMIC_COMPILER_BUILTINS = 0, include/atomic.h:697 will use
atomic_compare_and_exchange_val_rel, which is defined on
sysdeps/powerpc/atomic-machine.h and has release semantics.

Is there something else preventing this from happening?

> This issue will disappear as soon as you (can) set
> USE_ATOMIC_COMPILER_BUILTINS to 1.  What's your plan for powerpc
> regarding this?

I believe this is a long-term goal, but I don't think we should work on this
in a hurry due to a bug in GCC [1] and because that will suppress some
flexibility such as the one I implemented in the past [2] and which is still
on my backlog pending changes.

My next step is to replace part of the inline asm by compiler built-ins.

> Is the currently required  GCC 4.7 sufficient, or what
> is the first GCC version where this would be possible?

>From an API perspective, I think it's sufficient.
But we still have to deal with the performance issue from [1].

>     Remove atomic_compare_and_exchange_bool_rel.
>     
>     atomic_compare_and_exchange_bool_rel and
>     catomic_compare_and_exchange_bool_rel are removed and replaced with the
>     new C11-like atomic_compare_exchange_weak_release.  The concurrent code
>     in nscd/cache.c has not been reviewed yet, so this patch does not add
>     detailed comments.

Tested on ppc as well.

LGTM, but I also believe we should wait for Lei's confirmation.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66867
[2] http://patchwork.sourceware.org/patch/11307/

-- 
Tulio Magno