Support for LLVM 19#697
Conversation
elliottslaughter
left a comment
There was a problem hiding this comment.
Thanks for pushing this the rest of the way through. I really appreciate it.
Just a couple of questions inline on the new defaults.
| set(DEFAULT_TERRA_SLIB_INCLUDE_LLVM ${TERRA_STATIC_LINK_LLVM}) | ||
| else() | ||
| if(WIN32) | ||
| if(WIN32 OR LLVM_VERSION_MAJOR GREATER 18) |
There was a problem hiding this comment.
To be clear, would you expect DEFAULT_TERRA_SLIB_INCLUDE_LLVM to work with LLVM 19? I.e., is it just flipping the default or required?
| set(TERRA_STATIC_LINK_LLVM ON CACHE BOOL "Statically link Terra against LLVM") | ||
|
|
||
| if(LLVM_VERSION_MAJOR GREATER 18) | ||
| set(DEFAULT_TERRA_WHOLE_ARCHIVE_LLVM OFF) |
There was a problem hiding this comment.
I'm also wondering if DEFAULT_TERRA_WHOLE_ARCHIVE_LLVM could work with LLVM 19? Is it broken or is there some other reason to want it off here?
Indeed, it is required on UNIX if we want to avoid # CMakeLists.txt:43
if(LLVM_VERSION_MAJOR GREATER 18 AND TERRA_SLIB_INCLUDE_LLVM)
message(FATAL_ERROR "TERRA_SLIB_INCLUDE_LLVM is not supported on LLVM 19+")
endif()
It appears to be broken with the same reason as above. I'm going to spend a bit more time looking into the duplicate command registration and see if we can't clean that up a bit for newer LLVM versions. |
|
Yes, please add an error message like you suggested. If we later find a fix for this issue we can always merge that as a separate PR later. But otherwise this looks ready to go. You can also update the README support table to save me the trouble of doing it in a follow-up commit (add a line for LLVM 19 after this one): terra/release/share/terra/README.md Line 155 in 1d0ac6b |
|
Thanks again for figuring this out! |
|
@dawsonfrakes I updated the LLVM 20 branch and put it in #698. There are binaries so the CI can run now. I had forgotten there were a couple other unresolved issues discussed at the top of #684 (comment), one of which I discuss more at #684 (comment). Unfortunately I'm at the end of my time slice at the moment, but if you have time and desire to work on it, you're certainly welcome to do so (but no pressure). |
Follow up to #684.
It took two attempts a week apart, but I finally got it. Tested (except for FreeBSD) with CI and passing. The key for newer LLVM versions (that took three hours to figure out) on Linux seems to be using
-Wl,--start-groupand-Wl,--end-groupfor LLVM libraries since apparently there's a cyclic dependency now.I no longer see the error
llvm: JIT has not been linked in.either, which might be due to the fix I implemented for Windows #694 since it had a similar issue?This should also allow for LLVM 20+ work to begin if
terralang/llvm-buildgets updated.Thanks @elliottslaughter and @KaruroChori for your work in #684!