kou commented on code in PR #38116: URL: https://github.com/apache/arrow/pull/38116#discussion_r1354124148
########## cpp/src/gandiva/engine.cc: ########## @@ -152,6 +153,7 @@ Status Engine::LoadFunctionIRs() { if (!functions_loaded_) { ARROW_RETURN_NOT_OK(LoadPreCompiledIR()); ARROW_RETURN_NOT_OK(DecimalIR::AddFunctions(this)); + ARROW_RETURN_NOT_OK(LoadExternalPreCompiledIR()); Review Comment: > Could you please explain with more details? Sure! Sorry for missing information... > Do you mean we add a new API called `AddPreCompiledIR` (do you mean `IR` instead of `ID`?) to the `Engine` class so that developers can call this API directly to add a third party IR? Yes. (It's a typo. Sorry...) > Currently, the `Engine` class is not used by Gandiva users directly, typically Gandiva users uses the `Projector` API, which internally calls the `LLVMGenerator` which internally calls the `Engine` API. For example: > > ``` > std::shared_ptr<Projector> projector; > ARROW_EXPECT_OK(Projector::Make(schema, {multiply}, TestConfiguration(), &projector)); > ... > ARROW_EXPECT_OK(projector->Evaluate(*in_batch, pool_, &outs)); > ``` > > So if we add `AddPreCompiledIR` to the `Engine` class, this will be a new API surface for users as well. > > In this PR, it is expected Gandiva users to use the new `LLVMIRStore` API to load third party IR into Gandiva, it is possible we move this API to `Engine` class, but since the `Engine` class is not a global singleton, and is re-instantiated every time by the `LLVMGenerator`, if we store IR in `Engine`, we may have to introduce similar global states in `Engine` like we do for `LLVMIRStore`. How about adding `FunctionRegistry::Add(NativeFunction func, const std::string& bitcode_path)` and using it and `Engine::AddPreCompiledIR()` in `LLVMGenerator::Add()`? Then, users can only use `FunctionRegistry::Add()` to add an external function instead of using `FunctionRegistry::AddFunction()` and LVMIRStore`. -- 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