niyue commented on code in PR #39098: URL: https://github.com/apache/arrow/pull/39098#discussion_r1428837720
########## cpp/src/gandiva/engine.cc: ########## @@ -86,6 +88,13 @@ #include <llvm/Transforms/Utils.h> #include <llvm/Transforms/Vectorize.h> +// JITLink is available in LLVM 9+ +// but the `InProcessMemoryManager::Create` API was added since LLVM 14 +#if LLVM_VERSION_MAJOR >= 14 && !defined(_WIN32) && !defined(_WIN64) Review Comment: I initially didn't include this, but without including `!defined(_WIN64)` makes several CI checks on Windows failed, sorry I didn't kept the specific CI job links for that but I remembered the CI reported something like "incorrect object file format". `JITLink` does support Windows/COFF format [1][2], but it is fairly new feature, probably landed in LLVM 15 (released in 2022-09), and since I tried to make the `JITLink` related change minimum, and I don't have Windows env to cover different LLVM versions' impact, so I simply skip Windows in this case to make the version check simple. [1] NATIVE WINDOWS JITING IN LLVM, https://llvm.org/devmtg/2022-11/slides/Tutorial2-JITLink.pdf [2] 2022 LLVM Dev Mtg: JITLink: Native Windows JITing in LLVM, https://www.youtube.com/watch?v=UwHgCqQ2DDA -- 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]
