On Thu, Dec 4, 2025 at 4:52 AM Andres Freund <[email protected]> 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!
