Skip to content

libLLVM.a doesn't exist, so fix how enzyme is linking LLVM #2302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

libLLVM.a doesn't exist, so fix how enzyme is linking LLVM #2302

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3eff2e4
libLLVM.a doesn't exist, so how enzyme is linking LLVM
ZuseZ4 May 4, 2025
fb07f16
only update non-libLLVM.so case
ZuseZ4 May 4, 2025
3039698
cleanup cxx call
ZuseZ4 May 4, 2025
a2a6728
fix test
ZuseZ4 May 4, 2025
2327cb7
fix test
ZuseZ4 May 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions enzyme/Enzyme/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ endif()

get_target_property(TBL_LINKED_LIBS LLVMSupport INTERFACE_LINK_LIBRARIES)
if (NOT TBL_LINKED_LIBS)
message(STATUS "No TBL_LINKED_LIBS found")
else()
list(REMOVE_ITEM TBL_LINKED_LIBS "ZLIB::ZLIB")
message(STATUS "TBL_LINKED_LIBS (test): ${TBL_LINKED_LIBS}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly certain this was intentionally removed due to a bug in LLVM's packaging of the dylib

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you work around it, since linking failed on the Julia side without this line.
Also, any other requests before landing this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I am also OK with having another switch, so julia can keep it's old code path.)
Rust can deal with upstream LLVM if the zlib packaging comes back to cause us issues, in case that its' still the case. I feel like Julia should probably still move over since I think it currently only works by coincidence, see my first point above, but I don't want to debug julia precompilation since I already have enough cmake and bootstrap challenges.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do these things one at a time to confirm they're fine.

Can you leave this as a non-functional change (aka keep old zlib behavior) first, but add the non dylib linking, and then a follow up for zlib change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that I only added the zlib change, because Julia CI was failing (locally it worked fine without the patch). See https://github.com/EnzymeAD/Enzyme/actions/runs/14826186325/job/41619569312
For some reason it doesn't seem to pick up the /usr zlib library?

#list(REMOVE_ITEM TBL_LINKED_LIBS "ZLIB::ZLIB")
set_property(TARGET LLVMSupport PROPERTY INTERFACE_LINK_LIBRARIES ${TBL_LINKED_LIBS})
endif()

Expand Down Expand Up @@ -145,7 +147,36 @@ if (${ENZYME_EXTERNAL_SHARED_LIB})
add_dependencies(Enzyme-${LLVM_VERSION_MAJOR} BlasDeclarationsIncGen)
add_dependencies(Enzyme-${LLVM_VERSION_MAJOR} BlasTAIncGen)
add_dependencies(Enzyme-${LLVM_VERSION_MAJOR} BlasDiffUseIncGen)
target_link_libraries(Enzyme-${LLVM_VERSION_MAJOR} LLVM)

if (${LLVM_BUILD_LLVM_DYLIB})
target_link_libraries(Enzyme-${LLVM_VERSION_MAJOR} LLVM)
else()
# This would be the desired way to link against LLVM components,
# however this function is bugged and does not work with `all`, see:
# https://github.com/llvm/llvm-project/issues/46347
#llvm_map_components_to_libnames(llvm_libraries Passes)
# Therefore, manually invoke llvm-config
if (EXISTS "${LLVM_TOOLS_BINARY_DIR}/llvm-config")
message(STATUS "Using llvm-config from ${LLVM_TOOLS_BINARY_DIR}")
else()
message(SEND_ERROR "llvm-config not found in ${LLVM_TOOLS_BINARY_DIR}")
endif()
execute_process(COMMAND ${LLVM_TOOLS_BINARY_DIR}/llvm-config --libs all
OUTPUT_VARIABLE llvm_libraries)
string(STRIP "${llvm_libraries}" llvm_libraries)
message(STATUS "Linking against LLVM libraries: ${llvm_libraries}")
# In theory, adding --libs should also add all the -l flags,
# but it isn't picked up correctly by clang, so we call target_link_libraries
execute_process(COMMAND ${LLVM_TOOLS_BINARY_DIR}/llvm-config --ldflags
OUTPUT_VARIABLE llvm_ldflags)
string(STRIP "${llvm_ldflags}" llvm_ldflags)
message(STATUS "Linking against LLVM ldflags: ${llvm_ldflags}")
set_target_properties(Enzyme-${LLVM_VERSION_MAJOR} PROPERTIES LINK_FLAGS ${llvm_ldflags})
target_link_libraries(Enzyme-${LLVM_VERSION_MAJOR} ${llvm_libraries})

llvm_map_components_to_libnames(llvm_librariess Passes Support)
target_link_libraries(Enzyme-${LLVM_VERSION_MAJOR} ${llvm_librariess})
endif()
install(TARGETS Enzyme-${LLVM_VERSION_MAJOR}
EXPORT EnzymeTargets
LIBRARY DESTINATION lib COMPONENT shlib
Expand Down
Loading