Remove a FIXME and unused variables in Meson

Started by Tristan Partinalmost 2 years ago4 messages
#1Tristan Partin
tristan@neon.tech

Two meson patches.

One of them adds version gates to two LLVM flags (-frwapv,
-fno-strict-aliasing). I believe we moved the minimum LLVM version
recently, so these might not be necessary, but maybe it helps for
historictal reasons. If not, I'll just remove the comment in a different
patch.

Second patch removes some unused variables. Were they analogous to
things in autotools and the Meson portions haven't been added yet?

I was looking into adding LLVM JIT support to Meson since there is
a TODO about it, but it wasn't clear what was missing except adding some
variables into the PGXS Makefile.

--
Tristan Partin
Neon (https://neon.tech)

#2Michael Paquier
michael@paquier.xyz
In reply to: Tristan Partin (#1)
Re: Remove a FIXME and unused variables in Meson

On Thu, Mar 14, 2024 at 12:13:18AM -0500, Tristan Partin wrote:

One of them adds version gates to two LLVM flags (-frwapv,
-fno-strict-aliasing). I believe we moved the minimum LLVM version recently,
so these might not be necessary, but maybe it helps for historictal reasons.
If not, I'll just remove the comment in a different patch.

Second patch removes some unused variables. Were they analogous to things in
autotools and the Meson portions haven't been added yet?

I was looking into adding LLVM JIT support to Meson since there is a TODO
about it, but it wasn't clear what was missing except adding some variables
into the PGXS Makefile.

It looks like you have forgotten to attach the patches. :)
--
Michael

#3Tristan Partin
tristan@neon.tech
In reply to: Michael Paquier (#2)
2 attachment(s)
Re: Remove a FIXME and unused variables in Meson

On Thu Mar 14, 2024 at 12:15 AM CDT, Michael Paquier wrote:

On Thu, Mar 14, 2024 at 12:13:18AM -0500, Tristan Partin wrote:

One of them adds version gates to two LLVM flags (-frwapv,
-fno-strict-aliasing). I believe we moved the minimum LLVM version recently,
so these might not be necessary, but maybe it helps for historictal reasons.
If not, I'll just remove the comment in a different patch.

Second patch removes some unused variables. Were they analogous to things in
autotools and the Meson portions haven't been added yet?

I was looking into adding LLVM JIT support to Meson since there is a TODO
about it, but it wasn't clear what was missing except adding some variables
into the PGXS Makefile.

It looks like you have forgotten to attach the patches. :)

CLASSIC!

--
Tristan Partin
Neon (https://neon.tech)

Attachments:

v1-0001-Protect-adding-llvm-flags-if-found-version-is-not.patchtext/x-patch; charset=utf-8; name=v1-0001-Protect-adding-llvm-flags-if-found-version-is-not.patchDownload
From 615acd91d353defd7fbe136fd115515452d6d00b Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Thu, 14 Mar 2024 00:02:11 -0500
Subject: [PATCH v1 1/2] Protect adding llvm flags if found version is not
 enough

-fwrapv: https://github.com/llvm/llvm-project/commit/51924e517bd2d25faea6ef873db3c59ec4d09bf8
-fno-strict-aliasing: https://github.com/llvm/llvm-project/commit/10169b94cfe6838f881339f1944891f6d8451174
---
 src/backend/jit/llvm/meson.build | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/backend/jit/llvm/meson.build b/src/backend/jit/llvm/meson.build
index 41c759f73c5..0e1de7bc98e 100644
--- a/src/backend/jit/llvm/meson.build
+++ b/src/backend/jit/llvm/meson.build
@@ -58,8 +58,13 @@ else
 endif
 
 
-# XXX: Need to determine proper version of the function cflags for clang
-bitcode_cflags = ['-fno-strict-aliasing', '-fwrapv']
+bitcode_cflags = []
+if llvm.version().version_compare('>=2.9.0')
+  bitcode_cflags += ['-fno-strict-aliasing']
+endif
+if llvm.version().version_compare('>=2.8.0')
+  bitcode_cflags += ['-fwrapv']
+endif
 if llvm.version().version_compare('=15.0')
   bitcode_cflags += ['-Xclang', '-no-opaque-pointers']
 endif
-- 
Tristan Partin
Neon (https://neon.tech)

v1-0002-Remove-some-unused-variables-in-Meson.patchtext/x-patch; charset=utf-8; name=v1-0002-Remove-some-unused-variables-in-Meson.patchDownload
From 4e5541352cd582c7c7320d02c075cf7a95c6bfda Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Thu, 14 Mar 2024 00:05:38 -0500
Subject: [PATCH v1 2/2] Remove some unused variables in Meson

They weren't being used, so don't assign to them.
---
 meson.build             | 5 -----
 src/backend/meson.build | 1 -
 2 files changed, 6 deletions(-)

diff --git a/meson.build b/meson.build
index 85788f9dd8f..b0a45a7d834 100644
--- a/meson.build
+++ b/meson.build
@@ -202,7 +202,6 @@ if host_system == 'cygwin'
   dlsuffix = '.dll'
   mod_link_args_fmt = ['@0@']
   mod_link_with_name = 'lib@0@.exe.a'
-  mod_link_with_dir = 'libdir'
 
 elif host_system == 'darwin'
   dlsuffix = '.dylib'
@@ -212,7 +211,6 @@ elif host_system == 'darwin'
   export_fmt = '-Wl,-exported_symbols_list,@0@'
 
   mod_link_args_fmt = ['-bundle_loader', '@0@']
-  mod_link_with_dir = 'bindir'
   mod_link_with_name = '@0@'
 
   sysroot_args = [files('src/tools/darwin_sysroot'), get_option('darwin_sysroot')]
@@ -269,7 +267,6 @@ elif host_system == 'windows'
     mod_link_with_name = 'lib@0@.exe.a'
   endif
   mod_link_args_fmt = ['@0@']
-  mod_link_with_dir = 'libdir'
 
   shmem_kind = 'win32'
   sema_kind = 'win32'
@@ -488,10 +485,8 @@ dir_pgxs = dir_lib_pkg / 'pgxs'
 dir_include = get_option('includedir')
 
 dir_include_pkg = dir_include
-dir_include_pkg_rel = ''
 if not (dir_prefix_contains_pg or dir_include_pkg.contains('pgsql') or dir_include_pkg.contains('postgres'))
   dir_include_pkg = dir_include_pkg / pkg
-  dir_include_pkg_rel = pkg
 endif
 
 dir_man = get_option('mandir')
diff --git a/src/backend/meson.build b/src/backend/meson.build
index 436c04af080..d2c10d1abd7 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -154,7 +154,6 @@ if mod_link_args_fmt.length() > 0
 
   name = mod_link_with_name.format('postgres')
   link_with_uninst = meson.current_build_dir() / name
-  link_with_inst = '${@0@}/@1@'.format(mod_link_with_dir, name)
 
   foreach el : mod_link_args_fmt
     pg_mod_link_args += el.format(link_with_uninst)
-- 
Tristan Partin
Neon (https://neon.tech)

#4Peter Eisentraut
peter@eisentraut.org
In reply to: Tristan Partin (#1)
Re: Remove a FIXME and unused variables in Meson

On 14.03.24 06:13, Tristan Partin wrote:

One of them adds version gates to two LLVM flags (-frwapv,
-fno-strict-aliasing). I believe we moved the minimum LLVM version
recently, so these might not be necessary, but maybe it helps for
historictal reasons. If not, I'll just remove the comment in a different
patch.

We usually remove version gates once the overall minimum required
version is new enough. So this doesn't seem like a step in the right
direction.

Second patch removes some unused variables. Were they analogous to
things in autotools and the Meson portions haven't been added yet?

Hmm, yeah, no idea. These were not used even in the first commit for
Meson support. Might have had a purpose in earlier draft patches.