Add llvm version into the version string

Started by Dmitry Dolgovover 1 year ago10 messages
#1Dmitry Dolgov
9erthalion6@gmail.com
1 attachment(s)

In many jit related bug reports, one of the first questions is often
"which llvm version is used". How about adding it into the
PG_VERSION_STR, similar to the gcc version?

Attachments:

v1-0001-Add-llvm-version-to-the-PG_VERSION_STR.patchtext/plain; charset=us-asciiDownload
From 9a4404c0df92f96ab98fca3ff0d7952287c30ece Mon Sep 17 00:00:00 2001
From: Dmitrii Dolgov <9erthalion6@gmail.com>
Date: Tue, 20 Aug 2024 22:54:43 +0200
Subject: [PATCH v1] Add llvm version to the PG_VERSION_STR

If built with llvm, add llvm version into the version string. This
should help to get more relevant information in bug reports about JIT.
---
 configure.ac | 8 +++++++-
 meson.build  | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4e279c4bd6..3b483ad401 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2421,8 +2421,14 @@ else
   cc_string=$CC
 fi
 
+if test "$with_llvm" = yes ; then
+    llvm_string=", llvm $pgac_llvm_version"
+else
+    llvm_string=""
+fi
+
 AC_DEFINE_UNQUOTED(PG_VERSION_STR,
-                   ["PostgreSQL $PG_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"],
+                   ["PostgreSQL $PG_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit$llvm_string"],
                    [A string containing the version number, platform, and C compiler])
 
 # Supply a numeric version string for use by 3rd party add-ons
diff --git a/meson.build b/meson.build
index cd711c6d01..e4e4b0bc32 100644
--- a/meson.build
+++ b/meson.build
@@ -2759,12 +2759,18 @@ cdata.set('USE_@0@_SEMAPHORES'.format(sema_kind.to_upper()), 1)
 cdata.set('MEMSET_LOOP_LIMIT', memset_loop_limit)
 cdata.set_quoted('DLSUFFIX', dlsuffix)
 
+if llvm.found()
+  llvm_string = ', llvm @0@'.format(llvm.version())
+else
+  llvm_string = ''
+endif
 
 # built later than the rest of the version metadata, we need SIZEOF_VOID_P
 cdata.set_quoted('PG_VERSION_STR',
-  'PostgreSQL @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format(
+  'PostgreSQL @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit@6@'.format(
     pg_version, host_machine.cpu_family(), host_system,
     cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8,
+    llvm_string,
   )
 )
 

base-commit: 7ff9afbbd1df7c256024edb447eae7269c1bab03
-- 
2.45.1

#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: Dmitry Dolgov (#1)
Re: Add llvm version into the version string

st 21. 8. 2024 v 12:20 odesílatel Dmitry Dolgov <9erthalion6@gmail.com>
napsal:

In many jit related bug reports, one of the first questions is often
"which llvm version is used". How about adding it into the
PG_VERSION_STR, similar to the gcc version?

+1

Pave

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dmitry Dolgov (#1)
Re: Add llvm version into the version string

Dmitry Dolgov <9erthalion6@gmail.com> writes:

In many jit related bug reports, one of the first questions is often
"which llvm version is used". How about adding it into the
PG_VERSION_STR, similar to the gcc version?

I'm pretty down on this idea as presented, first because our version
strings are quite long already, and second because llvm is an external
library. So I don't have any faith that what llvm-config said at
configure time is necessarily the exact version we're using at run
time. (Probably, the major version must be the same, but that doesn't
prove anything about minor versions.)

Is there a way to get the llvm library's version at run time? If so
we could consider building a new function to return that.

regards, tom lane

#4Dmitry Dolgov
9erthalion6@gmail.com
In reply to: Tom Lane (#3)
Re: Add llvm version into the version string

On Sat, Sep 21, 2024 at 05:25:30PM GMT, Tom Lane wrote:

Is there a way to get the llvm library's version at run time? If so
we could consider building a new function to return that.

Yes, there is a C api LLVMGetVersion to get the major, minor and patch
numbers. The jit provider could be extended to return this information.

About a new function, I think that the llvm runtime version has to be
shown in some form by pgsql_version. The idea is to skip an emails
exchange like "here is the bug report" -> "can you attach the llvm
version?". If it's going to be a new separate function, I guess it won't
make much difference to ask either to call the new function or the
llvm-config, right?

#5Dmitry Dolgov
9erthalion6@gmail.com
In reply to: Dmitry Dolgov (#4)
1 attachment(s)
Re: Add llvm version into the version string

On Sun, Sep 22, 2024 at 01:15:54PM GMT, Dmitry Dolgov wrote:

On Sat, Sep 21, 2024 at 05:25:30PM GMT, Tom Lane wrote:

Is there a way to get the llvm library's version at run time? If so
we could consider building a new function to return that.

Yes, there is a C api LLVMGetVersion to get the major, minor and patch
numbers. The jit provider could be extended to return this information.

About a new function, I think that the llvm runtime version has to be
shown in some form by pgsql_version. The idea is to skip an emails
exchange like "here is the bug report" -> "can you attach the llvm
version?". If it's going to be a new separate function, I guess it won't
make much difference to ask either to call the new function or the
llvm-config, right?

Here is what I had in mind. It turns out the LLVMGetVersion API is
available only starting from LLVM 16, so there is need to indicate if
the version string is available (otherwise llvmjit would have to fully
format the version string).

Attachments:

v2-0001-Add-jit-provider-s-version-into-the-pgsql_version.patchtext/plain; charset=us-asciiDownload
From dca9347c39cfca45de969ee49536962836ccbba4 Mon Sep 17 00:00:00 2001
From: Dmitrii Dolgov <9erthalion6@gmail.com>
Date: Mon, 23 Sep 2024 19:08:52 +0200
Subject: [PATCH v2] Add jit provider's version into the pgsql_version

If jit provider is available and initialized, add its version string
into the pgsql_version output. This should help to get more relevant
information in bug reports about JIT.
---
 src/backend/jit/jit.c           | 13 +++++++++++++
 src/backend/jit/llvm/llvmjit.c  | 17 +++++++++++++++++
 src/backend/utils/adt/version.c | 15 ++++++++++++++-
 src/include/jit/jit.h           |  8 ++++++++
 src/include/jit/llvmjit.h       |  2 ++
 5 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c
index 815b58f..b62422c 100644
--- a/src/backend/jit/jit.c
+++ b/src/backend/jit/jit.c
@@ -188,3 +188,16 @@ InstrJitAgg(JitInstrumentation *dst, JitInstrumentation *add)
 	INSTR_TIME_ADD(dst->optimization_counter, add->optimization_counter);
 	INSTR_TIME_ADD(dst->emission_counter, add->emission_counter);
 }
+
+/*
+ * Return JIT provider's version string for troubleshooting purposes.
+ */
+const char *
+jit_get_version(bool *available)
+{
+	if (provider_init())
+		return provider.get_version(available);
+
+	*available = false;
+	return "";
+}
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 0f6cec5..3ab1080 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -165,6 +165,7 @@ _PG_jit_provider_init(JitProviderCallbacks *cb)
 	cb->reset_after_error = llvm_reset_after_error;
 	cb->release_context = llvm_release_context;
 	cb->compile_expr = llvm_compile_expr;
+	cb->get_version = llvm_version;
 }
 
 
@@ -1382,3 +1383,19 @@ ResOwnerReleaseJitContext(Datum res)
 	context->resowner = NULL;
 	jit_release_context(&context->base);
 }
+
+const char *
+llvm_version(bool *available)
+{
+#if LLVM_VERSION_MAJOR > 15
+	unsigned int major, minor, patch;
+
+	LLVMGetVersion(&major, &minor, &patch);
+
+	*available = true;
+	return (const char*) psprintf("llvm: %d.%d.%d", major, minor, patch);
+#else
+	*available = false;
+	return "";
+#endif
+}
diff --git a/src/backend/utils/adt/version.c b/src/backend/utils/adt/version.c
index 28db1f6..fabaf73 100644
--- a/src/backend/utils/adt/version.c
+++ b/src/backend/utils/adt/version.c
@@ -15,10 +15,23 @@
 #include "postgres.h"
 
 #include "utils/builtins.h"
+#include "jit/jit.h"
 
 
 Datum
 pgsql_version(PG_FUNCTION_ARGS)
 {
-	PG_RETURN_TEXT_P(cstring_to_text(PG_VERSION_STR));
+	bool jit_available = false;
+	const char *jit_version = jit_get_version(&jit_available);
+
+	/* Add jit provides's version string if available. */
+	if (jit_available)
+	{
+		PG_RETURN_TEXT_P(cstring_to_text(psprintf("%s, %s", PG_VERSION_STR,
+												  jit_version)));
+	}
+	else
+	{
+		PG_RETURN_TEXT_P(cstring_to_text(PG_VERSION_STR));
+	}
 }
diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h
index d9a080c..35d5a62 100644
--- a/src/include/jit/jit.h
+++ b/src/include/jit/jit.h
@@ -70,12 +70,14 @@ typedef void (*JitProviderResetAfterErrorCB) (void);
 typedef void (*JitProviderReleaseContextCB) (JitContext *context);
 struct ExprState;
 typedef bool (*JitProviderCompileExprCB) (struct ExprState *state);
+typedef const char* (*JitProviderVersion) (bool *available);
 
 struct JitProviderCallbacks
 {
 	JitProviderResetAfterErrorCB reset_after_error;
 	JitProviderReleaseContextCB release_context;
 	JitProviderCompileExprCB compile_expr;
+	JitProviderVersion get_version;
 };
 
 
@@ -102,5 +104,11 @@ extern void jit_release_context(JitContext *context);
 extern bool jit_compile_expr(struct ExprState *state);
 extern void InstrJitAgg(JitInstrumentation *dst, JitInstrumentation *add);
 
+/*
+ * Get the provider's version string. The flag indicating availability is
+ * passed as an argument, and will be set accordingly if it's not possible to
+ * get the version.
+ */
+extern const char *jit_get_version(bool *available);
 
 #endif							/* JIT_H */
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 420775b..898848a 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -136,6 +136,8 @@ extern LLVMValueRef slot_compile_deform(struct LLVMJitContext *context, TupleDes
 extern LLVMTypeRef LLVMGetFunctionReturnType(LLVMValueRef r);
 extern LLVMTypeRef LLVMGetFunctionType(LLVMValueRef r);
 
+extern const char* llvm_version(bool *available);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

base-commit: 6aa44060a3c94ee10273bb8a89e98a5bb2fbbacb
-- 
2.45.1

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dmitry Dolgov (#5)
Re: Add llvm version into the version string

Dmitry Dolgov <9erthalion6@gmail.com> writes:

On Sun, Sep 22, 2024 at 01:15:54PM GMT, Dmitry Dolgov wrote:

About a new function, I think that the llvm runtime version has to be
shown in some form by pgsql_version. The idea is to skip an emails
exchange like "here is the bug report" -> "can you attach the llvm
version?".

I'm not in favor of that, for a couple of reasons:

* People already have expectations about what version() returns.
Some distro and forks even modify it (see eg --extra-version).
I think we risk breaking obscure use-cases if we add stuff onto that.
Having version() return something different from the PG_VERSION_STR
constant could cause other problems too, perhaps.

* Believing that it'll save us questioning a bug submitter seems
fairly naive to me. Nobody includes the result of version() unless
specifically asked for it.

* I'm not in favor of overloading version() with subsystem-related
version numbers, because it doesn't scale. Who's to say we're not
going to need the version of ICU, or libxml2, to take a couple of
obvious examples? When you take that larger view, multiple
subsystem-specific version functions seem to me to make more sense.

Maybe another idea could be a new system view?

=> select * from pg_system_version;
property | value
----------------------------------------
core version | 18.1
architecture | x86_64-pc-linux-gnu
word size | 64 bit
compiler | gcc (GCC) 12.5.0
ICU version | 60.3
LLVM version | 18.1.0
...

Adding rows to a view over time seems way less likely to cause
problems than messing with a string people probably have crufty
parsing code for.

If it's going to be a new separate function, I guess it won't
make much difference to ask either to call the new function or the
llvm-config, right?

I do think that if we can get a version number out of the loaded
library, that is worth doing. I don't trust the llvm-config that
happens to be in somebody's PATH to be the same version that their
PG is actually built with.

regards, tom lane

#7Dmitry Dolgov
9erthalion6@gmail.com
In reply to: Tom Lane (#6)
Re: Add llvm version into the version string

On Mon, Sep 23, 2024 at 02:45:18PM GMT, Tom Lane wrote:
Maybe another idea could be a new system view?

=> select * from pg_system_version;
property | value
----------------------------------------
core version | 18.1
architecture | x86_64-pc-linux-gnu
word size | 64 bit
compiler | gcc (GCC) 12.5.0
ICU version | 60.3
LLVM version | 18.1.0
...

Adding rows to a view over time seems way less likely to cause
problems than messing with a string people probably have crufty
parsing code for.

Agree, the idea with a new system view sounds interesting. I'll try to
experiment in this direction, thanks.

#8Alastair Turner
minion@decodable.me
In reply to: Tom Lane (#6)
Re: Add llvm version into the version string

On Mon, 23 Sept 2024 at 19:45, Tom Lane <tgl@sss.pgh.pa.us> wrote:
...

Maybe another idea could be a new system view?

=> select * from pg_system_version;
property | value
----------------------------------------
core version | 18.1
architecture | x86_64-pc-linux-gnu
word size | 64 bit
compiler | gcc (GCC) 12.5.0
ICU version | 60.3
LLVM version | 18.1.0
...

A view like that sounds very useful.

Adding rows to a view over time seems way less likely to cause
problems than messing with a string people probably have crufty
parsing code for.

If it's going to be a new separate function, I guess it won't
make much difference to ask either to call the new function or the
llvm-config, right?

I do think that if we can get a version number out of the loaded
library, that is worth doing. I don't trust the llvm-config that
happens to be in somebody's PATH to be the same version that their
PG is actually built with.

Since the build and runtime versions may differ for some things like llvm,
libxml2 and the interpreters behind some of the PLs, it may be valuable to
expand the view and show two values - a build time (or configure time)
value and a runtime value for these.

Regards

Alastair

#9Andres Freund
andres@anarazel.de
In reply to: Alastair Turner (#8)
Re: Add llvm version into the version string

Hi,

On 2024-09-24 13:53:49 +0100, Alastair Turner wrote:

Since the build and runtime versions may differ for some things like llvm,
libxml2 and the interpreters behind some of the PLs, it may be valuable to
expand the view and show two values - a build time (or configure time)
value and a runtime value for these.

+1

Somewhat orthogonal: I've wondered before whether it'd be useful to have a
view showing the file path and perhaps the soversion of libraries dynamically
loaded into postgres. That's currently hard to figure out over a connection
(which is all a lot of our users have access to).

Greetings,

Andres Freund

#10Joe Conway
mail@joeconway.com
In reply to: Andres Freund (#9)
Re: Add llvm version into the version string

On 9/24/24 09:52, Andres Freund wrote:

Hi,

On 2024-09-24 13:53:49 +0100, Alastair Turner wrote:

Since the build and runtime versions may differ for some things like llvm,
libxml2 and the interpreters behind some of the PLs, it may be valuable to
expand the view and show two values - a build time (or configure time)
value and a runtime value for these.

+1

Somewhat orthogonal: I've wondered before whether it'd be useful to have a
view showing the file path and perhaps the soversion of libraries dynamically
loaded into postgres. That's currently hard to figure out over a connection
(which is all a lot of our users have access to).

+1

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com