This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Optimization of conditional stores (was: Re: [PATCH] Add adaptive elision to rwlocks)


On Mon, 2014-04-07 at 19:52 +0200, Andi Kleen wrote:
> On Mon, Apr 07, 2014 at 08:54:57PM +0400, Alexander Monakov wrote: 
> > > > I would also suggest making the intent (perform the store only when necessary)
> > > > explicit, and make sure to disallow the compiler optimization, for example:
> > > > 
> > > >     if (*ptr != value)
> > > >         *(volatile typeof(*ptr)*)ptr = value;
> > > 
> > > That's really ugly.
> > 
> > I simply expanded kernel's ACCESS_ONCE macro by hand for the sake of the
> > example.
> 
> Ok I guess we can add a ACCESS_ONCE()
> 
> The existing elision code also uses this construct.

I'm not a kernel developer, but I think there ACCESS_ONCE is rather used
to get reads in spin-loops right (in terms of forward progress).  If we
transition to C11 atomics eventually, that specific workaround won't be
needed because just having an atomic load in a spin loop will be enough
to tell the compiler to not optimize the load out.

However, this case here is different because it's not about forward
progress but about whether idempotent stores are fine or not.  I guess
it's really a case of inefficient code generation if the compiler is
optimizing out checks for idempotent stores.  Alexander's example is
slightly different because there, the compiler sees that the location
just has been written to.  It shouldn't assume this in the lock elision
code.

Thus, I think it's not worth adding (something else than) ACCESS_ONCE
here; rather, someone should look at the compiler.  Maybe the case would
be clearer for the compiler if we'd actually used atomic accesses and
atomic-typed variables (because for synchronizing code, it's more likely
to be inefficient if you just store, idempotent or not...).


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]