Re: Minor LLVM cleanups

On Wed, Dec 3, 2025 at 7:53 AM Matheus Alcantara
<matheusssilv97(at)gmail(dot)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.