This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 5/8] Move i486/pthread_spin_trylock.S to pthread_spin_trylock.S
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Roland McGrath <roland at hack dot frob dot com>, Torvald Riegel <triegel at redhat dot com>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>, Joseph Myers <joseph at codesourcery dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Wed, 26 Aug 2015 17:06:47 -0400
- Subject: Re: [PATCH 5/8] Move i486/pthread_spin_trylock.S to pthread_spin_trylock.S
- Authentication-results: sourceware.org; auth=none
- References: <20150826173721 dot GF21261 at intel dot com> <alpine dot DEB dot 2 dot 10 dot 1508261800370 dot 13146 at digraph dot polyomino dot org dot uk> <CAMe9rOp7zoMCsMLY2WoEucLKkrR6oJZg9XDJa2S-OJmfA08C8A at mail dot gmail dot com> <1440622616 dot 30828 dot 46 dot camel at localhost dot localdomain> <20150826210102 dot 47F1A2C39E3 at topped-with-meat dot com>
On 08/26/2015 05:01 PM, Roland McGrath wrote:
>> Could you instead please try to replace the custom asm implementation
>> with a C implementation, preferably the generic one in
>> nptl/pthread_spin_trylock.c? And, if necessary, and improve the latter
>> if there is a significant performance difference for uncontended locks?
>
> As I just said in another thread, meaningful changes such as that should
> not be conflated with the mechanical directory restructuring HJ is doing
> now. Of course, doing removals first reduces the number of renamings
> required. But the directory restructuring is entirely mechanical and so
> its review can consist of verifying that nothing materially changed in the
> build whatsoever. Meaningful changes require meaningful review.
>
> Directory restructuring changes, when generally desireable, should never be
> gated on unrelated meaningful changes. So if HJ wants to do those
> meaningful improvements first, that's fine. But nobody should be objecting
> to the directory restructuring changes on the grounds that some other
> meaningful change is also desireable.
+1
c.