Re: [Patch][aarch64] Use IFUNCs to enable LSE instructions in libatomic on aarch64

[James Greenhalgh <james.greenhalgh@arm.com>]

On Mon, Nov 20, 2017 at 07:22:15PM +0000, Steve Ellcey wrote:
> On Mon, 2017-11-20 at 18:27 +0000, James Greenhalgh wrote:
> > 
> > If you have the time, would you mind giving me a quick run-down of what
> > design decisions went in to this patch, and why they are the right thing
> > to do? Sorry to offload that, but it will be the most efficient route
> > to a review.
> 
> The main design decision was to use the existing IFUNC infrastructure
> that is used on ARM32 to enable atomic instructions that were added
> with armv7-a, on i386 to enable instructions added with i586, and on
> x86_64 to enable instructions added with cx16.
> 
> The basic idea for all these is to allow users who create programs that
> use the atomic_* functions to use new instructions on machines that
> support them while also working on older machines that do not support
> them and to not have to create two separate executables.
> 
> Some atomic_* functions get inlined into programs, and those will
> either use or not use LSE instructions based on the compiler arguments
> used during compilations.  If you want your program to work on all
> machines you have to not compile for LSE intructions.  But other
> functions (or all functions if -fno-inline-atomics is used) will call
> the libatomic library.  Currently those functions do not use LSE
> instructions but with this patch we can use the IFUNC infrastructure to
> check for LSE support and use LSE in libatomic on machines where it is
> supported or not use it on machines where it is not supported.
> 
> As an example of what this change does, __atomic_compare_exchange_8 will
> turn into a call to libat_compare_exchange_8_i1 on a machine that supports
> LSE:
> 
> 0000000000000000 <libat_compare_exchange_8_i1>:
>    0:	f9400023 	ldr	x3, [x1]
>    4:	aa0303e4 	mov	x4, x3
>    8:	c8e4fc02 	casal	x4, x2, [x0]
>    c:	eb03009f 	cmp	x4, x3
>   10:	1a9f17e0 	cset	w0, eq
>   14:	35000040 	cbnz	w0, 1c <libat_compare_exchange_8_i1+0x1c>
>   18:	f9000024 	str	x4, [x1]
>   1c:	d65f03c0 	ret
> 
> But call libat_compare_exchange_8 on a machine without LSE:
> 
> 0000000000000000 <libat_compare_exchange_8>:
>    0:	f9400023 	ldr	x3, [x1]
>    4:	c85ffc04 	ldaxr	x4, [x0]
>    8:	eb03009f 	cmp	x4, x3
>    c:	54000061 	b.ne	18 <libat_compare_exchange_8+0x18>
>   10:	c805fc02 	stlxr	w5, x2, [x0]
>   14:	35ffff85 	cbnz	w5, 4 <libat_compare_exchange_8+0x4>
>   18:	1a9f17e0 	cset	w0, eq
>   1c:	34000040 	cbz	w0, 24 <libat_compare_exchange_8+0x24>
>   20:	d65f03c0 	ret
>   24:	f9000024 	str	x4, [x1]
>   28:	d65f03c0 	ret
>   2c:	d503201f 	nop

Thanks for the detailed explanation. I understood this, and my opinion is
that the AArch64 parts of this patch are OK (and I don't know who needs to
Ack the small generic changes you require).

Let's give Richard/Marcus 48 hours to object while we wait for an OK on the
generic bits, and then OK for AArch64.

Thanks,
James

Reviewed-By: James Greenhalgh <james.greenhalgh@arm.com>