On Wed, Dec 3, 2025 at 7:53 AM Matheus Alcantara <[email protected]> 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.
