dmitry-chirkov-dremio commented on code in PR #40031:
URL: https://github.com/apache/arrow/pull/40031#discussion_r3581642185
##########
cpp/src/gandiva/engine.cc:
##########
@@ -132,8 +135,8 @@ arrow::Result<T> AsArrowResult(llvm::Expected<T>& expected,
Result<llvm::orc::JITTargetMachineBuilder> MakeTargetMachineBuilder(
const Configuration& conf) {
- llvm::orc::JITTargetMachineBuilder jtmb(
- (llvm::Triple(llvm::sys::getDefaultTargetTriple())));
+ static auto default_target_triple = llvm::sys::getDefaultTargetTriple();
+ llvm::orc::JITTargetMachineBuilder
jtmb((llvm::Triple(default_target_triple)));
Review Comment:
I think this helper whitelist is still incomplete. After this change, a C
helper only gets registered if it is either explicitly present in
`used_functions_` or hardcoded in `internal_functions_`, but some precompiled
IR entry points still call helper stubs transitively.
Two examples I found:
- `castVARCHAR_decimal128_int64` in `precompiled/decimal_wrapper.cc` calls
`gdv_fn_dec_to_string`
- `castTIMESTAMP_utf8` in `precompiled/time.cc` calls `gdv_fn_time_with_zone`
Those helpers are not added to `internal_functions_`, and they are not
top-level expression functions that `ExprDecomposer` would ever record in
`used_functions_`. That means this optimization can skip registering symbols
that the linked IR still needs at runtime.
Can we either keep registering all IR-only helper stubs, or teach this
tracking to include transitive C helpers used from precompiled IR?
--
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]
