meson vs. llvm bitcode files

Started by Peter Eisentrautover 1 year ago14 messages
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

The meson build currently does not produce llvm bitcode (.bc) files.
AFAIK, this is the last major regression for using meson for production
builds.

Is anyone working on that? I vaguely recall that some in-progress code
was shared a couple of years ago, but I haven't seen anything since. It
would be great if we could collect any existing code and notes to maybe
get this moving again.

#2Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Peter Eisentraut (#1)
Re: meson vs. llvm bitcode files

Hi,

On Thu, 5 Sept 2024 at 11:56, Peter Eisentraut <peter@eisentraut.org> wrote:

The meson build currently does not produce llvm bitcode (.bc) files.
AFAIK, this is the last major regression for using meson for production
builds.

Is anyone working on that? I vaguely recall that some in-progress code
was shared a couple of years ago, but I haven't seen anything since. It
would be great if we could collect any existing code and notes to maybe
get this moving again.

I found that Andres shared a patch
(v17-0021-meson-Add-LLVM-bitcode-emission.patch) a while ago [1]/messages/by-id/20220927011951.j3h4o7n6bhf7dwau@awork3.anarazel.de.

[1]: /messages/by-id/20220927011951.j3h4o7n6bhf7dwau@awork3.anarazel.de

--
Regards,
Nazir Bilal Yavuz
Microsoft

#3Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#2)
Re: meson vs. llvm bitcode files

Hi,

On Thu, 5 Sept 2024 at 12:24, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

I found that Andres shared a patch
(v17-0021-meson-Add-LLVM-bitcode-emission.patch) a while ago [1].

Andres and I continued to work on that. I think the patches are in
sharable state now and I wanted to hear opinions before proceeding
further. After applying the patches, bitcode files should be installed
into $pkglibdir/bitcode/ directory if the llvm is found.

There are 6 patches attached:

v1-0001-meson-Add-generated-header-stamps:

This patch is trivial. Instead of having targets depending directly on
the generated headers, have them depend on a stamp file. The benefit
of using a stamp file is that it makes ninja.build smaller and meson
setup faster.
----------

v1-0002-meson-Add-postgresql-extension.pc-for-building-extension-libraries:

This patch is for generating postgresql-extension.pc file which can be
used for building extensions libraries.

Normally, there is no need to use this .pc file for generating bitcode
files. However, since there is no clear way to get all include paths
for building bitcode files, this .pc file is later used for this
purpose (by running pkg-config --cflags-only-I
postgresql-extension-uninstalled.pc) [1].
----------

v1-0003-meson-Test-building-extensions-by-using-postgresql-extension.pc:
[Not needed for generating bitcode files]

This is a patch for testing if extensions can be built by using
postgresql-extension.pc. I added that commit as an example of using
postgresql-extension.pc to build extensions.
----------

v1-0004-meson-WIP-Add-docs-for-postgresql-extension.pc: [Not needed
for generating bitcode files]

I added this patch in case we recommend people to use
postgresql-extension.pc to build extension libraries. I am not sure if
we want to do that because there are still TODOs about
postgresql-extension.pc like running test suites. I just wanted to
show my plan, dividing 'Extension Building Infrastructure' into two,
'PGXS' and 'postgresql-extension.pc'.
----------

v1-0005-meson-Add-LLVM-bitcode-emission:

This patch adds required infrastructure to generate bitcode files and
uses postgresql-extension-uninstalled.pc to get include paths for
generating bitcode files [1].
----------

v1-0006-meson-Generate-bitcode-files-of-contrib-extension.patch:

This patch adds manually selected contrib libraries to generate their
bitcode files. These libraries are selected manually, depending on
- If they have SQL callable functions
- If the library functions are short enough (the performance gain from
bitcode files is too minimal compared to the function's run time, so
this type of libraries are omitted).

Any kind of feedback would be appreciated.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v1-0001-meson-Add-generated-header-stamps.patchtext/x-patch; charset=US-ASCII; name=v1-0001-meson-Add-generated-header-stamps.patchDownload+29-10
v1-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v1-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v1-0003-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v1-0003-meson-Test-building-extensions-by-using-postgresq.patchDownload+206-1
v1-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v1-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v1-0005-meson-Add-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v1-0005-meson-Add-LLVM-bitcode-emission.patchDownload+120-12
v1-0006-meson-Generate-bitcode-files-of-contrib-extension.patchtext/x-patch; charset=US-ASCII; name=v1-0006-meson-Generate-bitcode-files-of-contrib-extension.patchDownload+250-1
#4Diego Fronza
diego.fronza@percona.com
In reply to: Nazir Bilal Yavuz (#3)
Re: meson vs. llvm bitcode files

Hello,

I did a full review on the provided patches plus some tests, I was able to
validate that the loading of bitcode modules is working also JIT works for
both backend and contrib modules.

To test JIT on contrib modules I just lowered the costs for all jit
settings and used the intarray extension, using the data/test__int.data:
CREATE EXTENSION intarray;
CREATE TABLE test__int( a int[] );1
\copy test__int from 'data/test__int.data'

For queries any from line 98+ on contrib/intarray/sql/_int.sql will work.

Then I added extra debug messages to llvmjit_inline.cpp
on add_module_to_inline_search_path() function, also
on llvm_build_inline_plan(), I was able to see many functions in this
module being successfully inlined.

I'm attaching a new patch based on your original work which add further
support for generating bitcode from:
- Generated backend sources: processed by flex, bison, etc.
- Generated contrib module sources,

On this patch I just included fmgrtab.c and src/backend/parser for the
backend generated code.
For contrib generated sources I added contrib/cube as an example.

All relevant details about the changes are included in the patch itself.

As you may know already I also created a PR focused on llvm bitcode
emission on meson, it generates bitcode for all backend and contribution
modules, currently under review by some colleagues at Percona:
https://github.com/percona/postgres/pull/103
I'm curious if we should get all or some of the generated backend sources
compiled to bitcode, similar to contrib modules.
Please let me know your thoughts and how we can proceed to get this feature
included, thank you.

Regards,
Diego Fronza
Percona

On Fri, Mar 7, 2025 at 7:52 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Show quoted text

Hi,

On Thu, 5 Sept 2024 at 12:24, Nazir Bilal Yavuz <byavuz81@gmail.com>
wrote:

I found that Andres shared a patch
(v17-0021-meson-Add-LLVM-bitcode-emission.patch) a while ago [1].

Andres and I continued to work on that. I think the patches are in
sharable state now and I wanted to hear opinions before proceeding
further. After applying the patches, bitcode files should be installed
into $pkglibdir/bitcode/ directory if the llvm is found.

There are 6 patches attached:

v1-0001-meson-Add-generated-header-stamps:

This patch is trivial. Instead of having targets depending directly on
the generated headers, have them depend on a stamp file. The benefit
of using a stamp file is that it makes ninja.build smaller and meson
setup faster.
----------

v1-0002-meson-Add-postgresql-extension.pc-for-building-extension-libraries:

This patch is for generating postgresql-extension.pc file which can be
used for building extensions libraries.

Normally, there is no need to use this .pc file for generating bitcode
files. However, since there is no clear way to get all include paths
for building bitcode files, this .pc file is later used for this
purpose (by running pkg-config --cflags-only-I
postgresql-extension-uninstalled.pc) [1].
----------

v1-0003-meson-Test-building-extensions-by-using-postgresql-extension.pc:
[Not needed for generating bitcode files]

This is a patch for testing if extensions can be built by using
postgresql-extension.pc. I added that commit as an example of using
postgresql-extension.pc to build extensions.
----------

v1-0004-meson-WIP-Add-docs-for-postgresql-extension.pc: [Not needed
for generating bitcode files]

I added this patch in case we recommend people to use
postgresql-extension.pc to build extension libraries. I am not sure if
we want to do that because there are still TODOs about
postgresql-extension.pc like running test suites. I just wanted to
show my plan, dividing 'Extension Building Infrastructure' into two,
'PGXS' and 'postgresql-extension.pc'.
----------

v1-0005-meson-Add-LLVM-bitcode-emission:

This patch adds required infrastructure to generate bitcode files and
uses postgresql-extension-uninstalled.pc to get include paths for
generating bitcode files [1].
----------

v1-0006-meson-Generate-bitcode-files-of-contrib-extension.patch:

This patch adds manually selected contrib libraries to generate their
bitcode files. These libraries are selected manually, depending on
- If they have SQL callable functions
- If the library functions are short enough (the performance gain from
bitcode files is too minimal compared to the function's run time, so
this type of libraries are omitted).

Any kind of feedback would be appreciated.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v2-0001-meson-Add-LLVM-bitcode-emission-for-generated-source.patchtext/x-patch; charset=US-ASCII; name=v2-0001-meson-Add-LLVM-bitcode-emission-for-generated-source.patchDownload+44-7
#5Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Diego Fronza (#4)
Re: meson vs. llvm bitcode files

Hi,

On Tue, 11 Mar 2025 at 01:04, Diego Fronza <diego.fronza@percona.com> wrote:

I did a full review on the provided patches plus some tests, I was able to validate that the loading of bitcode modules is working also JIT works for both backend and contrib modules.

Thank you!

To test JIT on contrib modules I just lowered the costs for all jit settings and used the intarray extension, using the data/test__int.data:
CREATE EXTENSION intarray;
CREATE TABLE test__int( a int[] );1
\copy test__int from 'data/test__int.data'

For queries any from line 98+ on contrib/intarray/sql/_int.sql will work.

Then I added extra debug messages to llvmjit_inline.cpp on add_module_to_inline_search_path() function, also on llvm_build_inline_plan(), I was able to see many functions in this module being successfully inlined.

I'm attaching a new patch based on your original work which add further support for generating bitcode from:

Thanks for doing that!

- Generated backend sources: processed by flex, bison, etc.
- Generated contrib module sources,

I think we do not need to separate these two.

   foreach srcfile : bitcode_module['srcfiles']
-    if meson.version().version_compare('>=0.59')
+    srcfilename = '@0@'.format(srcfile)
+    if srcfilename.startswith('<CustomTarget')
+      srcfilename = srcfile.full_path().split(meson.build_root() + '/')[1]
+    elif meson.version().version_compare('>=0.59')

Also, checking if the string starts with '<CustomTarget' is a bit
hacky, and 'srcfilename = '@0@'.format(srcfile)' causes a deprecation
warning. So, instead of this we can process all generated sources like
how generated backend sources are processed. I updated the patch with
that.

On this patch I just included fmgrtab.c and src/backend/parser for the backend generated code.
For contrib generated sources I added contrib/cube as an example.

I applied your contrib/cube example and did the same thing for the contrib/seg.

All relevant details about the changes are included in the patch itself.

As you may know already I also created a PR focused on llvm bitcode emission on meson, it generates bitcode for all backend and contribution modules, currently under review by some colleagues at Percona: https://github.com/percona/postgres/pull/103
I'm curious if we should get all or some of the generated backend sources compiled to bitcode, similar to contrib modules.

I think we can do this. I added other backend sources like you did in
the PR but attached it as another patch (0007) because I wanted to
hear other people's opinions on that first.

v3 is attached.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v3-0001-meson-Add-generated-header-stamps.patchtext/x-patch; charset=US-ASCII; name=v3-0001-meson-Add-generated-header-stamps.patchDownload+29-10
v3-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v3-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v3-0003-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v3-0003-meson-Test-building-extensions-by-using-postgresq.patchDownload+206-1
v3-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v3-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v3-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v3-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v3-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v3-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
v3-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v3-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
#6Diego Fronza
diego.fronza@percona.com
In reply to: Nazir Bilal Yavuz (#5)
Re: meson vs. llvm bitcode files

Hi,

The v7 patch looks good to me, handling the bitcode modules in a uniform
way and also avoiding the hacky code and warnings, much better now.

A small note about the bitcode emission for generated sources in contrib,
using cube as example, currently it creates two dict entries in a list:
bc_seg_gen_sources = [{'srcfiles': [seg_scan]}]
bc_seg_gen_sources += {'srcfiles': [seg_parse[0]]}

Then pass it to the bitcode_modules:
bitcode_modules += {
...
'gen_srcfiles': bc_seg_gen_sources,
}

It could be passed as a list with a single dict, since both generated
sources share the same compilation flags:
bitcode_modules += {
...
'gen_srcfiles': [
{ 'srcfiles': [cube_scan, cube_parse[0]] }.
]
}

Both approaches work, the first one has the advantage of being able to pass
separate additional_flags per generated source.

Thanks for your reply Nazir, also waiting for more opinions on this.

Regards,
Diego

On Wed, Mar 12, 2025 at 7:27 AM Nazir Bilal Yavuz <byavuz81@gmail.com>
wrote:

Show quoted text

Hi,

On Tue, 11 Mar 2025 at 01:04, Diego Fronza <diego.fronza@percona.com>
wrote:

I did a full review on the provided patches plus some tests, I was able

to validate that the loading of bitcode modules is working also JIT works
for both backend and contrib modules.

Thank you!

To test JIT on contrib modules I just lowered the costs for all jit

settings and used the intarray extension, using the data/test__int.data:

CREATE EXTENSION intarray;
CREATE TABLE test__int( a int[] );1
\copy test__int from 'data/test__int.data'

For queries any from line 98+ on contrib/intarray/sql/_int.sql will work.

Then I added extra debug messages to llvmjit_inline.cpp on

add_module_to_inline_search_path() function, also on
llvm_build_inline_plan(), I was able to see many functions in this module
being successfully inlined.

I'm attaching a new patch based on your original work which add further

support for generating bitcode from:

Thanks for doing that!

- Generated backend sources: processed by flex, bison, etc.
- Generated contrib module sources,

I think we do not need to separate these two.

foreach srcfile : bitcode_module['srcfiles']
-    if meson.version().version_compare('>=0.59')
+    srcfilename = '@0@'.format(srcfile)
+    if srcfilename.startswith('<CustomTarget')
+      srcfilename = srcfile.full_path().split(meson.build_root() + '/')[1]
+    elif meson.version().version_compare('>=0.59')

Also, checking if the string starts with '<CustomTarget' is a bit
hacky, and 'srcfilename = '@0@'.format(srcfile)' causes a deprecation
warning. So, instead of this we can process all generated sources like
how generated backend sources are processed. I updated the patch with
that.

On this patch I just included fmgrtab.c and src/backend/parser for the

backend generated code.

For contrib generated sources I added contrib/cube as an example.

I applied your contrib/cube example and did the same thing for the
contrib/seg.

All relevant details about the changes are included in the patch itself.

As you may know already I also created a PR focused on llvm bitcode

emission on meson, it generates bitcode for all backend and contribution
modules, currently under review by some colleagues at Percona:
https://github.com/percona/postgres/pull/103

I'm curious if we should get all or some of the generated backend

sources compiled to bitcode, similar to contrib modules.

I think we can do this. I added other backend sources like you did in
the PR but attached it as another patch (0007) because I wanted to
hear other people's opinions on that first.

v3 is attached.

--
Regards,
Nazir Bilal Yavuz
Microsoft

#7Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Diego Fronza (#6)
Re: meson vs. llvm bitcode files

Hi,

On Wed, 12 Mar 2025 at 16:39, Diego Fronza <diego.fronza@percona.com> wrote:

Hi,

The v7 patch looks good to me, handling the bitcode modules in a uniform way and also avoiding the hacky code and warnings, much better now.

A small note about the bitcode emission for generated sources in contrib, using cube as example, currently it creates two dict entries in a list:
bc_seg_gen_sources = [{'srcfiles': [seg_scan]}]
bc_seg_gen_sources += {'srcfiles': [seg_parse[0]]}

Then pass it to the bitcode_modules:
bitcode_modules += {
...
'gen_srcfiles': bc_seg_gen_sources,
}

It could be passed as a list with a single dict, since both generated sources share the same compilation flags:
bitcode_modules += {
...
'gen_srcfiles': [
{ 'srcfiles': [cube_scan, cube_parse[0]] }.
]
}

Both approaches work, the first one has the advantage of being able to pass separate additional_flags per generated source.

I liked the current approach as it makes bitcode_modules easier to
understand but both approaches work for me as well.

One thing I noticed is that gen_srcfiles['srcfiles'] seems wrong.
gen_sources is a better name compared to gen_srcfiles. So, I changed
it to gen_sources in v4.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v4-0001-meson-Add-generated-header-stamps.patchtext/x-patch; charset=US-ASCII; name=v4-0001-meson-Add-generated-header-stamps.patchDownload+29-10
v4-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v4-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v4-0003-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v4-0003-meson-Test-building-extensions-by-using-postgresq.patchDownload+206-1
v4-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v4-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v4-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v4-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v4-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v4-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
v4-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v4-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
#8Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#7)
Re: meson vs. llvm bitcode files

Hi,

On Thu, 13 Mar 2025 at 13:11, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

One thing I noticed is that gen_srcfiles['srcfiles'] seems wrong.
gen_sources is a better name compared to gen_srcfiles. So, I changed
it to gen_sources in v4.

Rebase is needed due to b1720fe63f, v5 is attached.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v5-0001-meson-Add-generated-header-stamps.patchtext/x-patch; charset=US-ASCII; name=v5-0001-meson-Add-generated-header-stamps.patchDownload+29-10
v5-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v5-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v5-0003-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v5-0003-meson-Test-building-extensions-by-using-postgresq.patchDownload+205-1
v5-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v5-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v5-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v5-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v5-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v5-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
v5-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v5-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
#9Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#8)
Re: meson vs. llvm bitcode files

Hi,

On Tue, 29 Apr 2025 at 11:23, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

On Thu, 13 Mar 2025 at 13:11, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

One thing I noticed is that gen_srcfiles['srcfiles'] seems wrong.
gen_sources is a better name compared to gen_srcfiles. So, I changed
it to gen_sources in v4.

Rebase is needed due to b1720fe63f, v5 is attached.

Mandatory rebase, v6 is attached.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v6-0001-meson-Add-generated-header-stamps.patchtext/x-patch; charset=US-ASCII; name=v6-0001-meson-Add-generated-header-stamps.patchDownload+29-10
v6-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v6-0002-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v6-0003-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v6-0003-meson-Test-building-extensions-by-using-postgresq.patchDownload+205-1
v6-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v6-0004-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v6-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v6-0005-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v6-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v6-0006-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
v6-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v6-0007-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
#10Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#9)
Re: meson vs. llvm bitcode files

Hi,

On Mon, 7 Jul 2025 at 11:45, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Mandatory rebase, v6 is attached.

Rebase is needed due to 01d6832c10, v7 is attached.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v7-0001-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v7-0001-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v7-0002-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v7-0002-meson-Test-building-extensions-by-using-postgresq.patchDownload+205-1
v7-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v7-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v7-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v7-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v7-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v7-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
v7-0006-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v7-0006-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
#11Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#10)
Re: meson vs. llvm bitcode files

Hi,

On Wed, 13 Aug 2025 at 16:25, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

On Mon, 7 Jul 2025 at 11:45, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Mandatory rebase, v6 is attached.

Rebase is needed due to 01d6832c10, v7 is attached.

Rebase is needed due to 16607718c0, v8 is attached.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v8-0001-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v8-0001-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v8-0002-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v8-0002-meson-Test-building-extensions-by-using-postgresq.patchDownload+205-1
v8-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v8-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v8-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v8-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v8-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v8-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
v8-0006-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v8-0006-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
#12Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#11)
Re: meson vs. llvm bitcode files

Hi,

On Fri, 31 Oct 2025 at 15:13, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

On Wed, 13 Aug 2025 at 16:25, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

On Mon, 7 Jul 2025 at 11:45, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Mandatory rebase, v6 is attached.

Rebase is needed due to 01d6832c10, v7 is attached.

Rebase is needed due to 16607718c0, v8 is attached.

Rebase is needed. Also, there is small functional change in 0002:

def remove_duplicates(duplicate_str):
-    words = duplicate_str.split()
+    # Remove duplicates based on basename as there could be a mix of both full
+    # paths and bare binary names.
+    words = [os.path.basename(word) for word in duplicate_str.split()]
    return ' '.join(sorted(set(words), key=words.index))

It is because MacOS was failing due to there being 2 instances of
ccache, one is with full path '/opt/local/bin/ccache' and one is just
the binary name 'ccache'. remove_duplicates() function did not remove
them as it compared full strings before, now it compares only
basenames.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v9-0006-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchtext/x-patch; charset=US-ASCII; name=v9-0006-meson-Add-LLVM-bitcode-emission-for-backend-sourc.patchDownload+17-1
v9-0001-meson-Add-postgresql-extension.pc-for-building-ex.patchtext/x-patch; charset=US-ASCII; name=v9-0001-meson-Add-postgresql-extension.pc-for-building-ex.patchDownload+110-1
v9-0002-meson-Test-building-extensions-by-using-postgresq.patchtext/x-patch; charset=US-ASCII; name=v9-0002-meson-Test-building-extensions-by-using-postgresq.patchDownload+209-1
v9-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v9-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patchDownload+66-40
v9-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v9-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v9-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchtext/x-patch; charset=US-ASCII; name=v9-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patchDownload+256-1
#13Peter Eisentraut
peter_e@gmx.net
In reply to: Nazir Bilal Yavuz (#12)
Re: meson vs. llvm bitcode files

On 16.01.26 12:33, Nazir Bilal Yavuz wrote:

Hi,

On Fri, 31 Oct 2025 at 15:13, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

On Wed, 13 Aug 2025 at 16:25, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Hi,

On Mon, 7 Jul 2025 at 11:45, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

Mandatory rebase, v6 is attached.

Rebase is needed due to 01d6832c10, v7 is attached.

Rebase is needed due to 16607718c0, v8 is attached.

Rebase is needed. Also, there is small functional change in 0002:

def remove_duplicates(duplicate_str):
-    words = duplicate_str.split()
+    # Remove duplicates based on basename as there could be a mix of both full
+    # paths and bare binary names.
+    words = [os.path.basename(word) for word in duplicate_str.split()]
return ' '.join(sorted(set(words), key=words.index))

It is because MacOS was failing due to there being 2 instances of
ccache, one is with full path '/opt/local/bin/ccache' and one is just
the binary name 'ccache'. remove_duplicates() function did not remove
them as it compared full strings before, now it compares only
basenames.

Some review comments from me.

v9-0001-meson-Add-postgresql-extension.pc-for-building-ex.patch

Need to think about whether "extension" is the correct term.

New meson message:

NOTICE: Future-deprecated features used:
* 0.62.0: {'pkgconfig.generate variable for builtin directories'}

The comment that introduces postgresql-extension-warnings.pc says

+# Extension modules should likely also use -fwrapv etc. But it it's a 
bit odd
+# to expose it to a .pc file?

but then -fwrapv ends up in postgresql-extension.pc anyway. Not sure
what was intended here.

Also, the description "PostgreSQL Extension Support - Compiler
Warnings" could be clarified, like "with recommended compiler
warnings" or "with compiler warnings same as core code" or similar.

The Requires list in my case is for example

Requires: krb5-gssapi, icu-uc, icu-i18n, ldap, libxml-2.0 >= 2.6.23,
liblz4, openssl, zlib, libzstd >= 1.4.0

but I don't think these are actually required for building extensions
(unless a particular extension directly makes use of one of them, in
which case they should declare that on their own).

If we are going to install these .pc files, we also need to build them
with with makefiles. Alternatively, we could not install them for now
and just use them internally.

v9-0002-meson-Test-building-extensions-by-using-postgresq.patch

Not sure if this was meant to be kept or it's just for local testing.

New meson warnings:

WARNING: Deprecated features used:
* 0.55.0: {'ExternalProgram.path'}
* 0.56.0: {'meson.build_root'}

src/test/modules/ seems like the wrong location, since it's not a
module or a test module.

I don't know if it's possible to make meson use a different file than
meson.build, but if so, it might be better to keep these test
meson.build files together with their extensions, like
contrib/amcheck/meson-test.build. Similar to how we have "PGXS" build
support in the makefiles. Otherwise, I'm afraid this will get
annoying and error-prone if one has to remember to update other files
under src/test/ when adding for example a new .sql file to amcheck.

Also, the driver script is at 'src/tools/ci/test_meson_extensions',
but you are using it outside of CI, so that's not a good location.

v9-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patch

Let's not rename existing ids.

It seems to me that the .pc file can also be used without meson.
Let's take that into account a bit. For example, the
id="extend-postgres-meson" could be id="extend-postgres-pkg-config" or
similar.

Your text ends with a colon. Did you mean to add more text? Maybe an
example meson.build would be good.

v9-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patch

+# some libraries include "hstore/hstore.h" instead of "hstore.h"

It seems to me that the former is correct, but if not then we should
fix it.

#14Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Peter Eisentraut (#13)
Re: meson vs. llvm bitcode files

Hi,

On Thu, 12 Mar 2026 at 13:54, Peter Eisentraut <peter@eisentraut.org> wrote:

Some review comments from me.

Thank you for looking into this!

v9-0001-meson-Add-postgresql-extension.pc-for-building-ex.patch

Need to think about whether "extension" is the correct term.

It looks correct to me. Do you have any suggestions?

New meson message:

NOTICE: Future-deprecated features used:
* 0.62.0: {'pkgconfig.generate variable for builtin directories'}

Fixed.

The comment that introduces postgresql-extension-warnings.pc says

+# Extension modules should likely also use -fwrapv etc. But it it's a
bit odd
+# to expose it to a .pc file?

but then -fwrapv ends up in postgresql-extension.pc anyway. Not sure
what was intended here.

I asked Andres off-list and Andres said that we need to have these
flags inside the .pc file but it is not very nice since these flags
(-fwrapv for example) change the behavior. Maybe Andres could clarify
this better.

Also, the description "PostgreSQL Extension Support - Compiler
Warnings" could be clarified, like "with recommended compiler
warnings" or "with compiler warnings same as core code" or similar.

Done. I changed it to "PostgreSQL Extension Support with compiler
warnings the same as core code". I am not sure about
uppercase/lowercase but it seems okay to me.

The Requires list in my case is for example

Requires: krb5-gssapi, icu-uc, icu-i18n, ldap, libxml-2.0 >= 2.6.23,
liblz4, openssl, zlib, libzstd >= 1.4.0

but I don't think these are actually required for building extensions
(unless a particular extension directly makes use of one of them, in
which case they should declare that on their own).

It seems that is how meson pkgconfig.generate() handles the
dependencies, please see [1]https://mesonbuild.com/Pkgconfig-module.html#implicit-dependencies:

...
* Dependencies provided by pkg-config are added into Requires: or
Requires.private:. If a version was specified when declaring that
dependency it will be written into the generated file too.
...

If we are going to install these .pc files, we also need to build them
with with makefiles. Alternatively, we could not install them for now
and just use them internally.

Unfortunately, these .pc files are always installed in meson build. I
added a WIP patch (0007) for building .pc files with makefiles, I am
not sure if I am following the correct way. I would appreciate any
help on this.

v9-0002-meson-Test-building-extensions-by-using-postgresq.patch

Not sure if this was meant to be kept or it's just for local testing.

I think we can have it in Postgres, it shows that generated .pc files
work and extensions can be built by using these .pc files and using
meson build. But maybe we can build one extension instead of three (or
a dummy extension), what do you think?

New meson warnings:

WARNING: Deprecated features used:
* 0.55.0: {'ExternalProgram.path'}
* 0.56.0: {'meson.build_root'}

Fixed.

src/test/modules/ seems like the wrong location, since it's not a
module or a test module.

I don't know if it's possible to make meson use a different file than
meson.build, but if so, it might be better to keep these test
meson.build files together with their extensions, like
contrib/amcheck/meson-test.build. Similar to how we have "PGXS" build
support in the makefiles. Otherwise, I'm afraid this will get
annoying and error-prone if one has to remember to update other files
under src/test/ when adding for example a new .sql file to amcheck.

I don't think we can use something other than meson.build. I solved
that by editing the test_meson_extension script, now meson-test.build
files live under the actual contrib/${extension}/ directory and the
test script moves them to the correct directory. I needed to use the
get_option('meson_source_dir') hack to get paths of the source files.

Also, the driver script is at 'src/tools/ci/test_meson_extensions',
but you are using it outside of CI, so that's not a good location.

You are right, I moved the test_meson_extensions script under the 'src/tools/'.

v9-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patch

Let's not rename existing ids.

It seems to me that the .pc file can also be used without meson.
Let's take that into account a bit. For example, the
id="extend-postgres-meson" could be id="extend-postgres-pkg-config" or
similar.

Sorry but I didn't understand how we can add a pkg-config
documentation without renaming existing ids. 'Extension Building
Infrastructure' is covered by <sect1 id="extend-pgxs">. I guess we
would want to add pkg-config documentation under the extension
building infrastructure, but it is something other than PGXS. So, it
being under '<sect1 id="extend-pgxs">' doesn't sound correct to me.

Your text ends with a colon. Did you mean to add more text? Maybe an
example meson.build would be good.

Yes, sorry for that. I added an example meson.build file.

v9-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-libr.patch

+# some libraries include "hstore/hstore.h" instead of "hstore.h"

It seems to me that the former is correct, but if not then we should
fix it.

I think both of them are correct and the comment is wrong. Source
files in the contrib/hstore directory include "hstore.h" and files
outside of this directory include "hstore/hstore.h". I changed this
comment to '# Files outside of the current directory include hstore as
"hstore/hstore.h"'.

[1]: https://mesonbuild.com/Pkgconfig-module.html#implicit-dependencies

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v10-0001-meson-Add-postgresql-extension.pc-for-building-e.patchtext/x-patch; charset=US-ASCII; name=v10-0001-meson-Add-postgresql-extension.pc-for-building-e.patchDownload+113-2
v10-0002-meson-Test-building-extensions-by-using-postgres.patchtext/x-patch; charset=US-ASCII; name=v10-0002-meson-Test-building-extensions-by-using-postgres.patchDownload+227-1
v10-0003-meson-Add-docs-for-postgresql-extension.pc.patchtext/x-patch; charset=US-ASCII; name=v10-0003-meson-Add-docs-for-postgresql-extension.pc.patchDownload+95-40
v10-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchtext/x-patch; charset=US-ASCII; name=v10-0004-meson-Add-architecture-for-LLVM-bitcode-emission.patchDownload+141-12
v10-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-lib.patchtext/x-patch; charset=US-ASCII; name=v10-0005-meson-Add-LLVM-bitcode-emissions-for-contrib-lib.patchDownload+256-1
v10-0006-meson-Add-LLVM-bitcode-emission-for-backend-sour.patchtext/x-patch; charset=US-ASCII; name=v10-0006-meson-Add-LLVM-bitcode-emission-for-backend-sour.patchDownload+17-1
v10-0007-WIP-Generate-postgresql-extension.pc-in-autoconf.patchtext/x-patch; charset=US-ASCII; name=v10-0007-WIP-Generate-postgresql-extension.pc-in-autoconf.patchDownload+58-2