Message-ID: <e0dc2bff-300b-4edd-912b-b6c52274bdc5@iki.fi>
Date: Wed, 25 Dec 2024 12:22:14 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: AIX support
To: Srirama Kucherlapati <sriram.rk@in.ibm.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 "postgres-ibm-aix@wwpdl.vnet.ibm.com" <postgres-ibm-aix@wwpdl.vnet.ibm.com>
Cc: Bruce Momjian <bruce@momjian.us>, Robert Haas <robertmhaas@gmail.com>,
 Peter Eisentraut <peter@eisentraut.org>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Laurenz Albe <laurenz.albe@cybertec.at>, Noah Misch <noah@leadboat.com>,
 Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>,
 Thomas Munro <thomas.munro@gmail.com>, "tvk1271@gmail.com"
 <tvk1271@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
References: <CY5PR11MB639218D04CFAED0FC09ED9DFFDED2@CY5PR11MB6392.namprd11.prod.outlook.com>
 <7751f9c5-e2e6-4252-a9fa-14b3e78ddec9@iki.fi>
 <DS0PR15MB5623ECC0499C5087EFD729EADB652@DS0PR15MB5623.namprd15.prod.outlook.com>
 <CY8PR15MB5602561E7BF438A1B6C5371BDB612@CY8PR15MB5602.namprd15.prod.outlook.com>
 <CY8PR15MB5602F4F63D8F8811A5987EFFDB682@CY8PR15MB5602.namprd15.prod.outlook.com>
 <73dafd11-32f4-4238-b374-74eb917aa7fc@iki.fi>
 <CY8PR15MB56023A063EFA06C0BAB89442DB692@CY8PR15MB5602.namprd15.prod.outlook.com>
 <CY8PR15MB5602EB54EFE30D4C21F061BEDB722@CY8PR15MB5602.namprd15.prod.outlook.com>
 <CY8PR15MB5602A27B26F2D4FB16A86DDFDB2E2@CY8PR15MB5602.namprd15.prod.outlook.com>
 <CA+TgmoYedqK0gaJ6FYkyQ5kkSH3=OywnTLC1p6eOW760+rQABQ@mail.gmail.com>
 <CY8PR15MB5602F897E478A24903B480C4DB292@CY8PR15MB5602.namprd15.prod.outlook.com>
 <CY8PR15MB56025F74BDA3E078288A6A0ADB372@CY8PR15MB5602.namprd15.prod.outlook.com>
 <CY8PR15MB56026092DF51E9D300557B5ADB022@CY8PR15MB5602.namprd15.prod.outlook.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <CY8PR15MB56026092DF51E9D300557B5ADB022@CY8PR15MB5602.namprd15.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/e0dc2bff-300b-4edd-912b-b6c52274bdc5%40iki.fi>
Precedence: bulk

On 23/12/2024 17:55, Srirama Kucherlapati wrote:
> Hello Team,
> 
> In response to our previous discussions, we have refined the code to address
> 
> only the necessary adjustments focusing solely on the essential 
> modifications
> 
> for AIX compatibility. The codebase was pulled from the GitHub repository
> 
> (https://github.com/postgres/postgres.git <https://github.com/postgres/ 
> postgres.git>) and we resolved all issues
> 
> encountered during the configure and build processes. The following are the
> 
> minimal alterations needed to ensure AIX is supported.

Thanks, getting better.

Please generate the patches against 'master', rather than v17. We might 
choose to backport this to v17 later, but let's focus on the current 
development branch for now.

> Overview of Changes
> 
> We consulted with internal teams (GCC/Linker and Open Source) for each
> 
> modification. Details of the changes for the relevant files are as follows:
> 
> - configure:
> 
>    - Addressed AIX-specific alignment issues to support MAXALIGN_ALIGNOF.
> 
>    - Added comments regarding AIX alignment rules in the file.

The patch modifies 'configure', but that file is generated from 
'configure.ac'. Please show the changes to 'configure.ac'

Please also update the meson build system accordingly.

> +# AIX alignment info: We assume long's alignment is at least as strong as char, short, or
> +# int; but we must check long long (if it is being used for int64) and double.
> +#
> +# The AIX 'power' alignment rules apply the natural alignment of the "first
> +# member" if it is of a floating-point data type (or is an aggregate whose
> +# recursively "first" member or element is such a type). The alignment
> +# associated with these types for subsequent members use an alignment value
> +# where the floating-point data type is considered to have 4-byte alignment.
> +# 
> +# The double is aligned to 4-bytes on AIX in aggregates. But to maintain
> +# alignement across platforms the max alignment of long should be considered.

I don't quite understand this comment. Maybe some rephrasing or examples 
would help.

What is an aggregate in this context? A struct or union?

We use C99 int64_t for the int64 datatype nowadays, not "long long", so 
this seems a little outdated. (Since commit 962da900ac)

> - doc/src/sgml/dfunc.sgml:
> 
>    - Updated related documentation.

> diff --git a/doc/src/sgml/dfunc.sgml b/doc/src/sgml/dfunc.sgml
> index b94aefcd0c..554f9fac4c 100644
> --- a/doc/src/sgml/dfunc.sgml
> +++ b/doc/src/sgml/dfunc.sgml
> @@ -202,4 +202,23 @@ gcc -G -o foo.so foo.o
>    server expects to find the shared library files.
>   </para>
>  
> +<!--
> +Under AIX, object files are compiled normally but building the shared
> +library requires a couple of steps.  First, create the object file:
> +.nf
> +cc <other flags> -c foo.c
> +.fi
> +You must then create a symbol \*(lqexports\*(rq file for the object
> +file:
> +.nf
> +mkldexport foo.o `pwd` &gt; foo.exp
> +.fi
> +Finally, you can create the shared library:
> +.nf
> +ld <other flags> -H512 -T512 -o foo.so -e _nostart \e
> +   -bI:.../lib/postgres.exp -bE:foo.exp foo.o \e
> +   -lm -lc 2>/dev/null
> +.fi
> +  -->
> +
>  </sect2>

Why is this commented out?

You say "updated related documentation", but these instructions were 
first added to the tree in year 1999, in commit f2f43efbe1, which 
apparently copied them from old man pages. Are they still current and 
relevant?

> - Shared Libraries Build Process:
> 
>    - Files Affected:
> 
>      src/Makefile.shlib
> 
>      src/backend/Makefile
> 
> - src/backend/port/aix/mkldexport.sh
> 
>    - Implemented AIX-specific changes for shared library creation using 
> export
> 
>      files.

>    - As per discussions with the linker team, the current design 
> mandates using
> 
>      the script method to extract symbols and build shared libraries.
> 
>    - This approach aligns with practices in other open-source projects like
> 
>      Python, OpenBLAS and other gnu tools(references, link).
> 
> https://github.com/python/cpython/blob/main/Modules/ld_so_aix.in 
> <https://github.com/python/cpython/blob/main/Modules/ld_so_aix.in>
> 
> https://github.com/OpenMathLib/OpenBLAS/ 
> commit/892f8ff3e55e24fda9af3f6364319cce3f60116b#diff-4011a114c75fe804332139868806cbe23f275963e4e6afa9b57e51b2b9817293R261 <https://github.com/OpenMathLib/OpenBLAS/commit/892f8ff3e55e24fda9af3f6364319cce3f60116b#diff-4011a114c75fe804332139868806cbe23f275963e4e6afa9b57e51b2b9817293R261>

Thanks for the links. It's disappointing there isn't a standard way to 
do this. It's nice to see the comments in cpython's makeexp_aix script 
explaining the "hidden tricks". I'd love to see similar comments in 
mkldexport.sh explaining what it does. And also why the script is needed 
on AIX in the first place.

I wonder if meson's AIX support would have something built-in to do this?

>    - src/makefiles/Makefile.aix: Introduced AIX-specific makefile for 
> building.

> +# when building with gcc, need to make sure that libgcc can be found
> +ifeq ($(GCC), yes)
> +libpath := $(libpath):$(dir $(shell gcc -print-libgcc-file-name))
> +endif
> +
> +rpath = -Wl,-blibpath:'$(rpathdir)$(libpath)'

Is this still needed? I have no idea, but it seems surprising if gcc and 
ld don't handle this automatically.

> +LDFLAGS_SL += -Wl,-bnoentry -Wl,-H512 -Wl,-bM:SRE

What do all these options do? Why are they needed?

>    - src/template/aix: Defined memset flags for AIX.

Why? Did you benchmark it? How big is the difference?

I don't know if the default we have for MEMSET_LOOP_LIMIT is any good on 
modern compilers and systems in general. Perhaps we should just always 
use the system memset(). So for this patch, e key question is whether 
there's something AIX specific here. I'd assume no. I'd assume it to 
depend on the hardware rather than the OS.

-- 
Heikki Linnakangas
Neon (https://neon.tech)