Merge branch 'main' into grokstar

.github/workflows/ci_linux_x64-libshortfin.yml

@@ -57,25 +57,6 @@ jobs:
         git submodule update --init --depth 1 -- third_party/googletest
         git submodule update --init --depth 1 -- third_party/hip-build-deps/
 
-    - name: Build IREE runtime
-      run: |
-        mkdir ${{ env.IREE_REPO_DIR }}/build
-        cd ${{ env.IREE_REPO_DIR }}/build
-        cmake -GNinja \
-          -DCMAKE_C_COMPILER=clang-18 \
-          -DCMAKE_CXX_COMPILER=clang++-18 \
-          -DIREE_ENABLE_LLD=ON \
-          -DIREE_ERROR_ON_MISSING_SUBMODULES=OFF \
-          -DIREE_HAL_DRIVER_DEFAULTS=OFF \
-          -DIREE_HAL_DRIVER_LOCAL_SYNC=ON \
-          -DIREE_HAL_DRIVER_LOCAL_TASK=ON \
-          -DIREE_HAL_DRIVER_HIP=ON \
-          -DIREE_BUILD_COMPILER=OFF \
-          -DIREE_BUILD_SAMPLES=OFF \
-          -DIREE_BUILD_TESTS=OFF \
-          ..
-        cmake --build . --target all
-
     - name: Setup Python
       uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5.1.1
       with:
@@ -85,6 +66,7 @@ jobs:
       # TODO: Switch to `pip install -r requirements.txt -e libshortfin/`.
       run: |
         pip install -r ${{ env.LIBSHORTFIN_DIR }}/requirements-tests.txt
+        pip install -r ${{ env.LIBSHORTFIN_DIR }}/requirements-iree-compiler.txt
         pip freeze
 
     - name: Build libshortfin (full)
@@ -96,7 +78,7 @@ jobs:
           -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_IREE_SOURCE_DIR=${{ env.IREE_REPO_DIR }} \
           -DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
           ..
         cmake --build . --target all
@@ -107,7 +89,7 @@ jobs:
         cd ${{ env.LIBSHORTFIN_DIR }}/build
         ctest --timeout 30 --output-on-failure
         cd ${{ env.LIBSHORTFIN_DIR }}
-        pytest -s -v -m "not requires_amd_gpu"
+        pytest -s
 
     - name: Build libshortfin (host-only)
       run: |
@@ -119,7 +101,7 @@ jobs:
           -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_IREE_SOURCE_DIR=${{ env.IREE_REPO_DIR }} \
           -DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
           -DSHORTFIN_HAVE_AMDGPU=OFF \
           -DSHORTFIN_BUILD_STATIC=ON \

.github/workflows/ci_linux_x64_asan-libshortfin.yml

@@ -76,8 +76,10 @@ jobs:
     needs: [setup-python-asan]
     runs-on: ubuntu-24.04
     env:
-      # TODO(#151): Don't ignore ODR violations
-      ASAN_OPTIONS: detect_odr_violation=0
+      # We can't count on being leak free in general (i.e. pip, etc) so disable
+      # leak checker by default. Here we suppress any ASAN features needed to
+      # pass the build. Test configuration is done specially just for that step.
+      ASAN_OPTIONS: detect_leaks=0,detect_odr_violation=0
       LSAN_OPTIONS: suppressions=${{ github.workspace }}/libshortfin/build_tools/python_lsan_suppressions.txt
     steps:
     - name: Install dependencies
@@ -145,32 +147,19 @@ jobs:
     - name: Build libshortfin
       run: |
         eval "$(pyenv init -)"
-        mkdir ${{ env.LIBSHORTFIN_DIR }}/build
-        cd ${{ env.LIBSHORTFIN_DIR }}/build
-        cmake -GNinja \
-          -DCMAKE_BUILD_TYPE=Debug \
-          -DCMAKE_C_COMPILER=clang-18 \
-          -DCMAKE_CXX_COMPILER=clang++-18 \
-          -DCMAKE_LINKER_TYPE=LLD \
-          -DSHORTFIN_BUNDLE_DEPS=ON \
-          -DSHORTFIN_IREE_SOURCE_DIR=${{ env.IREE_SOURCE_DIR }} \
-          -DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
-          -DSHORTFIN_ENABLE_ASAN=ON \
-          ..
-        cmake --build . --target all
+        cd ${{ env.LIBSHORTFIN_DIR }}
+        SHORTFIN_IREE_SOURCE_DIR="${{ env.IREE_SOURCE_DIR }}" \
+        SHORTFIN_ENABLE_ASAN=ON \
+        SHORTFIN_DEV_MODE=ON \
+        SHORTFIN_RUN_CTESTS=ON \
         pip install -v -e .
 
-    - name: Run ctest
-      if: ${{ !cancelled() }}
-      env:
-        CTEST_OUTPUT_ON_FAILURE: 1
-      run: |
-        cd ${{ env.LIBSHORTFIN_DIR }}/build
-        ctest --timeout 30 --output-on-failure
-
     - name: Run pytest
       if: ${{ !cancelled() }}
+      env:
+        # TODO(#151): Don't ignore ODR violations
+        ASAN_OPTIONS: detect_odr_violation=0
       run: |
         eval "$(pyenv init -)"
         cd ${{ env.LIBSHORTFIN_DIR }}
-        pytest -m "not requires_amd_gpu" -s
+        pytest -s

libshortfin/CMakeLists.txt

@@ -4,7 +4,7 @@
 # https://llvm.org/LICENSE.txt for license information. SPDX-License-Identifier:
 # Apache-2.0 WITH LLVM-exception
 
-cmake_minimum_required(VERSION 3.28)
+cmake_minimum_required(VERSION 3.29)
 
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
   message(
@@ -37,6 +37,7 @@ endif()
 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" ON)
+option(SHORTFIN_ENABLE_TRACING "Enable runtime tracing for iree and shortfin" OFF)
 
 set(SHORTFIN_IREE_SOURCE_DIR "" CACHE FILEPATH "Path to IREE source")
 
@@ -126,24 +127,8 @@ endif()
 
 ## iree runtime
 
-if (NOT SHORTFIN_IREE_SOURCE_DIR AND SHORTFIN_BUNDLE_DEPS)
-  FetchContent_Declare(
-    iree
-    GIT_REPOSITORY https://github.com/iree-org/iree.git
-    GIT_TAG candidate-20240904.1006
-    # TODO: We shouldn't have to pull googletest when we are not building tests.
-    #       This needs to be fixed with IREE.
-    GIT_SUBMODULES "third_party/benchmark third_party/cpuinfo third_party/flatcc third_party/hip-build-deps third_party/googletest"
-    GIT_SHALLOW TRUE
-  )
-  FetchContent_GetProperties(iree)
-  if(NOT iree_POPULATED)
-    FetchContent_Populate(iree)
-  endif()
-  set(SHORTFIN_IREE_SOURCE_DIR ${iree_SOURCE_DIR})
-endif()
-
-if(SHORTFIN_IREE_SOURCE_DIR)
+if (SHORTFIN_IREE_SOURCE_DIR OR SHORTFIN_BUNDLE_DEPS)
+  # Set IREE build flags, if we are building from source
   set(IREE_BUILD_COMPILER OFF)
   set(IREE_BUILD_TESTS OFF)
   set(IREE_BUILD_SAMPLES OFF)
@@ -156,7 +141,43 @@ if(SHORTFIN_IREE_SOURCE_DIR)
   if(SHORTFIN_SYSTEMS_AMDGPU)
     set(IREE_HAL_DRIVER_HIP ON)
   endif()
+  if (SHORTFIN_ENABLE_TRACING)
+    set(IREE_ENABLE_RUNTIME_TRACING ON)
+    # When using shared libraries there are some issues that need to be
+    # explored more on static initialization order. Something is getting
+    # initialized and is emitting tracy events before tracy objects are
+    # initialized. This can point to some shared library overloading allocation
+    # functions and making them emit tracy events, which are further used in
+    # some static allocation. See https://github.com/wolfpld/tracy/issues/196
+    # for a similar issue and discussion. Using the workaround suggested in
+    # that issue for now. Note that this does not happen when using static
+    # libraries.
+    set(TRACY_DELAYED_INIT ON CACHE BOOL "Enable delayed init for tracy")
+  endif()
+endif()
+
+if(SHORTFIN_IREE_SOURCE_DIR)
   add_subdirectory(${SHORTFIN_IREE_SOURCE_DIR} shortfin_iree SYSTEM EXCLUDE_FROM_ALL)
+elseif (SHORTFIN_BUNDLE_DEPS)
+  # TODO: We shouldn't have to pull googletest when we are not building tests.
+  #       This needs to be fixed with IREE.
+  set(IREE_SUBMODULES "third_party/benchmark third_party/cpuinfo third_party/flatcc third_party/hip-build-deps third_party/googletest")
+  if (SHORTFIN_ENABLE_TRACING)
+    set(IREE_SUBMODULES "${IREE_SUBMODULES} third_party/tracy")
+  endif()
+  FetchContent_Declare(
+    shortfin_iree
+    GIT_REPOSITORY https://github.com/iree-org/iree.git
+    GIT_TAG candidate-20240904.1006
+    GIT_SUBMODULES ${IREE_SUBMODULES}
+    GIT_SHALLOW TRUE
+    SYSTEM
+    EXCLUDE_FROM_ALL
+  )
+  FetchContent_GetProperties(shortfin_iree)
+  if(NOT shortfin_iree_POPULATED)
+    FetchContent_MakeAvailable(shortfin_iree)
+  endif()
 else()
   # Try to find iree using find_package
   find_package(IREERuntime)
@@ -165,7 +186,7 @@ endif()
 # tests
 
 if(SHORTFIN_BUILD_TESTS)
-  if (NOT SHORTFIN_IREE_SOURCE_DIR)
+  if (NOT SHORTFIN_BUNDLE_DEPS AND NOT SHORTFIN_IREE_SOURCE_DIR)
     # For now we use gtest shipped alongside with IREE.
     FetchContent_Declare(
       googletest
@@ -184,7 +205,7 @@ add_subdirectory(src)
 
 if(SHORTFIN_BUILD_PYTHON_BINDINGS)
   find_package(Python 3.8 COMPONENTS Interpreter Development.Module REQUIRED)
-  add_subdirectory(bindings/python)
+  add_subdirectory(python)
   set(SHORTFIN_PYTHON_CPP_PREBUILT "TRUE") # See setup.py.
   configure_file(setup.py setup.py @ONLY)
   configure_file(pyproject.toml pyproject.toml COPYONLY)

libshortfin/README.md

@@ -1,50 +1,66 @@
 # libshortfin - SHARK C++ inference library
 
-## Dev Builds
+Build options
 
-Library dependencies:
+1. Native C++ build
+2. Python release build
+3. Python dev build
 
-* [spdlog](https://github.com/gabime/spdlog)
-* [xtensor](https://github.com/xtensor-stack/xtensor)
-* [iree runtime](https://github.com/iree-org/iree)
-
-On recent Ubuntu, the primary dependencies can be satisfied via:
+## Native C++ Builds
 
 ```
-apt install libspdlog-dev libxtensor-dev
+cmake -GNinja -S. -Bbuild \
+    -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
+    -DCMAKE_LINKER_TYPE=LLD
 ```
 
-CMake must be told how to find the IREE runtime, either from a distribution
-tarball, or local build/install dir. For a local build directory, pass:
+If Python bindings are enabled in this mode (`-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON`),
+then `pip install -e build/` will install from the build dir (and support
+build/continue).
+
+## Python Release Builds
 
 ```
-# Assumes that the iree-build directory is adjacent to this repo.
--DCMAKE_PREFIX_PATH=$(pwd)/../../iree-build/lib/cmake/IREE
+pip install -v -e .
 ```
 
-One liner recommended CMake command (note that CMAKE_LINKER_TYPE requires
-cmake>=3.29):
+## Python Dev Builds
 
 ```
-cmake -GNinja -S. -Bbuild \
-    -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
-    -DCMAKE_LINKER_TYPE=LLD \
-    -DCMAKE_PREFIX_PATH=$(pwd)/../../iree-build/lib/cmake/IREE
+# Install build system pre-reqs (since we are building in dev mode, this
+# is not done for us). See source of truth in pyproject.toml:
+pip install setuptools wheel
+
+# Optionally install cmake and ninja if you don't have them or need a newer
+# version. If doing heavy development in Python, it is strongly recommended
+# to install these natively on your system as it will make it easier to
+# switch Python interpreters and build options (and the launcher in debug/asan
+# builds of Python is much slower). Note CMakeLists.txt for minimum CMake
+# version, which is usually quite recent.
+pip install cmake ninja
+
+SHORTFIN_DEV_MODE=ON pip install --no-build-isolation -v -e .
 ```
 
-## Building Python Bindings
+Note that the `--no-build-isolation` flag is useful in development setups
+because it does not create an intermediate venv that will keep later
+invocations of cmake/ninja from working at the command line. If just doing
+a one-shot build, it can be ommitted.
 
-If using a Python based development flow, there are two options:
+Once built the first time, `cmake`, `ninja`, and `ctest` commands can be run
+directly from `build/cmake` and changes will apply directly to the next
+process launch.
 
-1. `pip install -v .` to build and install the library (TODO: Not yet implemented).
-2. Build with cmake and `-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON` and then
-   from the `build/` directory, run `pip install -v -e .` to create an
-   editable install that will update as you build the C++ project.
+Several optional environment variables can be used with setup.py:
 
-If predominantly developing with a C++ based flow, the second option is
-recommended. Your python install should track any source file changes or
-builds without further interaction. Re-installing will be necessary if package
-structure changes significantly.
+* `SHORTFIN_CMAKE_BUILD_TYPE=Debug` : Sets the CMAKE_BUILD_TYPE. Defaults to
+  `Debug` for dev mode and `Release` otherwise.
+* `SHORTFIN_ENABLE_ASAN=ON` : Enables an ASAN build. Requires a Python runtime
+  setup that is ASAN clean (either by env vars to preload libraries or set
+  suppressions or a dev build of Python with ASAN enabled).
+* `SHORTFIN_IREE_SOURCE_DIR=$(pwd)/../../iree`
+* `SHORTFIN_RUN_CTESTS=ON` : Runs `ctest` as part of the build. Useful for CI
+  as it uses the version of ctest installed in the pip venv.
 
 ## Running Tests
 
@@ -61,6 +77,20 @@ does mean that the C++ core of the library must always be built with the
 Python bindings to test the most behavior. Given the target of the project,
 this is not considered to be a significant issue.
 
+### Python tests
+
+Run platform independent tests only:
+
+```
+pytest tests/
+```
+
+Run tests including for a specific platform:
+
+```
+pytest tests/ --system amdgpu
+```
+
 # Production Library Building
 
 In order to build a production library, additional build steps are typically

libshortfin/build_tools/python_lsan_suppressions.txt

@@ -1,2 +1,8 @@
 leak:PyUnicode_New
+leak:_PyUnicodeWriter_PrepareInternal
 leak:_PyUnicodeWriter_Finish
+leak:numpy
+leak:_mlir_libs
+leak:google/_upb
+leak:import_find_and_load
+leak:ufunc

libshortfin/pyproject.toml

@@ -12,9 +12,6 @@ addopts = [
     "-ra",
     "--import-mode=importlib",
 ]
-markers = [
-    "requires_amd_gpu: tests that require and AMD GPU (deselect with '-m \"not requires_amd_gpu\"')",
-]
 testpaths = [
     "tests",
 ]