Skip to content
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

Shared lib for tt-forge #257

Merged
merged 5 commits into from
Aug 7, 2024
Merged

Shared lib for tt-forge #257

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d7da85
Shared lib for `tt-forge`
mtopalovicTT Jul 31, 2024
7ee5dfe
Adding SPDX header
topke011 Jul 31, 2024
1592277
Remove files which were added by mistake
topke011 Jul 31, 2024
f7edbcc
Addressing comments
mtopalovicTT Aug 1, 2024
c0bc269
Addressing comments
mtopalovicTT Aug 1, 2024
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
1 change: 1 addition & 0 deletions lib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ add_subdirectory(CAPI)
add_subdirectory(Conversion)
add_subdirectory(Dialect)
add_subdirectory(Target)
add_subdirectory(SharedLib)

add_mlir_library(TTMLIR STATIC RegisterAll.cpp
mtopalovicTT marked this conversation as resolved.
Show resolved Hide resolved
LINK_LIBS PUBLIC
Expand Down
65 changes: 65 additions & 0 deletions lib/SharedLib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# We supply empty.cpp because CMake does not allow creating a library without sources.
add_library(TTMLIRSO SHARED empty.cpp)

set(METAL_LIB_DIR "${CMAKE_SOURCE_DIR}/third_party/tt-metal/src/tt-metal-build/lib")
mtopalovicTT marked this conversation as resolved.
Show resolved Hide resolved

add_library(ttnn SHARED IMPORTED)
set_property(TARGET ttnn PROPERTY IMPORTED_LOCATION "${METAL_LIB_DIR}/_ttnn.so")

add_dependencies(TTMLIRSO
TTNNTargetFlatbuffer
MLIRTTDialect
MLIRTTIRDialect
MLIRTTNNDialect
MLIRTTKernelDialect
TTMLIRTTIRToTTNN
MLIRTTMetalDialect
MLIRTTIRTransforms
MLIRTTNNTransforms
MLIRTTIRAnalysis
MLIRTTNNPipelines
TTMLIRTTNNToEmitC
TTRuntime
TTRuntimeTTNN)

target_link_libraries(TTMLIRSO PRIVATE
# LLVM
LLVM
MLIR

# Forces the inclusion of all symbols in the shared object
# This is necessary because the JIT will not be able to find the symbols otherwise
-Wl,--whole-archive
# MLIR libraries
TTNNTargetFlatbuffer
MLIRTTDialect
MLIRTTIRDialect
MLIRTTNNDialect
MLIRTTKernelDialect
TTMLIRTTIRToTTNN
MLIRTTMetalDialect
MLIRTTIRTransforms
MLIRTTNNTransforms
MLIRTTIRAnalysis
MLIRTTNNPipelines
TTMLIRTTNNToEmitC
mtopalovicTT marked this conversation as resolved.
Show resolved Hide resolved
TTRuntime
TTRuntimeTTNN
# MLIR libraries
-Wl,--no-whole-archive
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing these linker flags we should change the target_link_libraries, PRIVATE to PUBLIC, which should export the symbols of these static libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this work because tt-mlir is built in separate project and its targets are not visible to tt-forge, hence adding public wouldn't change anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think PUBLIC implies -Wl,--no-whole-archive, would you mind trying?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean PUBLIC implies -Wl,--whole-archive?
Btw, I tried PUBLIC initially, thats how I've ended up with whole-archive. Just to confirm I tried it again today and same issue. But I will try again to be triple sure :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I don't want to block the review because we should get this in. But something else must be messed up somewhere in the cmake tree, this is what PUBLIC means so I'm not sure why it would be hiding the symbols.


# Metal
tt_metal
device
tt_eager
ttnn
tt_metal
device
tt_eager
mtopalovicTT marked this conversation as resolved.
Show resolved Hide resolved

# Other
flatbuffers)

target_link_directories(TTMLIRSO PRIVATE
${TTMLIR_TOOLCHAIN_DIR}/lib
${METAL_LIB_DIR})
3 changes: 3 additions & 0 deletions lib/SharedLib/empty.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// SPDX-FileCopyrightText: (c) 2024 Tenstorrent AI ULC
//
// SPDX-License-Identifier: Apache-2.0
Loading