caiyu/nextpnr
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

CMake: remove -DSERIALIZE_CHIPDBS= option.

Browse Source 
The impetus for this commit is the fact that it causes rare but
build-breaking race conditions when used with `make -jN` with `N > 1`.
These race conditions are difficult to track down or fix because of
the very rudmentary debugging tools provided by `make` and opaque
semantics of CMake's Makefile generator. They break the build by
running two `.bba` generation processes, then one of them renaming
the `.bba.new` file once it's done, leaving the other one to fail.

After reflection (as the author of this code path) and discussion with
community members who use it, I've concluded that this isn't the right
approach.
1. In practice, on targets where `-DSERIALIZE_CHIPDBS=` matters, you
   also care about other build steps, like linking nextpnr, which
   are not serializable this way. So you use a workaround anyway, like
   `make`ing individual targets instead.
2. The way to serialize the build with Make is the `-j1` option. Trying
   to work around `-jN` to make it work like `-j1` is inherently error
   prone. While there is some utility in not serializing C++ compilation
   this utility could be more easily achieved by providing a single
   target that builds all chipdbs, running `make <chipdb-target> -j1`,
   then running `make -jN` for the rest of the build.
This commit is contained in:
Catherine 2025-01-21 14:37:16 +00:00
parent dbba1328bf
commit 17943a51cb
2 changed files with 0 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CMakeLists.txt
View File

@ -25,7 +25,6 @@ option(BUILD_TESTS "Build tests" OFF)
option(USE_OPENMP "Use OpenMP to accelerate analytic placer" OFF)
option(STATIC_BUILD "Create static build" OFF)
option(EXTERNAL_CHIPDB "Create build with pre-built chipdb binaries" OFF)
option(SERIALIZE_CHIPDBS "Serialize device data preprocessing to minimize memory use" ON)
option(WERROR "pass -Werror to compiler (used for CI)" OFF)
option(PROFILER "Link against libprofiler" OFF)
option(USE_IPO "Compile nextpnr with IPO" ON)
@ -67,8 +66,6 @@ else()
set(BBASM_MODE "string")
endif()
set(BBASM_SERIALIZE ${SERIALIZE_CHIPDBS})
find_package(Threads)
if (NOT Threads_FOUND)
add_definitions(-DNPNR_DISABLE_THREADS)

11
cmake/BBAsm.cmake
View File

@ -39,20 +39,9 @@ function(add_bba_produce_command)
DEPENDS
${arg_EXECUTABLE}
${arg_INPUTS}
$ENV{SERIALIZE_BBA_TARGET}
VERBATIM
)
if (BBASM_SERIALIZE)
# Have to insert a custom target in between two custom commands, else CMake will try to
# depend on the previous (in serialization order) command directly, which will fail if
# they're in different directories. Unfortunately this makes the terminal output uglier.
math(EXPR next_count "$ENV{SERIALIZE_BBA_COUNT} + 1")
add_custom_target(--bbasm-serialize-${next_count} DEPENDS ${arg_OUTPUT})
set(ENV{SERIALIZE_BBA_COUNT} ${next_count})
set(ENV{SERIALIZE_BBA_TARGET} --bbasm-serialize-${next_count})
endif()
endfunction()
# Example usage: