syn: fix static-library dependency cycle between syn_flow and syn_lib#10557
Conversation
libsyn_flow.a references Synthesis::dontUse(), defined in syn_lib. Add syn_lib to the link line to resolve it. Matches Bazel BUILD. Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates src/syn/test/CMakeLists.txt to link syn_lib to several test targets and adjusts the linking order. The reviewer notes that these changes introduce a circular dependency between syn_flow and syn_lib, which can cause build failures on platforms with single-pass linkers. They suggest breaking this dependency by refactoring how syn_flow accesses syn_lib data, such as using a callback or passing the data directly.
| foreach(_t opt_test bitblast_test) | ||
| add_executable(${_t} ${_t}.cc) | ||
| target_link_libraries(${_t} ${_syn_test_libs} syn_flow syn_ir) | ||
| target_link_libraries(${_t} ${_syn_test_libs} syn_flow syn_lib syn_ir) |
There was a problem hiding this comment.
While adding syn_lib and adjusting the link order resolves the immediate build issue on your local environment, it exposes a circular dependency between syn_flow and syn_lib (since syn_lib already links to syn_flow in src/syn/src/CMakeLists.txt).\n\nWith static libraries, circular dependencies (syn_lib -> syn_flow and syn_flow -> syn_lib) are highly fragile. A single-pass linker (like standard GNU ld) cannot resolve symbols in both directions unless libraries are repeated (e.g., syn_flow syn_lib syn_flow) or linker groups are used. This can lead to build failures on other platforms or toolchains.\n\n### Suggested Solution\nTo ensure a robust and portable build, we should break the circular dependency by removing syn_flow's direct dependency on syn_lib. Since syn_flow only references Synthesis::dontUse(), you can:\n1. Pass a callback or interface: Define an abstract interface or a std::function callback in syn_flow (or syn_ir) that queries the dont_use status. Synthesis can implement/provide this callback when invoking syn_flow.\n2. Pass the data directly: Pass the set of dont_use cells as a parameter to the flow.\n\nThis keeps the dependency graph as a clean Directed Acyclic Graph (DAG): syn_lib -> syn_flow -> syn_ir.
|
@povik why is there a cycle between syn_flow and syn_lib? |
|
syn_lib is the entry point and calls individual syn_flow passes. The dependency back from syn_flow to syn_lib is because of one Synthesis::dontUse helper to query the dont use status of individual cells. I think we should collapse the libraries into one. That has been already done on the Bazel side. |
|
@povik should we make that change instead of this one? |
|
We should make it eventually but we can merge this one in the interim. |
|
In the long term it seems overly heavy to pass the whole top level engine just to get a dontuse method. |
3 participants
libsyn_flow.a references Synthesis::dontUse(), defined in syn_lib. Add syn_lib to the link line to resolve it. Matches Bazel BUILD.
Summary
This fixes build issues I have been facing.
Type of Change
Impact
Succesfully build when using ORFS build script.
Verification
./etc/Build.sh).