Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. jit: Fix jit_profiling_support when unavailable.

  2. jit: Drop redundant LLVM configure probes.

  3. jit: Support opaque pointers in LLVM 16.

  1. Minor LLVM cleanups

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-28T03:41:46Z

    Hi,

0001:  These days we handle LLVM API evolution with LLVM_VERSION_MAJOR
guards.  These GDB and Perf support probes escaped recent garbage
collection cycles by not being phrased like that.  Function probes are
generally better for cross-platform variations and library build
options that are exposed by function visibility, but in this case all
supported versions have the functions, even when the relevant feature
isn't enabled in LLVM.

0002:  On my FreeBSD box (and presumably any non-Linux system), if I
set jit_profiling_support=1 then LLVMCreatePerfJITEventListener() is a
dummy function that returns NULL and we crash.  The attached just
silently skips in that case.  If we raised an error instead I suppose
it would have to be FATAL given the call site in a callback invoked by
LLVM/C++.  We could work harder and teach the GUC to probe LLVM when
you try to turn it on, but apparently no one tried to turn on perf on
a system without perf in all these years...  Should the manual say
that it's only available on Linux?  Would it be reasonable to
additionally assume that __linux__ implies LLVM_USE_PERF and disable
the GUC otherwise?

(There are more kinds of profiling support available, which I might
learn more about as part of the JITLink work.)

0003:  While contemplating how close we are to an empty
llvmjit_wrap.cpp file, I considered whether the two wrappers added by
commit 37d5babb should be upstreamed, and then realised that this one
is not needed if you jump though one extra hoop.

0004:  I *think* the second one is redundant too: all the functions in
question are either global or we have a template function of the same
type that is.  From a spartan trail of bread crumbs[1][2] I realised
that we should be able to use LLVMGlobalGetValueType() instead.  make
check with passes with TEMP_CONFIG set to define jit_above_cost=0
against bleeding-edge LLVM built with
-DLLVM_USE_SANITIZER="Address;Undefined" and
-DLLVM_ENABLE_ASSERTIONS=ON.

[1] https://github.com/llvm/llvm-project/blob/06c8ee61ab80305be88380e6aa2f1b2fe32f859d/llvm/include/llvm-c/Core.h#L2672
[2] https://github.com/llvm/llvm-project/blob/06c8ee61ab80305be88380e6aa2f1b2fe32f859d/llvm/include/llvm/IR/Function.h#L210

  2. Re: Minor LLVM cleanups

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-12-02T18:53:55Z

    Hi, I did a quick look at the patches and here are my comments.

On Fri Nov 28, 2025 at 12:41 AM -03, Thomas Munro wrote:
> 0001:  These days we handle LLVM API evolution with LLVM_VERSION_MAJOR
> guards.  These GDB and Perf support probes escaped recent garbage
> collection cycles by not being phrased like that.  Function probes are
> generally better for cross-platform variations and library build
> options that are exposed by function visibility, but in this case all
> supported versions have the functions, even when the relevant feature
> isn't enabled in LLVM.
>
LGTM

> 0002:  On my FreeBSD box (and presumably any non-Linux system), if I
> set jit_profiling_support=1 then LLVMCreatePerfJITEventListener() is a
> dummy function that returns NULL and we crash.
>
Just confirming that I tested this on MacOS and it also crashes.

> The attached just silently skips in that case.  If we raised an error
> instead I suppose it would have to be FATAL given the call site in a
> callback invoked by LLVM/C++.  We could work harder and teach the GUC
> to probe LLVM when you try to turn it on, but apparently no one tried
> to turn on perf on a system without perf in all these years...  Should
> the manual say that it's only available on Linux?  Would it be
> reasonable to additionally assume that __linux__ implies LLVM_USE_PERF
> and disable the GUC otherwise?
>
The patch looks good, it fix the crash and IMHO the documentation change
would be enough. On guc_parameter.dat we have the following comment that
I agree and make my point about why just the documentation change would
be enough:
    # This is not guaranteed to be available, but given it's a developer
    # oriented option, it doesn't seem worth adding code checking
    # availability.

> (There are more kinds of profiling support available, which I might
> learn more about as part of the JITLink work.)
>
You are referring to this patch [1]? I've noticed that this patch didn't
get any review yet, I'm still learning about this area of the code but I
can try to give a review and test it.

> 0003:  While contemplating how close we are to an empty
> llvmjit_wrap.cpp file, I considered whether the two wrappers added by
> commit 37d5babb should be upstreamed, and then realised that this one
> is not needed if you jump though one extra hoop.
>
LGTM

> 0004:  I *think* the second one is redundant too: all the functions in
> question are either global or we have a template function of the same
> type that is.  From a spartan trail of bread crumbs[1][2] I realised
> that we should be able to use LLVMGlobalGetValueType() instead.  make
> check with passes with TEMP_CONFIG set to define jit_above_cost=0
> against bleeding-edge LLVM built with
> -DLLVM_USE_SANITIZER="Address;Undefined" and
> -DLLVM_ENABLE_ASSERTIONS=ON.
>
I think that these includes can be removed 
#include "jit/llvmjit.h"
#include "jit/llvmjit_backport.h"

I also did some tests and I didn't find any issue with this change.

[1] https://www.postgresql.org/message-id/CA%2BhUKGJBJx4fDGLv8zUtmsmg16Swry7DJbMr2_GNZcd6sgE0rg%40mail.gmail.com

--
Matheus Alcantara
EDB: http://www.enterprisedb.com

  3. Re: Minor LLVM cleanups

    Andres Freund <andres@anarazel.de> — 2025-12-03T15:52:51Z

    Hi,


On 2025-11-28 16:41:46 +1300, Thomas Munro wrote:
> 0001:  These days we handle LLVM API evolution with LLVM_VERSION_MAJOR
> guards.  These GDB and Perf support probes escaped recent garbage
> collection cycles by not being phrased like that.  Function probes are
> generally better for cross-platform variations and library build
> options that are exposed by function visibility, but in this case all
> supported versions have the functions, even when the relevant feature
> isn't enabled in LLVM.

WFM.

> 0002:  On my FreeBSD box (and presumably any non-Linux system), if I
> set jit_profiling_support=1 then LLVMCreatePerfJITEventListener() is a
> dummy function that returns NULL and we crash.  The attached just
> silently skips in that case.  If we raised an error instead I suppose
> it would have to be FATAL given the call site in a callback invoked by
> LLVM/C++.  We could work harder and teach the GUC to probe LLVM when
> you try to turn it on, but apparently no one tried to turn on perf on
> a system without perf in all these years...  Should the manual say
> that it's only available on Linux?  Would it be reasonable to
> additionally assume that __linux__ implies LLVM_USE_PERF and disable
> the GUC otherwise?

> (There are more kinds of profiling support available, which I might
> learn more about as part of the JITLink work.)

LGTM.


> 0003:  While contemplating how close we are to an empty
> llvmjit_wrap.cpp file, I considered whether the two wrappers added by
> commit 37d5babb should be upstreamed, and then realised that this one
> is not needed if you jump though one extra hoop.


> 0004:  I *think* the second one is redundant too: all the functions in
> question are either global or we have a template function of the same
> type that is.  From a spartan trail of bread crumbs[1][2] I realised
> that we should be able to use LLVMGlobalGetValueType() instead.  make
> check with passes with TEMP_CONFIG set to define jit_above_cost=0
> against bleeding-edge LLVM built with
> -DLLVM_USE_SANITIZER="Address;Undefined" and
> -DLLVM_ENABLE_ASSERTIONS=ON.

Hm, I guess this reduces the sanity checking a tiny bit, because presumably
LLVMGlobalGetValueType() will also return non-function types?

I am not sure this buys us all that much?

Greetings,

Andres Freund

  4. Re: Minor LLVM cleanups

    Thomas Munro <thomas.munro@gmail.com> — 2025-12-31T02:09:44Z

    On Wed, Dec 3, 2025 at 7:53 AM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
> On Fri Nov 28, 2025 at 12:41 AM -03, Thomas Munro wrote:
> > 0001:  These days we handle LLVM API evolution with LLVM_VERSION_MAJOR
> > guards.  These GDB and Perf support probes escaped recent garbage
> > collection cycles by not being phrased like that.  Function probes are
> > generally better for cross-platform variations and library build
> > options that are exposed by function visibility, but in this case all
> > supported versions have the functions, even when the relevant feature
> > isn't enabled in LLVM.
> >
> LGTM

Thanks, pushed.

> > 0002:  On my FreeBSD box (and presumably any non-Linux system), if I
> > set jit_profiling_support=1 then LLVMCreatePerfJITEventListener() is a
> > dummy function that returns NULL and we crash.
> >
> Just confirming that I tested this on MacOS and it also crashes.

Thanks for testing!

> > The attached just silently skips in that case.  If we raised an error
> > instead I suppose it would have to be FATAL given the call site in a
> > callback invoked by LLVM/C++.  We could work harder and teach the GUC
> > to probe LLVM when you try to turn it on, but apparently no one tried
> > to turn on perf on a system without perf in all these years...  Should
> > the manual say that it's only available on Linux?  Would it be
> > reasonable to additionally assume that __linux__ implies LLVM_USE_PERF
> > and disable the GUC otherwise?
> >
> The patch looks good, it fix the crash and IMHO the documentation change
> would be enough. On guc_parameter.dat we have the following comment that
> I agree and make my point about why just the documentation change would
> be enough:
>     # This is not guaranteed to be available, but given it's a developer
>     # oriented option, it doesn't seem worth adding code checking
>     # availability.

Right, thanks.  I think the documentation already covers it by saying
that it needs support to actually work.  It's pretty obscure...  So,
pushed.

I also noticed that on my Debian box, the files go to /tmp/.debug/jit,
not ~/.debug/jit as our documentation claims.  I couldn't immediately
see why in a quick look at the source... hmm.

> > (There are more kinds of profiling support available, which I might
> > learn more about as part of the JITLink work.)
> >
> You are referring to this patch [1]? I've noticed that this patch didn't
> get any review yet, I'm still learning about this area of the code but I
> can try to give a review and test it.

Yeah.  I have made some progress on that front and will CC you on that
thread for interest.

Will reply to Andres's email for the other patches.

  5. Re: Minor LLVM cleanups

    Thomas Munro <thomas.munro@gmail.com> — 2025-12-31T02:11:10Z

    On Thu, Dec 4, 2025 at 4:52 AM Andres Freund <andres@anarazel.de> wrote:
> On 2025-11-28 16:41:46 +1300, Thomas Munro wrote:
> > 0001:  These days we handle LLVM API evolution with LLVM_VERSION_MAJOR
> > guards.  These GDB and Perf support probes escaped recent garbage
> > collection cycles by not being phrased like that.  Function probes are
> > generally better for cross-platform variations and library build
> > options that are exposed by function visibility, but in this case all
> > supported versions have the functions, even when the relevant feature
> > isn't enabled in LLVM.
>
> WFM.

Thanks, pushed as already noted.

> > 0002:  On my FreeBSD box (and presumably any non-Linux system), if I
> > set jit_profiling_support=1 then LLVMCreatePerfJITEventListener() is a
> > dummy function that returns NULL and we crash.  The attached just
> > silently skips in that case.  If we raised an error instead I suppose
> > it would have to be FATAL given the call site in a callback invoked by
> > LLVM/C++.  We could work harder and teach the GUC to probe LLVM when
> > you try to turn it on, but apparently no one tried to turn on perf on
> > a system without perf in all these years...  Should the manual say
> > that it's only available on Linux?  Would it be reasonable to
> > additionally assume that __linux__ implies LLVM_USE_PERF and disable
> > the GUC otherwise?
>
> > (There are more kinds of profiling support available, which I might
> > learn more about as part of the JITLink work.)
>
> LGTM.

Ditto.

> > 0003:  While contemplating how close we are to an empty
> > llvmjit_wrap.cpp file, I considered whether the two wrappers added by
> > commit 37d5babb should be upstreamed, and then realised that this one
> > is not needed if you jump though one extra hoop.
>
>
> > 0004:  I *think* the second one is redundant too: all the functions in
> > question are either global or we have a template function of the same
> > type that is.  From a spartan trail of bread crumbs[1][2] I realised
> > that we should be able to use LLVMGlobalGetValueType() instead.  make
> > check with passes with TEMP_CONFIG set to define jit_above_cost=0
> > against bleeding-edge LLVM built with
> > -DLLVM_USE_SANITIZER="Address;Undefined" and
> > -DLLVM_ENABLE_ASSERTIONS=ON.
>
> Hm, I guess this reduces the sanity checking a tiny bit, because presumably
> LLVMGlobalGetValueType() will also return non-function types?
>
> I am not sure this buys us all that much?

Yeah, on reflection it's also a little more confusing to the reader.
Abandoning these ones for now.  Thanks for looking!