Re: [PATCH] Mirror i386 change on sparc: 'Avoid "anonymous" codein pthread_spin_lock'

[Chris Metcalf <cmetcalf at tilera dot com>]

On 5/2/2012 10:23 PM, David Miller wrote:
> diff --git a/nptl/sysdeps/sparc/sparc32/pthread_spin_lock.S b/nptl/sysdeps/sparc/sparc32/pthread_spin_lock.S
> [...]
> \ No newline at end of file

That's probably not good.

> -int
> -pthread_spin_lock (pthread_spinlock_t *lock)
> -{
> -  __asm __volatile
> -    ("1: ldstub [%0], %%g2\n"
> -     "   orcc   %%g2, 0x0, %%g0\n"
> -     "   bne,a  2f\n"
> -     "   ldub   [%0], %%g2\n"
> -     ".subsection 2\n"
> -     "2: orcc   %%g2, 0x0, %%g0\n"
> -     "   bne,a  2b\n"
> -     "   ldub   [%0], %%g2\n"
> -     "   b,a    1b\n"
> -     ".previous"
> -     : /* no outputs */
> -     : "r" (lock)
> -     : "g2", "memory", "cc");
> -  return 0;
> -}

Wouldn't you get the same result as the assembly code just by removing the
.subsection and .previous directives?  In that case the code would all be
under the C function label, which is the point of the change.

That said, the .S file approach is probably cleaner here anyway, just
because the entire function is in assembly, so the C wrapper provides no
value (and a lot of gratuitous double-quotes and "\n").

> -int
> -pthread_spin_trylock (pthread_spinlock_t *lock)
> -{
> -  int res;
> -  __asm __volatile ("ldstub [%1], %0" : "=r" (res) : "r" (lock) : "memory");
> -  return res == 0 ? 0 : EBUSY;
> -}

But the cases where the assembly is more inline (as here) do seem like
plausible candidates to keep in C, rather than convert.  What's the advantage?

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com