RE: AIX support

From: Srirama Kucherlapati <sriram(dot)rk(at)in(dot)ibm(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Heikki Linnakangas <hlinnaka(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Noah Misch <noah(at)leadboat(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Andres Freund <andres(at)anarazel(dot)de>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, "tvk1271(at)gmail(dot)com" <tvk1271(at)gmail(dot)com>, "postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com" <postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>
Subject: RE: AIX support
Date: 2024-09-13 11:49:24
Message-ID: DS0PR15MB5623ECC0499C5087EFD729EADB652@DS0PR15MB5623.namprd15.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> The PPC asm code was originally written in 2002, and the first use of
> _ sync_lock_test_and_set(), for ARM, appeared in 2012. The commit that
> made __sync_lock_test_and_set() be chosen automatically for platforms
> that don't specify anything else was added in 2022.
Thanks for the info.

------------------
> Ok, if we don't need the assembler code at all, that's good. A patch to
> introduce AIX support should not change it for non-AIX powerpc systems
> though. That might be a good change, but would need to be justified
> separately, e.g. by some performance testing, and should be a separate
> patch.

> If you make no changes to s_lock.h at all, will it work? Why not?
With the existing asm code I see there are some syntax errors, being hit.
But after reverting the old changes the issues resolved. Below are diffs.

static __inline__ int
tas(volatile slock_t *lock)
{
@@ -424,17 +413,15 @@ tas(volatile slock_t *lock)
__asm__ __volatile__(
" lwarx %0,0,%3,1 \n"
" cmpwi %0,0 \n"
-" bne 1f \n"
+" bne $+16 \n" /* branch to li %1,1 */
" addi %0,%0,1 \n"
" stwcx. %0,0,%3 \n"
-" beq 2f \n"
-"1: \n"
+" beq $+12 \n" /* branch to lwsync */
" li %1,1 \n"
-" b 3f \n"
-"2: \n"
+" b $+12 \n" /* branch to end of asm sequence */
" lwsync \n"
" li %1,0 \n"
-"3: \n"
+
: "=&b"(_t), "=r"(_res), "+m"(*lock)
: "r"(lock)
: "memory", "cc");

Let me know if I need to run any perf tools to check the performance of
the __sync_lock_test_and_set change.

---------------
> > I mean this part of the code is needed as this is specific to AIX kernel memory
> > operation which is different from __sync_lock_test_and_set().
> >
> > I would like to mention that the changes made in src/include/storage/s_lock.h
> > are pretty much required and need to be operated in assemble specific to IBM
> > Power architecture.

> Was that earlier statement incorrect? Is the manual wrong or outdated or
> not applicable to us?

Here this change is specific to AIX, but since we are compiling with gcc, this
is not applicable. But I will try with __sync* routines and check.

---------------
> Do you still need mkldexport.sh? Surely there's a better way to do that
> in year 2024. Some quick googling says there's a '-bexpall' option to
> 'ld', which kind of sounds like what we want. Will that work? How do
> other programs do this?

Thanks for looking into this, I’m working on this, I will let you know.

Thanks,
Sriram.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Seino Yuki 2024-09-13 11:49:36 Add “FOR UPDATE NOWAIT” lock details to the log.
Previous Message Tomas Vondra 2024-09-13 11:35:35 Re: Why don't we consider explicit Incremental Sort?