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
