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.

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

[libshortfin] Clean up a few CI related items. #163

Merged
merged 10 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .github/workflows/ci_linux_x64-libshortfin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ jobs:
-DCMAKE_C_COMPILER=clang-18 \
-DCMAKE_CXX_COMPILER=clang++-18 \
-DCMAKE_LINKER_TYPE=LLD \
-DSHORTFIN_BUNDLE_DEPS=ON \
-DCMAKE_PREFIX_PATH=${{ env.IREE_REPO_DIR }}/build/lib/cmake/IREE \
-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
..
Expand All @@ -111,12 +112,16 @@ jobs:
run: |
mkdir ${{ env.LIBSHORTFIN_DIR }}/build-host-only
cd ${{ env.LIBSHORTFIN_DIR }}/build-host-only
# In this configuration, also build static+dynamic in order to verify
# that path structurally works.
cmake -GNinja \
-DCMAKE_C_COMPILER=clang-18 \
-DCMAKE_CXX_COMPILER=clang++-18 \
-DCMAKE_LINKER_TYPE=LLD \
-DCMAKE_PREFIX_PATH=${{ env.IREE_REPO_DIR }}/build/lib/cmake/IREE \
-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
-DSHORTFIN_HAVE_AMDGPU=OFF \
-DSHORTFIN_BUILD_STATIC=ON \
-DSHORTFIN_BUILD_DYNAMIC=ON \
..
cmake --build . --target all
21 changes: 12 additions & 9 deletions .github/workflows/ci_linux_x64_asan-libshortfin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,19 @@ env:
PYENV_ROOT: ${{ github.workspace }}/pyenv
PYENV_REF: 9ecd803bffaffb949fbdd8c70cb086227f6a3202 # v2.4.10
PYTHON_VER: 3.12.3
CACHE_ASAN_VER: 1
CACHE_ASAN_VER: 2
CACHE_DEPS_VER: 1
IREE_SOURCE_DIR: ${{ github.workspace }}/iree
LIBSHORTFIN_DIR: ${{ github.workspace }}/libshortfin/


jobs:
setup-python-asan:
name: Setup Python ASan
runs-on: ubuntu-24.04
env:
# The Python build process leaks. Here we just disable leak checking vs
# being more precise.
ASAN_OPTIONS: detect_leaks=0

steps:
- name: Cache Python ASan
Expand Down Expand Up @@ -64,15 +67,18 @@ jobs:
cd ${{ env.PYENV_ROOT }}
src/configure && make -C src
export PATH=${{ env.PYENV_ROOT }}/bin:$PATH && eval "$(pyenv init -)"
CC=clang-18 CXX=clang++-18 LDFLAGS="-lstdc++" PYTHON_CONFIGURE_OPTS="--with-address-sanitizer" pyenv install -v -g ${{ env.PYTHON_VER }}
pyenv global ${{ env.PYTHON_VER }}-debug
CC=clang-18 CXX=clang++-18 LDFLAGS="-lstdc++" PYTHON_CONFIGURE_OPTS="--with-address-sanitizer" pyenv install -v ${{ env.PYTHON_VER }}
pyenv global ${{ env.PYTHON_VER }}


build-and-test:
name: Build and test libshortfin
needs: [setup-python-asan]
runs-on: ubuntu-24.04

env:
# TODO(#151): Don't ignore ODR violations
ASAN_OPTIONS: detect_odr_violation=0
LSAN_OPTIONS: suppressions=${{ github.workspace }}/libshortfin/build_tools/python_lsan_suppressions.txt
steps:
- name: Install dependencies
run: |
Expand Down Expand Up @@ -135,9 +141,6 @@ jobs:
key: ${{ steps.cache-python-deps-restore.outputs.cache-primary-key }}

- name: Build libshortfin
env:
# TODO(#151): Don't ignore ODR violations
ASAN_OPTIONS=detect_odr_violation: 0
run: |
eval "$(pyenv init -)"
mkdir ${{ env.LIBSHORTFIN_DIR }}/build
Expand Down Expand Up @@ -168,4 +171,4 @@ jobs:
run: |
eval "$(pyenv init -)"
cd ${{ env.LIBSHORTFIN_DIR }}
pytest -m "not requires_amd_gpu"
pytest -m "not requires_amd_gpu" -s
20 changes: 19 additions & 1 deletion libshortfin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ project(
VERSION 0.9
LANGUAGES C CXX)

include(CMakeDependentOption)

set(SOVERSION 1)

set(CMAKE_C_STANDARD 11)
Expand All @@ -34,10 +36,26 @@ endif()
# build options
option(SHORTFIN_BUILD_PYTHON_BINDINGS "Builds Python Bindings" OFF)
option(SHORTFIN_BUILD_TESTS "Builds C++ tests" ON)
option(SHORTFIN_BUNDLE_DEPS "Download dependencies instead of using system libraries" OFF)
option(SHORTFIN_BUNDLE_DEPS "Download dependencies instead of using system libraries" ON)

set(SHORTFIN_IREE_SOURCE_DIR "" CACHE FILEPATH "Path to IREE source")

# Options for building static or dynamic libraries.
option(SHORTFIN_BUILD_STATIC "Builds static libraries" OFF)
option(SHORTFIN_BUILD_DYNAMIC "Builds dynamic libraries" ON)
cmake_dependent_option(SHORTFIN_LINK_DYNAMIC "Links internal binaries against static libshortfin.a" ON "SHORTFIN_BUILD_DYNAMIC" OFF)
if(NOT SHORTFIN_BUILD_STATIC AND NOT SHORTFIN_BUILD_DYNAMIC)
message(FATAL_ERROR "One of SHORTFIN_BUILD_STATIC or SHORTFIN_BUILD_DYNAMIC must be ON")
endif()
message(STATUS "Shortfin build static = ${SHORTFIN_BUILD_STATIC}, dynamic = ${SHORTFIN_BUILD_DYNAMIC}")
if(SHORTFIN_LINK_DYNAMIC)
message(STATUS "Dynamic linking to shortfin")
set(SHORTFIN_LINK_LIBRARY_NAME "shortfin")
else()
message(STATUS "Static linking to shortfin-static")
set(SHORTFIN_LINK_LIBRARY_NAME "shortfin-static")
endif()

# Enabling ASAN. Note that this will work best if building in a completely
# bundled fashion and with an ASAN rigged CPython. Otherwise, various LD_PRELOAD
# hacks are needed. This is merely a develope convenience: people are more
Expand Down
17 changes: 8 additions & 9 deletions libshortfin/bindings/python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
# build. - _shortfin_tracing.lib: Native library with tracing enabled (TODO). -
# Others.

# Detect the installed nanobind package and import it into CMake
execute_process(
COMMAND "${Python_EXECUTABLE}" -m nanobind --cmake_dir
OUTPUT_STRIP_TRAILING_WHITESPACE
OUTPUT_VARIABLE nanobind_ROOT)
find_package(nanobind CONFIG REQUIRED)
# nanobind
FetchContent_Declare(
nanobind
GIT_REPOSITORY https://github.com/wjakob/nanobind.git
GIT_TAG 9641bb7151f04120013b812789b3ebdfa7e7324f # 2.1.0
)
FetchContent_MakeAvailable(nanobind)

nanobind_add_module(shortfin_python_extension NB_STATIC LTO
array_binding.cc
Expand All @@ -26,9 +27,7 @@ set_target_properties(shortfin_python_extension
PROPERTIES OUTPUT_NAME "_shortfin_default/lib")

target_link_libraries(shortfin_python_extension
# TODO: This should be configurable as to whether we link to the static
# or dynamic version.
PRIVATE shortfin
PRIVATE ${SHORTFIN_LINK_LIBRARY_NAME}
)

nanobind_add_stub(
Expand Down
120 changes: 63 additions & 57 deletions libshortfin/build_tools/cmake/shortfin_library.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,24 @@ function(shortfin_public_library)
"COMPONENTS"
${ARGN}
)
shortfin_components_to_static_libs(_STATIC_COMPONENTS ${_RULE_COMPONENTS})
shortfin_components_to_dynamic_libs(_DYLIB_COMPONENTS ${_RULE_COMPONENTS})
# Static library.
add_library("${_RULE_NAME}-static" STATIC)
target_link_libraries(
"${_RULE_NAME}-static" PUBLIC ${_STATIC_COMPONENTS}
)
if(SHORTFIN_BUILD_STATIC)
# Static library.
shortfin_components_to_static_libs(_STATIC_COMPONENTS ${_RULE_COMPONENTS})
add_library("${_RULE_NAME}-static" STATIC)
target_link_libraries(
"${_RULE_NAME}-static" PUBLIC ${_STATIC_COMPONENTS}
)
endif()

# Dylib library.
add_library("${_RULE_NAME}" SHARED)
target_compile_definitions("${_RULE_NAME}" INTERFACE _SHORTFIN_USING_DYLIB)
target_link_libraries(
"${_RULE_NAME}" PUBLIC ${_DYLIB_COMPONENTS}
)
if(SHORTFIN_BUILD_DYNAMIC)
# Dylib library.
shortfin_components_to_dynamic_libs(_DYLIB_COMPONENTS ${_RULE_COMPONENTS})
add_library("${_RULE_NAME}" SHARED)
target_compile_definitions("${_RULE_NAME}" INTERFACE _SHORTFIN_USING_DYLIB)
target_link_libraries(
"${_RULE_NAME}" PUBLIC ${_DYLIB_COMPONENTS}
)
endif()
endfunction()

function(shortfin_cc_component)
Expand All @@ -48,50 +52,52 @@ function(shortfin_cc_component)
"HDRS;SRCS;DEPS;COMPONENTS"
${ARGN}
)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
set(_STATIC_OBJECTS_NAME "${_RULE_NAME}.objects")
set(_DYLIB_OBJECTS_NAME "${_RULE_NAME}.dylib.objects")

shortfin_components_to_static_libs(_STATIC_COMPONENTS ${_RULE_COMPONENTS})
shortfin_components_to_dynamic_libs(_DYLIB_COMPONENTS ${_RULE_COMPONENTS})

# Static object library.
add_library(${_STATIC_OBJECTS_NAME} OBJECT)
target_sources(${_STATIC_OBJECTS_NAME}
PRIVATE
${_RULE_SRCS}
${_RULE_HDRS}
)
target_compile_options(${_STATIC_OBJECTS_NAME} PRIVATE ${SHORTFIN_DEFAULT_COPTS})
target_link_libraries(${_STATIC_OBJECTS_NAME}
PUBLIC
_shortfin_defs
${_STATIC_COMPONENTS}
${_RULE_DEPS}
)
if(SHORTFIN_BUILD_STATIC)
# Static object library.
set(_STATIC_OBJECTS_NAME "${_RULE_NAME}.objects")
shortfin_components_to_static_libs(_STATIC_COMPONENTS ${_RULE_COMPONENTS})
add_library(${_STATIC_OBJECTS_NAME} OBJECT)
target_sources(${_STATIC_OBJECTS_NAME}
PRIVATE
${_RULE_SRCS}
${_RULE_HDRS}
)
target_compile_options(${_STATIC_OBJECTS_NAME} PRIVATE ${SHORTFIN_DEFAULT_COPTS})
target_link_libraries(${_STATIC_OBJECTS_NAME}
PUBLIC
_shortfin_defs
${_STATIC_COMPONENTS}
${_RULE_DEPS}
)
endif()

# Dylib object library.
add_library(${_DYLIB_OBJECTS_NAME} OBJECT)
target_sources(${_DYLIB_OBJECTS_NAME}
PRIVATE
${_RULE_SRCS}
${_RULE_HDRS}
)
target_compile_options(${_DYLIB_OBJECTS_NAME} PRIVATE ${SHORTFIN_DEFAULT_COPTS})
target_link_libraries(${_DYLIB_OBJECTS_NAME}
PUBLIC
_shortfin_defs
${_DYLIB_COMPONENTS}
${_RULE_DEPS}
)
set_target_properties(
${_DYLIB_OBJECTS_NAME} PROPERTIES
CXX_VISIBILITY_PRESET hidden
C_VISIBILITY_PRESET hidden
VISIBILITY_INLINES_HIDDEN ON
)
target_compile_definitions(${_DYLIB_OBJECTS_NAME}
PRIVATE _SHORTFIN_BUILDING_DYLIB)
if(SHORTFIN_BUILD_DYNAMIC)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
set(_DYLIB_OBJECTS_NAME "${_RULE_NAME}.dylib.objects")
shortfin_components_to_dynamic_libs(_DYLIB_COMPONENTS ${_RULE_COMPONENTS})
# Dylib object library.
add_library(${_DYLIB_OBJECTS_NAME} OBJECT)
target_sources(${_DYLIB_OBJECTS_NAME}
PRIVATE
${_RULE_SRCS}
${_RULE_HDRS}
)
target_compile_options(${_DYLIB_OBJECTS_NAME} PRIVATE ${SHORTFIN_DEFAULT_COPTS})
target_link_libraries(${_DYLIB_OBJECTS_NAME}
PUBLIC
_shortfin_defs
${_DYLIB_COMPONENTS}
${_RULE_DEPS}
)
set_target_properties(
${_DYLIB_OBJECTS_NAME} PROPERTIES
CXX_VISIBILITY_PRESET hidden
C_VISIBILITY_PRESET hidden
VISIBILITY_INLINES_HIDDEN ON
)
target_compile_definitions(${_DYLIB_OBJECTS_NAME}
PRIVATE _SHORTFIN_BUILDING_DYLIB)
endif()
endfunction()

function(shortfin_components_to_static_libs out_static_libs)
Expand Down Expand Up @@ -122,7 +128,7 @@ function(shortfin_gtest_test)
add_executable(${_RULE_NAME} ${_RULE_SRCS})
target_link_libraries(${_RULE_NAME} PRIVATE
${_RULE_DEPS}
shortfin
${SHORTFIN_LINK_LIBRARY_NAME}
GTest::gmock
GTest::gtest_main
)
Expand Down
2 changes: 2 additions & 0 deletions libshortfin/build_tools/python_lsan_suppressions.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
leak:PyUnicode_New
leak:_PyUnicodeWriter_Finish
1 change: 0 additions & 1 deletion libshortfin/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ requires = [
"cmake>=3.29",
"setuptools>=61.0",
"wheel",
"nanobind>=2.0",
"ninja",
]
build-backend = "setuptools.build_meta"
Expand Down
1 change: 0 additions & 1 deletion libshortfin/requirements-tests.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
nanobind==2.1.0
pytest
requests
fastapi
Expand Down
Loading