js8544 commented on PR #38116: URL: https://github.com/apache/arrow/pull/38116#issuecomment-1760993652
> It seems simpler for users to call just one API because I don't see any use scenario that users would only call one of the two APIs (FunctionRegistry::Add, LLVMExternalIRStore::Add) For now, yes, but the LLVM JIT engine also supports calling function pointers directly without the need of IRs. And Gandiva alreadys uses this mechanism (Gandiva calls it function stubs). For example, Gandiva functions like `random` and `regexp_replace` are not precompiled to IR. If a user wants to add a function to Gandiva via function pointers, not precompiled IR, they will need to: 1. Add the signature to function registry with flag `kNeedsFunctionHolder`. 2. Inherit `FunctionHolder` and implement their function in `operator()`. For example [RandomGeneratorHolder](https://github.com/apache/arrow/blob/main/cpp/src/gandiva/random_generator_holder.h#L33). 3. Wrap it in a Gandiva standard form. For example [`gdv_fn_random`](https://github.com/apache/arrow/blob/main/cpp/src/gandiva/gdv_function_stubs.cc#L61) 4. Register this exported function to `ExportedFuncsRegistry`. 5. Register their holder to [`FunctionHolderRegistry`](https://github.com/apache/arrow/blob/main/cpp/src/gandiva/function_holder_registry.h#L46). So, in this way user will need to only call `FunctionRegistry::Add` and a bunch of other registrations but not `LLVMExternalIRStore::Add`, which is why I suggest we separate the APIs. And as you mentioned there is also the dynamic library approach which also doesn't require IRs, but I think it won't be as convenient as using function stubs. cc @kou @niyue -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
