niyue commented on code in PR #38116:
URL: https://github.com/apache/arrow/pull/38116#discussion_r1364807572
##########
cpp/src/gandiva/engine.h:
##########
@@ -93,6 +93,9 @@ class GANDIVA_EXPORT Engine {
/// the main module.
Status LoadPreCompiledIR();
+ // load external pre-compiled IR modules from LLVMIRStore
+ Status LoadExternalPreCompiledIR();
Review Comment:
See the comment here
https://github.com/apache/arrow/pull/38116#discussion_r1364803533
Currently, users of Gandiva don't use the `Engine` class directly, instead,
they typically use `Projector` or `Filter` classes when working with
expressions, and `Projector` internally uses `LLVMGenerator`, which internally
uses `Engine` class. Even if we make the API here to take an `LLVMIRStore`
instance, `LLVMGenerator` will still have to populate this argument with a
global variable. As far as I can tell, putting the logic in `Engine` instead of
`LLVMGenerator` is more consistent with current logic, currently the
`LoadPreCompiledIR ` is loading IR from a static variable, which is not too
different with `LoadExternalPreCompiledIR` which loads IR from some global
variable.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
