public inbox for [email protected]  
help / color / mirror / Atom feed
From: Srirama Kucherlapati <[email protected]>
To: Heikki Linnakangas <[email protected]>
To: Heikki Linnakangas <[email protected]>
To: Tom Lane <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Laurenz Albe <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Bruce Momjian <[email protected]>
Subject: RE: AIX support
Date: Fri, 13 Sep 2024 11:49:24 +0000
Message-ID: <DS0PR15MB5623ECC0499C5087EFD729EADB652@DS0PR15MB5623.namprd15.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <CY5PR11MB639218D04CFAED0FC09ED9DFFDED2@CY5PR11MB6392.namprd11.prod.outlook.com>
	<[email protected]>
	<CY8PR15MB5602AA47C13A33DBF68BEA73DBFB2@CY8PR15MB5602.namprd15.prod.outlook.com>
	<[email protected]>
	<CY8PR15MB56029161FE37A1DE9DE36B70DBC72@CY8PR15MB5602.namprd15.prod.outlook.com>
	<CY8PR15MB5602CAFFE00371049D68CD25DBCD2@CY8PR15MB5602.namprd15.prod.outlook.com>
	<[email protected]>
	<CY8PR15MB560282A99569E110AEED3257DBCF2@CY8PR15MB5602.namprd15.prod.outlook.com>
	<CY8PR15MB56029A55A848E9089D29FBE6DBC92@CY8PR15MB5602.namprd15.prod.outlook.com>
	<CY8PR15MB5602C6D2B148615EBD7F1AF5DB872@CY8PR15MB5602.namprd15.prod.outlook.com>
	<[email protected]>
	<CY8PR15MB5602A5D9064A401347684A06DB872@CY8PR15MB5602.namprd15.prod.outlook.com>
	<[email protected]>
	<CY8PR15MB5602B407BBD1B7B7C5FF1D8CDB9B2@CY8PR15MB5602.namprd15.prod.outlook.com>
	<[email protected]>


    > 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.




view thread (20+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: AIX support
  In-Reply-To: <DS0PR15MB5623ECC0499C5087EFD729EADB652@DS0PR15MB5623.namprd15.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox