Thread

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!