Re-merge #1229 (#1346)

* compiler build

* unity dp works

* format

* fmt

* fix

* add substitutions, compiler, and their unit tests to CI

* disable runtime unit test

* minor fix

* (not compilable) visitable issue for OptimalCostState

* fix machine mapping hash & refactor dp algorithm

* minor fix

* fix variant issue

* fmt

* fix

* fmt

* fix

* add more unit tests

* fmt

* Fix post-merge

* Add shell hook for sapling development

* changed from nullopt to std::nullopt

* fix cast issue

* Fix spdlog cmake issue

* Re-remove submodules

* minor fix & fmt

* upd tests name to match ci

* Add TEST_SUITE declaration to make tests findable by ctest

* Remove unnecessary nix files, add utils test to ci

* Fix utils tests name, format

---------

Co-authored-by: wmdi <[email protected]>
Co-authored-by: Pietro Max Marsella <[email protected]>

.flake/patches/doctest-template-test.patch

@@ -0,0 +1,50 @@
+diff --git a/scripts/cmake/doctestAddTests.cmake b/scripts/cmake/doctestAddTests.cmake
+index 3b25485..d3ba906 100644
+--- a/scripts/cmake/doctestAddTests.cmake
++++ b/scripts/cmake/doctestAddTests.cmake
+@@ -56,12 +56,14 @@ foreach(line ${output})
+   if("${line}" STREQUAL "===============================================================================" OR "${line}" MATCHES [==[^\[doctest\] ]==])
+     continue()
+   endif()
+-  set(test ${line})
++  set(unescaped_test ${line})
++  # use escape commas to handle properly test cases with commas inside the name
++  string(REPLACE "," "\\," escaped_test ${unescaped_test})
+   set(labels "")
+   if(${add_labels})
+     # get test suite that test belongs to
+     execute_process(
+-      COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" --test-case=${test} --list-test-suites
++      COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" --test-case=${escaped_test} --list-test-suites
+       OUTPUT_VARIABLE labeloutput
+       RESULT_VARIABLE labelresult
+       WORKING_DIRECTORY "${TEST_WORKING_DIR}"
+@@ -85,24 +87,22 @@ foreach(line ${output})
+
+   if(NOT "${junit_output_dir}" STREQUAL "")
+     # turn testname into a valid filename by replacing all special characters with "-"
+-    string(REGEX REPLACE "[/\\:\"|<>]" "-" test_filename "${test}")
++    string(REGEX REPLACE "[/\\:\"|<>]" "-" test_filename "${unescaped_test}")
+     set(TEST_JUNIT_OUTPUT_PARAM "--reporters=junit" "--out=${junit_output_dir}/${prefix}${test_filename}${suffix}.xml")
+   else()
+     unset(TEST_JUNIT_OUTPUT_PARAM)
+   endif()
+-  # use escape commas to handle properly test cases with commas inside the name
+-  string(REPLACE "," "\\," test_name ${test})
+   # ...and add to script
+   add_command(add_test
+-    "${prefix}${test}${suffix}"
++    "${prefix}${unescaped_test}${suffix}"
+     ${TEST_EXECUTOR}
+     "${TEST_EXECUTABLE}"
+-    "--test-case=${test_name}"
++    "--test-case=${escaped_test}"
+     "${TEST_JUNIT_OUTPUT_PARAM}"
+     ${extra_args}
+   )
+   add_command(set_tests_properties
+-    "${prefix}${test}${suffix}"
++    "${prefix}${unescaped_test}${suffix}"
+     PROPERTIES
+     WORKING_DIRECTORY "${TEST_WORKING_DIR}"
+     ${properties}

.github/workflows/helpers/build_libs.sh

@@ -0,0 +1,9 @@
+#! /usr/bin/env bash
+
+set -euo pipefail
+
+DIR="$(realpath -- "$(dirname "${BASH_SOURCE[0]}")")"
+REPO="$(realpath -- "$DIR/../../../")"
+
+cd "$REPO/build-ci"
+make -j $(( $(nproc) < 2 ? 1 : $(nproc)-1 )) "$@"

.github/workflows/helpers/build_cuda.sh → .github/workflows/helpers/cmake_cuda.sh

@@ -8,22 +8,21 @@ REPO="$(realpath -- "$DIR/../../../")"
 
 export FF_GPU_BACKEND="cuda"
 export FF_CUDA_ARCH=70
-cd "$REPO"
-mkdir build
-cd build
+
+if [[ -d "$REPO/build-ci" ]]; then 
+  rm -rf "$REPO/build-ci"
+fi
+mkdir "$REPO/build-ci"
+cd "$REPO/build-ci"
 #if [[ "${FF_GPU_BACKEND}" == "cuda" ]]; then
 #  export FF_BUILD_ALL_EXAMPLES=ON
 #  export FF_BUILD_UNIT_TESTS=ON
 #fi
+IFS=" " read -r -a FLAGS <<< "$CMAKE_FLAGS"
 ../config/config.linux \
-        -DCMAKE_CXX_COMPILER="clang++" \
-        -DCMAKE_C_COMPILER="clang" \
         -DCMAKE_C_COMPILER_LAUNCHER=ccache \
         -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
         -DCMAKE_CUDA_COMPILER_LAUNCHER=ccache \
-        -DFF_USE_EXTERNAL_LEGION=ON \
-        -DFF_USE_EXTERNAL_JSON=ON \
-        -DFF_USE_EXTERNAL_FMT=ON \
-        -DFF_USE_EXTERNAL_SPDLOG=ON
+        "${FLAGS[@]}"
 
 # vim: set tabstop=2 shiftwidth=2 expandtab:

.github/workflows/helpers/test_libs.sh

@@ -0,0 +1,14 @@
+#! /usr/bin/env bash
+
+set -euo pipefail
+set -x
+
+DIR="$(realpath -- "$(dirname "${BASH_SOURCE[0]}")")"
+REPO="$(realpath -- "$DIR/../../../")"
+
+TEST_LIBS=("${@/%/-tests}")
+REGEX="^$(IFS='|'; echo "${TEST_LIBS[*]}")\$"
+
+cd "$REPO/build-ci"
+make -j $(( $(nproc) < 2 ? 1 : $(nproc)-1 )) "${TEST_LIBS[@]}"
+ctest --progress --output-on-failure -L "$REGEX"

.github/workflows/per-lib-check.yml

@@ -20,6 +20,9 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Add helpers directory to path
+        run: echo "${PWD}/.github/workflows/helpers" >> $GITHUB_PATH
+
       - name: Install nix
         uses: cachix/install-nix-action@v25
         with:
@@ -51,24 +54,40 @@ jobs:
 
       - name: Run cmake
         run: |
-          .github/workflows/helpers/build_${{ matrix.gpu_backend }}.sh
+          cmake_${{ matrix.gpu_backend }}.sh
 
       - name: Build utils
         run: |
-          cd build 
-          make -j $(( $(nproc) < 2 ? 1 : $(nproc)-1 )) utils
+          build_libs.sh utils
 
       - name: Build op-attrs
         run: |
-          cd build 
-          make -j $(( $(nproc) < 2 ? 1 : $(nproc)-1 )) op-attrs
+          build_libs.sh op-attrs
 
       - name: Build pcg
         run: |
-          cd build
-          make -j $(( $(nproc) < 2 ? 1 : $(nproc)-1 )) pcg
+          build_libs.sh pcg
 
       - name: Build kernels
         run: |
-          cd build
-          make -j $(( $(nproc) < 2 ? 1 : $(nproc)-1 )) kernels
+          build_libs.sh kernels
+
+      - name: Build substitutions
+        run: |
+          build_libs.sh substitutions
+
+      - name: Build compiler
+        run: |
+          build_libs.sh compiler
+
+      - name: Test utils
+        run: |
+          test_libs.sh utils
+
+      - name: Test substitutions
+        run: |
+          test_libs.sh substitutions
+
+      - name: Test compiler
+        run: |
+          test_libs.sh compiler

CMakeLists.txt

@@ -84,7 +84,7 @@ include(nccl)
 include(json)
 include(expected)
 include(spdlog)
-include(doctest)
+include(doctestlib) # named doctestlib to avoid a name collision with doctest.cmake in rapidcheck
 include(visit_struct)
 include(CTest)
 include(fmt)

cmake/doctestlib.cmake

@@ -0,0 +1,11 @@
+include(aliasing)
+
+if (FF_USE_EXTERNAL_DOCTEST)
+  find_package(doctest REQUIRED)
+  include(doctest) # import doctest_discover_tests
+else()
+  add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/deps/doctest)
+  include(${CMAKE_CURRENT_SOURCE_DIR}/deps/doctest/scripts/cmake/doctest.cmake)
+endif()
+
+alias_library(doctest doctest::doctest)

cmake/flexflow-utils.cmake

@@ -118,7 +118,9 @@ function(ff_add_test_executable)
     ${FF_TEST_EXEC_NAME}
     ${FF_TEST_EXEC_DEPS})
 
+  target_compile_definitions(${FF_TEST_EXEC_NAME} PRIVATE FF_TEST_SUITE="${FF_TEST_EXEC_NAME}")
+
   define_ff_vars(${FF_TEST_EXEC_NAME})
   ff_set_cxx_properties(${FF_TEST_EXEC_NAME})
-  doctest_discover_tests(${FF_TEST_EXEC_NAME})
+  doctest_discover_tests(${FF_TEST_EXEC_NAME} ADD_LABELS 1)
 endfunction()

cmake/fmt.cmake

@@ -4,6 +4,5 @@ if (FF_USE_EXTERNAL_FMT)
   find_package(fmt REQUIRED)
 else()
   add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/deps/fmt)
-
-  alias_library(fmt fmt::fmt)
 endif()
+alias_library(fmt fmt::fmt)

cmake/nccl.cmake

@@ -8,6 +8,7 @@ else()
   message(STATUS "Building NCCL from source")
   list(TRANSFORM CUDA_GENCODE PREPEND "NVCC_GENCODE=" OUTPUT_VARIABLE NCCL_BUILD_NVCC_GENCODE)
 
+  include(ExternalProject)
   ExternalProject_Add(nccl_source_build
     SOURCE_DIR ${PROJECT_SOURCE_DIR}/deps/${NCCL_NAME}
     PREFIX ${CMAKE_BINARY_DIR}/deps/${NCCL_NAME}

cmake/rapidcheck.cmake

@@ -1 +1,5 @@
-add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/deps/rapidcheck)
+if (FF_USE_EXTERNAL_RAPIDCHECK)
+  find_package(rapidcheck REQUIRED)
+else()
+  add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/deps/rapidcheck)
+endif()

cmake/spdlog.cmake

@@ -4,6 +4,9 @@ if (FF_USE_EXTERNAL_SPDLOG)
   find_package(spdlog REQUIRED)
 else()
   add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/deps/spdlog)
-
-  alias_library(spdlog spdlog::spdlog)
 endif()
+
+add_library(ff_spdlog INTERFACE)
+target_link_libraries(ff_spdlog INTERFACE spdlog::spdlog)
+target_compile_definitions(ff_spdlog INTERFACE SPDLOG_FMT_EXTERNAL)
+alias_library(spdlog ff_spdlog)