From 9caffca3faaf0fe0c0bd219494d378f9773ca03d Mon Sep 17 00:00:00 2001
From: Catherine <whitequark@whitequark.org>
Date: Tue, 21 Jan 2025 14:37:16 +0000
Subject: [PATCH] CMake: remove `-DSERIALIZE_CHIPDBS=` option.

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.
---
 CMakeLists.txt    |  3 ---
 cmake/BBAsm.cmake | 11 -----------
 2 files changed, 14 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f7dd645895..25c3809bf0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -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 (Threads_FOUND)
     if (NOT STATIC_BUILD)
diff --git a/cmake/BBAsm.cmake b/cmake/BBAsm.cmake
index 1e456c5f34..c72b64deca 100644
--- a/cmake/BBAsm.cmake
+++ b/cmake/BBAsm.cmake
@@ -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: