Hi Andres, > I think I'd probably try to apply this to master independent of the > larger patchset, to avoid a large dependency.
Awesome! +1. Attached 2nd version of patch rebased on master. (v2-0001-Resolve-PL-handler-names-for-JITed-code-instead-o.patch) > Did you check whether there's any cases this fails in the tree with your > patch applied? The way I usually do that is by running the regression > tests like > PGOPTIONS='-cjit_above_cost=0' make -s -Otarget check-world > > (which will take a bit longer if use an optimized LLVM build, and a > *lot* longer if you use a debug llvm build) Great suggestion! I used: PGOPTIONS='-c jit_above_cost=0' gmake installcheck-world It all passed except a couple of logical decoding tests that never pass on my machine for any tree (t/006_logical_decoding.pl and t/010_logical_decoding_timelines.pl) and point (which seems to be failing even on master as of: d80be6f2f) I have attached the regression.diffs which captures the point failure. > Hm. Aren't you breaking things here? If fmgr_symbol returns a basename > of NULL, as is the case for all internal functions, you're going to > print a NULL pointer, no? For internal functions, it is supposed to return modname = NULL but basename will be non-NULL right? As things stand, fmgr_symbol can never return a null basename. I have added an Assert to make that even more explicit. > Cool! I'll probably merge that into my patch (with attribution of > course). > > I wonder if it'd nicer to not have separate C variables for all of > these, and instead look them up on-demand from the module loaded in > llvm_create_types(). Not sure. Great! It is much nicer indeed. Attached version 2 with your suggested changes. (v2-0001-Rely-on-llvmjit_types-for-building-EvalFunc-calls.patch) Used the same testing method as above. > Sorry for not replying to that earlier. I'm not quite sure it's > actually worthwhile doing so - did you try to measure any memory / cpu > savings? No problem, thanks for the reply! Unfortunately, I did not do anything significant in terms of mem/cpu measurements. However, I have noticed non-trivial differences between optimized and unoptimized .bc files that were dumped from time to time. > The magnitude of wins aside, I also have a local patch that I'm going to > try to publish this or next week, that deduplicates functions more > aggressively, mostly to avoid redundant optimizations. It's quite > possible that we should run that before the function passes - or even > give up entirely on the function pass optimizations. As the module pass > manager does the same optimizations it's not that clear in which cases > it'd be beneficial to run it, especially if it means we can't > deduplicate before optimizations. Agreed, excited to see the patch! -- Soumyadeep
v2-0001-Rely-on-llvmjit_types-for-building-EvalFunc-calls.patch
Description: Binary data
v2-0001-Resolve-PL-handler-names-for-JITed-code-instead-o.patch
Description: Binary data
point_failure.diffs
Description: Binary data