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: [PATCH roland/lll_robust_trylock] Get rid of lll_robust_trylock.


On Wed, Jul 09, 2014 at 04:00:11PM -0700, Roland McGrath wrote:
> As discussed in the "[PATCH roland/lll-futex]" thread, every definition of
> lll_robust_trylock is either the same or has a bug.  It has exactly one
> user, and that user already presumes exactly what it must do.  So it's
> better to just get rid of it.
> 
> On x86 (GCC 4.6.3), this affected the generated code nontrivially, but only
> in good ways.  The compiler made some different register allocation and
> scheduling decisions.  It also noticed an opportunity to fuse redundant
> code that it had overlooked before.  I'm not really sure why it couldn't
> see it before.  My only guess is that it made some pessimistic assumption
> before when it was dealing with an asm (the old lll_robust_trylock) and now
> it groks more because atomic_compare_and_exchange_val_acq is actually just
> __sync_val_compare_and_swap rather than hand-coded asm.
> 
> On machines that had the "!= 0" comparison (aarch64, alpha, arm, hppa,
> ia64, microblaze, mips, s390, sh, sparc, and tile) this should affect the
> generated code slightly and that should be fixing a latent performance bug.
> 
> I'm not going to wait for every machine maintainer to chime in.  But I do
> want some review both that the new logic seems to be more right on the
> machines above where it's intentionally changing the acutal logic, and that
> the x86_64/i686 changes to generated code do only neutral or good things
> compared to the old assembly code (which I think had the right logic
> already, so no logic should be changed there).
> 
Yes, x86_64 looks ok. Difference is because compiler now has more
information. in assembly before it needed to assume that it could
overwrite memory in arbitrary way so it cannot move loads across.


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