lriggs commented on code in PR #51266:
URL: https://github.com/apache/arrow/pull/51266#discussion_r4009853982
##########
cpp/src/gandiva/engine.cc:
##########
@@ -439,7 +462,10 @@ Status Engine::LoadExternalPreCompiledIR() {
for (const auto& buffer : buffers) {
auto llvm_memory_buffer_ref = AsLLVMMemoryBuffer(*buffer);
auto module_or_error = llvm::parseBitcodeFile(llvm_memory_buffer_ref,
*context());
- ARROW_RETURN_NOT_OK(VerifyAndLinkModule(*module_,
std::move(module_or_error)));
+ ARROW_ASSIGN_OR_RAISE(
+ auto src_ir_module,
+ AsArrowResult(module_or_error, "Failed to verify and link module: "));
+ ARROW_RETURN_NOT_OK(VerifyAndLinkModule(*module_,
std::move(src_ir_module)));
Review Comment:
The first sentence from copilot doesn't make sense to me. But I think
leaving a comment here might be ok for now. There could be actual cases where
the precompiled bitcode needs certain cpu instructions. If those instructions
weren't available gandiva would crash at execution. Stripping those attributes
here might be better since then you could get a compile error instead which
would be more diagnosable. And if the architecture matches the precompile
instructions then it works fine with stripping.
So I think stripping them in both places would be best.
--
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]
