Thread

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.