Re: [PATCH v2] Remove atomic_compare_and_exchange_bool_rel.

[Torvald Riegel <triegel at redhat dot com>]

On Fri, 2016-06-17 at 19:17 -0300, Tulio Magno Quites Machado Filho
wrote:
> 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?

No, you're right, sorry.  Late-in-the-day reply.  I mixed this up with
the atomic RMW ops with release MO that I've been adding for the new
condvar.

> > 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.

I agree there's no real need to hurry.  All I want is for arch
maintainers to be aware of this and make the switch at the right time.

> 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.

Thanks.

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

Yes.