| From: | Aditya Kamath <Aditya(dot)Kamath1(at)ibm(dot)com> |
|---|---|
| To: | Srirama Kucherlapati <sriram(dot)rk(at)in(dot)ibm(dot)com>, "peter(at)eisentraut(dot)org" <peter(at)eisentraut(dot)org>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de> |
| Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "hlinnaka(at)iki(dot)fi" <hlinnaka(at)iki(dot)fi>, "tristan(at)partin(dot)io" <tristan(at)partin(dot)io>, "postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com" <postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com> |
| Subject: | RE: AIX support |
| Date: | 2026-01-28 15:20:25 |
| Message-ID: | LV8PR15MB64888765A43D229EA5D1CFE6D691A@LV8PR15MB6488.namprd15.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Andres,
I can explain this differently as well.
We know that include_directories property in meson will include the source and the build directory in the command. Ex: If we give “src/include” then “-Isrc/include” and “-I../src/include” are added.
>> + # This flag is required to make sure the user spefic float.h is
>> + # picked instead of the system float.h header file, which doesnot
>> + # have definition like float8, etc
>> + cflags += '-D_H_FLOAT’
>I don't understand this one - how does defining _H_FLOAT lead to a >different
>header being picked?
Below is the error message we get in AIX if we do not use '-D_H_FLOAT’
FAILED: src/backend/utils/activity/wait_event_names.a.p/wait_event_funcs.c.o
gcc -Isrc/backend/utils/activity/wait_event_names.a.p -Isrc/include/utils -I../src/include/utils -Isrc/include -I../src/include -I/opt/freeware/include -I/opt/freeware/include/libxml2 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O2 -g -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wmissing-prototypes -Wpointer-arith -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -Wdeclaration-after-statement -Wno-format-truncation -Wno-stringop-truncation -maix64 -fPIC -pthread -DBUILDING_DLL1233 -MD -MQ
./src/include/utils/float.h:28:19: error: expected ';' before 'int'
28 | extern PGDLLIMPORT int extra_float_digits;
There is a clear missing of float.h definitions, the reason being float.h got included before "src/include/c.h”.
Kindly observe the command "-I../src/include/utils” flag came in the front.
Now in c.h when stdio.h is called in AIX at line 65, then AIX system limits.h is called and then within that AIX system float.h is called.
But here is the catch. It won’t pick the AIX system float.h. It will pick the Postgres "src/include/utilsfloat.h”.
The reason being the compiler is designed to pick what comes first in -I flag. One proof of that is here. https://gcc.gnu.org/onlinedocs/cpp/Wrapper-Headers.html
I am assuming all systems behave like this.
This is the root cause of the problem. That "-I../src/include/utils” which meson adds.
So now "src/include/utils/float.h”.is picked up first even before "src/include/c.h”.
That is why we saw the error. When we define “_H_FLOAT” we essentially force the system header to be ignored in the system limits.h where it is called and then the Postgres "src/include/utils/float.h” takes over.
Hope this explains Andres.
There are multiple things we can do here.
One being define “_H_FLOAT”
The other being use #include_next float.h under ifdef _AIX guard
The third we are experimenting is to have implicit_include_directories: false as per document here https://mesonbuild.com/Include-directories.html, but that might difficult since recursive include directories exists in Postgres.
Kindly let us know if this explanation helps and what you think. Also If there is a preferred or cleaner way you know let us know.
Have a nice day ahead.
Thanks and regards,
Aditya.
From: Aditya Kamath <Aditya(dot)Kamath1(at)ibm(dot)com>
Date: Wednesday, 28 January 2026 at 3:55 PM
To: Srirama Kucherlapati <sriram(dot)rk(at)in(dot)ibm(dot)com>, peter(at)eisentraut(dot)org <peter(at)eisentraut(dot)org>, andres(at)anarazel(dot)de <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org <pgsql-hackers(at)lists(dot)postgresql(dot)org>, hlinnaka(at)iki(dot)fi <hlinnaka(at)iki(dot)fi>, tristan(at)partin(dot)io <tristan(at)partin(dot)io>, postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com <postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com>
Subject: Re: [EXTERNAL] Re: AIX support
Hi Andrew,
Thank you for your review comments for AIX so far.
While we are working on the rest of the comments you had sent here is our explanation for using the '-D_H_FLOAT’ flag in the source directory meson.build file.
>> + # This flag is required to make sure the user spefic float.h is
>> + # picked instead of the system float.h header file, which doesnot
>> + # have definition like float8, etc
>> + cflags += '-D_H_FLOAT’
>I don't understand this one - how does defining _H_FLOAT lead to a >different
>header being picked? Also, float8 is defined in c.h, so it hardly could >be
>influenced by a system float.h header?
>Our float.h header is only included as "utils/float.h", so it really >shouldn’t
>be confused with a system header?
So If we do not use the flag the error we get is as follows,
FAILED: src/backend/utils/activity/wait_event_names.a.p/wait_event_funcs.c.o
gcc -Isrc/backend/utils/activity/wait_event_names.a.p -Isrc/include/utils -I../src/include/utils -Isrc/include -I../src/include -I/opt/freeware/include -I/opt/freeware/include/libxml2 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O2 -g -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wmissing-prototypes -Wpointer-arith -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -Wdeclaration-after-statement -Wno-format-truncation -Wno-stringop-truncation -maix64 -fPIC -pthread -DBUILDING_DLL1233 -MD -MQ src/backend/utils/activity/wait_event_names.a.p/wait_event_funcs.c.o -MF src/backend/utils/activity/wait_event_names.a.p/wait_event_funcs.c.o.d -o src/backend/utils/activity/wait_event_names.a.p/wait_event_funcs.c.o -c ../src/backend/utils/activity/wait_event_funcs.c
In file included from /usr/include/sys/limits.h:307,
from /opt/freeware/lib/gcc/powerpc-ibm-aix7.3.0.0/13/include-fixed/stdio.h:589,
from ../src/include/c.h:65,
from ../src/include/postgres.h:48,
from ../src/backend/utils/activity/wait_event_funcs.c:15:
../src/include/utils/float.h:28:19: error: expected ';' before 'int'
28 | extern PGDLLIMPORT int extra_float_digits;
../src/include/utils/float.h:289:37: error: unknown type name 'float4'
289 | float4_min(const float4 val1, const float4 val2)
../src/include/utils/float.h:294:15: error: unknown type name 'float8'
294 | static inline float8
The reason this happened is because -I../src/include/utils argument in the command included the float.h header first before the ./src/include/c.h header of postgresql which actually has the
PGDLLIMPORT, float4 and float8 definitions.
So, the header files have mismatched in the order they are processed. If we eliminate -I../src/include/utils in the command, then it will work.
But that will not happen automatically since we use include_directories() everywhere which adds both the source and the build directory includes. If we can get meson to not use
-I../src/include/ in AIX while including headers we will be able to compile.
We are currently experimenting with implicit_include_directories from the document below to see if we can remove the same
https://mesonbuild.com/Include-directories.html.
Reason for -D_H_FLOAT
This prevents AIX libc’s float.h to be included via c.h -> stdio.h -> limits.h -> float.h. The real problem is header include order in AIX. Compiler sees src/include/utils/float.h first and then c.h. This is a problem. When this flag is set, AIX’s float.h is skipped causing system header include chain to change. AIX no longer injects float.h early which results in c.h included before utils/float.h. the way Postgres expects.
Let us know what you think.
Is there a preferred or cleaner way you know to ensure the correct include ordering like using implicit_include_directories so that utils/float.h is not included before c.h?
Have a nice day ahead.
Thanks and regards,
Aditya.
From: Srirama Kucherlapati <sriram(dot)rk(at)in(dot)ibm(dot)com>
Date: Wednesday, 28 January 2026 at 9:34 AM
To: Aditya Kamath <Aditya(dot)Kamath1(at)ibm(dot)com>
Subject: FW: [EXTERNAL] Re: AIX support
FYI
Warm regards,
Sriram.
------------------------------------------
VIOS/SSP Development,
ISDL, IBM India Pvt Ltd.
From: Andres Freund <andres(at)anarazel(dot)de>
Date: Monday, 26 January 2026 at 21:11
To: Srirama Kucherlapati <sriram(dot)rk(at)in(dot)ibm(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Tristan Partin <tristan(at)partin(dot)io>, AIX PG user <postgres-ibm-aix(at)wwpdl(dot)vnet(dot)ibm(dot)com>
Subject: [EXTERNAL] Re: AIX support
Hi,
From what I can tell the meson patch *AGAIN* is missing mkldexport.sh. Also,
you seem to reference the script as files('port/aix/mkldexport.sh'), but that
that's not a path that makes sense for our source code structure (nor where
the "complete" patch adds it).
You really need to actually start testing your patches.
Doesn't the meson patch also require the changes to src/tools/gen_export.pl?
On 2026-01-23 16:11:25 +0000, Srirama Kucherlapati wrote:
> diff --git a/meson.build b/meson.build
> index 6e7ddd74683..17ad9c6ca32 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -198,6 +198,8 @@ endif
> # that purpose.
> portname = host_system
>
> +dep_static_lib = declare_dependency()
Add a comment saying something like
# In some configurations we don't want to install static libraries. For those
# dep_static_lib can be set to disabler() below.
The introduction of dep_static_lib should be broken out into its own patch.
> + # This flag is required to make sure the user spefic float.h is
> + # picked instead of the system float.h header file, which doesnot
> + # have definition like float8, etc
> + cflags += '-D_H_FLOAT'
I don't understand this one - how does defining _H_FLOAT lead to a different
header being picked? Also, float8 is defined in c.h, so it hardly could be
influenced by a system float.h header?
Our float.h header is only included as "utils/float.h", so it really shouldn't
be confused with a system header?
> @@ -1765,10 +1793,49 @@ endforeach
> # as long, char, short, or int. Note that we intentionally do not consider
> # any types wider than 64 bits, as allowing MAXIMUM_ALIGNOF to exceed 8
> # would be too much of a penalty for disk and memory space.
> -alignof_double = cdata.get('ALIGNOF_DOUBLE')
> -if cc.alignment('int64_t', args: test_c_args, prefix: '#include <stdint.h>') > alignof_double
> - error('alignment of int64_t is greater than the alignment of double')
> -endif
> +if host_system != 'aix'
> + alignof_double = cdata.get('ALIGNOF_DOUBLE')
> + if cc.alignment('int64_t', args: test_c_args, prefix: '#include <stdint.h>') > alignof_double
> + error('alignment of int64_t is greater than the alignment of double')
> + endif
> +else
> + # 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.
> + # More info
> + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99557
> + #
> + # 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.
How are these "AIX 'power' alignment rules" for float not just completely
broken?
I assume this means that 4 byte aligned floats work just fine, but have
degraded peformance?
Is there documentation about this that isn't an already fixed bug report in gcc?
Maybe I'm confused, but doesn't this power alignment rule mean that the
cc.alignment('double') will always return 8? That computation won't apply the
"subsequent member" rule, and therefore will have an alignment of 8. Which in
turn seems to make this entire change pointless?
> + # Get the alignment values
> + ac_cv_alignof_long = cc.alignment('long', args: test_c_args, prefix: '#include <stdint.h>')
> + ac_cv_alignof_double = cc.alignment('double', args: test_c_args, prefix: '#include <stdint.h>')
> + ac_cv_alignof_int64_t = cc.alignment('int64_t', args: test_c_args, prefix: '#include <stdint.h>')
I've previously complained about these av_cv_ variable names. This isn't
autoconf. What is this doing here?
Why do we need a platform specific alignment determination for long, int64?
> + message('Alignment of long : @0@'.format(ac_cv_alignof_long))
> + message('Alignment of double : @0@'.format(ac_cv_alignof_double))
> + message('Alignment of int64_t : @0@'.format(ac_cv_alignof_int64_t))
These are already going to be output by cc.alignment, this is just redundant,
no?
> + # Start with long
> + alignof_double = ac_cv_alignof_long
> + message('MAX ALIGN ac_cv_alignof_long')
> +
> + # Compare with double
> + if alignof_double < ac_cv_alignof_double
> + alignof_double = ac_cv_alignof_double
> + message('MAX ALIGN ac_cv_alignof_double')
> + endif
> +
> + # Compare with int64_t
> + if alignof_double < ac_cv_alignof_int64_t
> + alignof_double = ac_cv_alignof_int64_t
> + message('MAX ALIGN ac_cv_alignof_int64_t')
> + endif
> +endif
> +message('MAX ALIGN OF DOUBLE : @0@'.format(alignof_double))
This is a lot of output for something that's just computing a maximum of three
variables.
> diff --git a/src/backend/meson.build b/src/backend/meson.build
> index b831a541652..4838f245ab3 100644
> --- a/src/backend/meson.build
> +++ b/src/backend/meson.build
> @@ -125,6 +125,24 @@ if host_system == 'windows'
> '--FILEDESC', 'PostgreSQL Server',])
> endif
>
> +if host_system == 'aix'
> + # The '.' argument leads mkldexport.sh to emit "#! .", which refers to the
> + # main executable, allowing extension libraries to resolve their undefined
> + # symbols to symbols in the postgres binary.
> + postgres_imp = custom_target('postgres.imp',
> + command: [files('port/aix/mkldexport.sh'), '@INPUT@', '.'],
> + input: postgres_lib,
> + output: 'postgres.imp',
> + capture: true,
> + install: true,
> + install_dir: dir_lib,
> + build_by_default: false,
> + )
> + backend_link_args += '-Wl,-bE:@0@'.format(postgres_imp.full_path())
> + backend_link_depends += postgres_imp
> +endif
This should be moved next to the msvc specific block (the one about
postgres_def) and should use an elif.
Greetings,
Andres Freund
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-01-28 15:25:58 | Re: AIX support |
| Previous Message | Amit Kapila | 2026-01-28 15:00:45 | Re: pgsql: Prevent invalidation of newly synced replication slots. |