Thread: [HACKERS] [PATCH] Enabling atomics on ARM64

[HACKERS] [PATCH] Enabling atomics on ARM64

From

Roman Shaposhnik

Date:

09 March 2017, 09:18:04

Hi!

I'd like to offer a forward port from a change I'm contributing
the Greenplum code base over here:
    https://github.com/greenplum-db/gpdb/pull/1983

While it is true that 64 bit atomics are mostly a no-op
for Postgres right now, perhaps this patch could still
be useful.

To make it easier to review, let me repeat the analysis I did
on the pull request in this email:

I built the following tiny example with -fno-inline-small-functions to
make sure you get to see all the guts (without the
-fno-inline-small-functions all of this gets inlined and somewhat
further optimized):

extern pg_atomic_uint64 test1;
extern uint64 test2;
extern uint64 test3;

int main() {
   pg_atomic_compare_exchange_u64(&test1, &test2, test3);
   pg_atomic_add_fetch_u64(&test1, 1);
   pg_atomic_add_fetch_u64(&test1, 2);
}

Here's the assembly I got from it:

Disassembly of section .text:

0000000000000000 <pg_atomic_compare_exchange_u64_impl>:
   0: d10083ff sub sp, sp, #0x20
   4: f9000fe0 str x0, [sp,#24]
   8: f9000be1 str x1, [sp,#16]
   c: f90007e2 str x2, [sp,#8]
  10: f9400fe1 ldr x1, [sp,#24]
  14: f9400be0 ldr x0, [sp,#16]
  18: f9400003 ldr x3, [x0]
  1c: f94007e4 ldr x4, [sp,#8]
  20: c85ffc22 ldaxr x2, [x1]
  24: eb03005f cmp x2, x3
  28: 54000061 b.ne 34 <pg_atomic_compare_exchange_u64_impl+0x34>
  2c: c805fc24 stlxr w5, x4, [x1]
  30: 35ffff85 cbnz w5, 20 <pg_atomic_compare_exchange_u64_impl+0x20>
  34: 1a9f17e1 cset w1, eq
  38: 7100003f cmp w1, #0x0
  3c: 54000041 b.ne 44 <pg_atomic_compare_exchange_u64_impl+0x44>
  40: f9000002 str x2, [x0]
  44: 2a0103e0 mov w0, w1
  48: 910083ff add sp, sp, #0x20
  4c: d65f03c0 ret

0000000000000050 <pg_atomic_fetch_add_u64_impl>:
  50: d10043ff sub sp, sp, #0x10
  54: f90007e0 str x0, [sp,#8]
  58: f90003e1 str x1, [sp]
  5c: f94007e0 ldr x0, [sp,#8]
  60: f94003e1 ldr x1, [sp]
  64: c85f7c02 ldxr x2, [x0]
  68: 8b010043 add x3, x2, x1
  6c: c804fc03 stlxr w4, x3, [x0]
  70: 35ffffa4 cbnz w4, 64 <pg_atomic_fetch_add_u64_impl+0x14>
  74: d5033bbf dmb ish
  78: aa0203e0 mov x0, x2
  7c: 910043ff add sp, sp, #0x10
  80: d65f03c0 ret

0000000000000084 <pg_atomic_add_fetch_u64_impl>:
  84: a9be7bfd stp x29, x30, [sp,#-32]!
  88: 910003fd mov x29, sp
  8c: f9000fa0 str x0, [x29,#24]
  90: f9000ba1 str x1, [x29,#16]
  94: f9400ba1 ldr x1, [x29,#16]
  98: f9400fa0 ldr x0, [x29,#24]
  9c: 97ffffed bl 50 <pg_atomic_fetch_add_u64_impl>
  a0: aa0003e1 mov x1, x0
  a4: f9400ba0 ldr x0, [x29,#16]
  a8: 8b000020 add x0, x1, x0
  ac: a8c27bfd ldp x29, x30, [sp],#32
  b0: d65f03c0 ret

00000000000000b4 <pg_atomic_compare_exchange_u64>:
  b4: a9bd7bfd stp x29, x30, [sp,#-48]!
  b8: 910003fd mov x29, sp
  bc: f90017a0 str x0, [x29,#40]
  c0: f90013a1 str x1, [x29,#32]
  c4: f9000fa2 str x2, [x29,#24]
  c8: f9400fa2 ldr x2, [x29,#24]
  cc: f94013a1 ldr x1, [x29,#32]
  d0: f94017a0 ldr x0, [x29,#40]
  d4: 97ffffcb bl 0 <pg_atomic_compare_exchange_u64_impl>
  d8: 53001c00 uxtb w0, w0
  dc: a8c37bfd ldp x29, x30, [sp],#48
  e0: d65f03c0 ret

00000000000000e4 <pg_atomic_add_fetch_u64>:
  e4: a9be7bfd stp x29, x30, [sp,#-32]!
  e8: 910003fd mov x29, sp
  ec: f9000fa0 str x0, [x29,#24]
  f0: f9000ba1 str x1, [x29,#16]
  f4: f9400ba1 ldr x1, [x29,#16]
  f8: f9400fa0 ldr x0, [x29,#24]
  fc: 97ffffe2 bl 84 <pg_atomic_add_fetch_u64_impl>
 100: a8c27bfd ldp x29, x30, [sp],#32
 104: d65f03c0 ret

0000000000000108 <main>:
 108: a9bf7bfd stp x29, x30, [sp,#-16]!
 10c: 910003fd mov x29, sp
 110: 90000000 adrp x0, 0 <_GLOBAL_OFFSET_TABLE_>
 114: f9400000 ldr x0, [x0]
 118: f9400002 ldr x2, [x0]
 11c: 90000000 adrp x0, 0 <_GLOBAL_OFFSET_TABLE_>
 120: f9400001 ldr x1, [x0]
 124: 90000000 adrp x0, 0 <_GLOBAL_OFFSET_TABLE_>
 128: f9400000 ldr x0, [x0]
 12c: 97ffffe2 bl b4 <pg_atomic_compare_exchange_u64>
 130: 90000000 adrp x0, 0 <_GLOBAL_OFFSET_TABLE_>
 134: f9400000 ldr x0, [x0]
 138: d2800021 mov x1, #0x1                   // #1
 13c: 97ffffea bl e4 <pg_atomic_add_fetch_u64>
 140: 90000000 adrp x0, 0 <_GLOBAL_OFFSET_TABLE_>
 144: f9400000 ldr x0, [x0]
 148: d2800041 mov x1, #0x2                   // #2
 14c: 97ffffe6 bl e4 <pg_atomic_add_fetch_u64>
 150: 52800000 mov w0, #0x0                   // #0
 154: a8c17bfd ldp x29, x30, [sp],#16
 158: d65f03c0 ret


Thanks,
Roman.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

0001-Enabling-atomics-on-ARM64.patch

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Robert Haas

Date:

10 March 2017, 00:01:07

On Wed, Mar 8, 2017 at 10:18 PM, Roman Shaposhnik <rvs@apache.org> wrote:
> I'd like to offer a forward port from a change I'm contributing
> the Greenplum code base over here:
>     https://github.com/greenplum-db/gpdb/pull/1983

Thanks.   Please add this to https://commitfest.postgresql.org/14/ so
it doesn't get forgotten.  I'm afraid your patch is probably too late
for v10 (the deadline for patches to be considered for v10 was March
1st, though somebody might propose to make an exception), but v11 will
be here soon enough.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Michael Paquier

Date:

10 March 2017, 07:29:39

On Fri, Mar 10, 2017 at 3:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Mar 8, 2017 at 10:18 PM, Roman Shaposhnik <rvs@apache.org> wrote:
>> I'd like to offer a forward port from a change I'm contributing
>> the Greenplum code base over here:
>>     https://github.com/greenplum-db/gpdb/pull/1983

+1.

> Thanks.   Please add this to https://commitfest.postgresql.org/14/ so
> it doesn't get forgotten.  I'm afraid your patch is probably too late
> for v10 (the deadline for patches to be considered for v10 was March
> 1st, though somebody might propose to make an exception), but v11 will
> be here soon enough.

Yeah, one month earlier would have been fine though. This is not
really invasive.
-- 
Michael

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Andres Freund

Date:

10 March 2017, 07:54:44

On 2017-03-10 10:29:39 +0900, Michael Paquier wrote:
> On Fri, Mar 10, 2017 at 3:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Wed, Mar 8, 2017 at 10:18 PM, Roman Shaposhnik <rvs@apache.org> wrote:
> >> I'd like to offer a forward port from a change I'm contributing
> >> the Greenplum code base over here:
> >>     https://github.com/greenplum-db/gpdb/pull/1983
> 
> +1.
> 
> > Thanks.   Please add this to https://commitfest.postgresql.org/14/ so
> > it doesn't get forgotten.  I'm afraid your patch is probably too late
> > for v10 (the deadline for patches to be considered for v10 was March
> > 1st, though somebody might propose to make an exception), but v11 will
> > be here soon enough.
> 
> Yeah, one month earlier would have been fine though. This is not
> really invasive.

FWIW, Unless somebody seriously protests, I'm going to commit this soon-ish.

- Andres

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Tom Lane

Date:

10 March 2017, 11:03:46

Andres Freund <andres@anarazel.de> writes:
> FWIW, Unless somebody seriously protests, I'm going to commit this soon-ish.

Sure, if you want to take responsibility for it, push away.
        regards, tom lane

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Andres Freund

Date:

11 March 2017, 01:20:43

Hi,

On 2017-03-08 19:18:04 -0800, Roman Shaposhnik wrote:
> I'd like to offer a forward port from a change I'm contributing
> the Greenplum code base over here

I think the change makes sense.  Any chance we could convince you to
bring up an arm64 buildfarm animal (our test infrastructure) up, so we
can be sure arm64 stays working?  See https://buildfarm.postgresql.org/

Pushed, thanks for your contribution.

Greetings,

Andres Freund

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Roman Shaposhnik

Date:

11 March 2017, 08:14:13

On Fri, Mar 10, 2017 at 11:20 AM, Andres Freund <andres@anarazel.de> wrote:
> Hi,
>
> On 2017-03-08 19:18:04 -0800, Roman Shaposhnik wrote:
>> I'd like to offer a forward port from a change I'm contributing
>> the Greenplum code base over here
>
> I think the change makes sense.  Any chance we could convince you to
> bring up an arm64 buildfarm animal (our test infrastructure) up, so we
> can be sure arm64 stays working?  See https://buildfarm.postgresql.org/

What does it take to be a your build farm slave? Can it be done via
a Docker container that calls out to your infrastructure? I can probably
run that as part of the Apache Bigtop CI.

> Pushed, thanks for your contribution.

Thanks for taking care of it so quickly!

Thanks,
Roman.

Re: [HACKERS] [PATCH] Enabling atomics on ARM64

From

Andres Freund

Date:

11 March 2017, 08:17:17

On 2017-03-10 18:14:13 -0800, Roman Shaposhnik wrote:
> On Fri, Mar 10, 2017 at 11:20 AM, Andres Freund <andres@anarazel.de> wrote:
> > Hi,
> >
> > On 2017-03-08 19:18:04 -0800, Roman Shaposhnik wrote:
> >> I'd like to offer a forward port from a change I'm contributing
> >> the Greenplum code base over here
> >
> > I think the change makes sense.  Any chance we could convince you to
> > bring up an arm64 buildfarm animal (our test infrastructure) up, so we
> > can be sure arm64 stays working?  See https://buildfarm.postgresql.org/
> 
> What does it take to be a your build farm slave? Can it be done via
> a Docker container that calls out to your infrastructure? I can probably
> run that as part of the Apache Bigtop CI.

Yes, it can.  The buildfarm stuff is all push from the test machine's
side, not pull.  You basically configure a cron job as described in
https://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto and then
register your animal with the form on
https://buildfarm.postgresql.org/cgi-bin/register-form.pl

Would be great. We have a bunch of 32bit arm animal, but no 64bit ones:
https://buildfarm.postgresql.org/cgi-bin/show_members.pl
https://buildfarm.postgresql.org/cgi-bin/show_status.pl

Regards,

Andres