-
Notifications
You must be signed in to change notification settings - Fork 977
CMake: Rebase From Main & Tests Working #5204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: micko/cmake
Are you sure you want to change the base?
Conversation
Thanks for cherry picking the changes into a new PR :) |
Thank you for giving me a chance to learn something new! |
5dd402f
to
083ffab
Compare
The second commit is to get libyosys.so working, according to the logs, it looks as if it should be functional |
Actually might want to roll back the latest commit, I've been spending a good chunk of time getting the cmake system working in a way thats more modular to get libyosys working and to get plugins building with cmake as well. I've had to take a hacksaw to a bunch of different parts to get it working (at least in a way that cmake likes). The project is not organized in a way that cmake likes and it seems to me to just be a big pool of object file soup. Regardless, the first commit should still be good, if there's ever a want to use CMake to build libyosys / plugins, it might warrant a refactor of the project to make the parts a little more modular e.g. There's a circular dependancy between kernel and frontend/verilog at the moment which is fine when its all linked into a single executable but has been causing me headaches |
Nextpnr is similar so you might want to take inspiration from it. (I handled the nextpnr build system refactor.) |
|
aae1fd9
to
79f7cfa
Compare
In the most recent commit, I got the CMake build working by creating an object library that both yosys and libyosys link into to build. All of the tests are passing except tests/various/plugin. The issue seems to be some kind of inconsistency between the makefile and CMake for how yosys and/or the yosys-config script are built. It seems very inconsistent and I'm not entirely sure how to fix it but the rest of the yosys functionality seems to be there. For the failing test, this is the output I get at the present moment:
And this is the diff between the two built yosys-config files:
|
I think when you've added the changes from main you missed a lot of files, for example gzip.cc is mentioned in the kernel cmakelists but isn't checked in |
Sorry, I may have misunderstood what I should have done. In my original PR, I rebased from main and that notified many people. I redid the PR as a cherry-pick of the changes. When main is merged into the branch, those changes are present. I don't contribute much to open source so I don't know the "correct" way of contributing, but I'd be happy to modify whatever needs modifying. |
I must have had some outdated build directory around or something myself then, I don't have that problem anymore, my bad! It builds fine on a second attempt |
CMakeLists.txt
Outdated
) | ||
|
||
add_custom_target(yosys-abc-copy ALL DEPENDS ${CMAKE_BINARY_DIR}/yosys-abc) | ||
add_custom_target(yosys-abc-copy ALL DEPENDS ${yosys_BINARY_DIR}/yosys-abc) | ||
endif() | ||
|
||
set(CMAKE_CXX_STANDARD ${CXXSTD}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rewrite is a great chance to make the C++ standard match what we want:
- we always intend to support the latest released standards
- we want to test that in CI (we do this by manually setting CXXSTD to C++20)
- we also want to test in CI the oldest supported standard (currently C++17)
- we want all users to build with the latest released standard available on their compiler because of features like consteval bringing potential runtime benefits to newer standards
- we want users to be capable of externally overriding the actual, exact standard that yosys will be built with
With CMake we can get this done with
CMAKE_CXX_KNOWN_FEATURES and avoiding overriding command-line set values for CMAKE_CXX_STANDARD
and CMAKE_CXX_STANDARD_REQUIRED
If you would prefer this out of scope for this work due to time constraints feel free to turn this into a followup issue though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just finished and commited it: ad800f7
Now what's used to set the CXX standard as well as an override option:
get_property(cxx_features GLOBAL PROPERTY CMAKE_CXX_KNOWN_FEATURES)
if(NOT CMAKE_CXX_STANDARD)
if("cxx_std_20" IN_LIST cxx_features)
message(STATUS "C++20 support enabled")
set(CMAKE_CXX_STANDARD 20 CACHE STRING "C++ standard to use for the build")
elseif("cxx_std_17" IN_LIST cxx_features)
message(STATUS "C++17 support enabled")
set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to use for the build")
elseif("cxx_std_14" IN_LIST cxx_features)
message(STATUS "C++14 support enabled")
set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to use for the build")
elseif("cxx_std_11" IN_LIST cxx_features)
message(STATUS "C++11 support enabled")
set(CMAKE_CXX_STANDARD 11 CACHE STRING "C++ standard to use for the build")
else()
message(STATUS "C++98 support enabled")
set(CMAKE_CXX_STANDARD 98 CACHE STRING "C++ standard to use for the build")
endif()
else()
message(STATUS "C++ standard set to ${CMAKE_CXX_STANDARD}")
endif()
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED ON)
set(CMAKE_C_EXTENSIONS ON)
Presently it only controls the CXX standard but if there were other features that were to be included, those should hopefully be a simple addition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think an alternate solution is that if it's unset, set CMAKE_CXX_STANDARD
to 23, and if it's already set, set CMAKE_CXX_STANDARD_REQUIRED
on. Then also error out if the C++17 feature is off. This provides easy overriding, ensures C++17 is supported (I believe the build will fail if not), but still lets.
Why 23? Beacuse we want to support building on "systems kinda like the oldest ubuntu LTS with standard support". That means ubuntu 22.04, which has CMake 3.22. That version of CMake only knows about C++23 and not about C++26. And it can't figure out that 26>23 and do a reasonable soft fallback. We should correspondingly set the required CMake version to 3.22 (it's from 2021, shouldn't annoy too many people).
Alternatively I guess you can just add std_23 and std_26 to your snippet as well and it should work even on older versions. I dislike both methods about the same. I'm a bit disappointed in CMake, not being able to work at all with not-yet-known year-named standards sucks, and the 98<17<98 problem wouldn't manifest until 2098 (or 2090 for C). We'll probably be back to stone tools by then at this rate anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think an alternate solution is that if it's unset, set CMAKE_CXX_STANDARD to 23, and if it's already set, set CMAKE_CXX_STANDARD_REQUIRED on. Then also error out if the C++17 feature is off. This provides easy overriding, ensures C++17 is supported (I believe the build will fail if not), but still lets.
If I understood what you were asking for, this should be the changes to use your above suggestion: 030b054
CMakeLists.txt
Outdated
target_link_libraries(yosys_exe PUBLIC yosys) | ||
set_target_properties(yosys_exe PROPERTIES OUTPUT_NAME yosys) | ||
install(TARGETS yosys_exe | ||
RUNTIME DESTINATION .) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case we definitely don't want to retain the current behavior. A proper isolated build
directory is what we want, consistent with the rest of the CMake world. I'm sure everybody would prefer for this project to no longer puke out files directly into the tree and its root
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very much agree, I was unsure about how much deviation with the existing implementation is acceptable.
If some amount is ok, then not only do I think that it is a good idea to standardize the install folder, but it would be quite nice to have the project separated into public & private headers & source files in a more standard structure:
e.g.
.
├── CMakeLists.txt
├── include
│ ├── CMakeLists.txt
│ └── public.hpp
└── src
├── CMakeLists.txt
├── private.cpp
└── private.hpp
And separating the class definitions, and class implementations as well would be quite nice and would bring yosys more in line with other CPP projects
I recognize some parts of that may not be feasible, but it would certainly be easier to read and easier for others to use and interact with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not yet modified the tests to work from the build root as opposed to the source root, but 8051724 has the initial changes to get the build folder working properly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nobody likes our Makefile and nobody likes our testing infrastructure, if you have ideas improvements that are ergonomic to carry out as part of the cmake rewrite, we'd be happy to accept them. However, we don't want to break header include paths in external projects. Could be a limitation, but maybe not necessarily
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would definitely be interested in helping out there. I'm interested in integrating yosys as a library into another project and a static lib / public interface headers would make life way simpler.
I believe that there should be a way at the time of install to place all the same headers in the same hierarchy as they are in the source tree to not break compatibility with external projects.
For the moment, would it be fair to say that the "add_include_files" calls across cmake constitute the public interface, and all other logic is effectively private?
add_include_file("kernel" "kernel/binding.h")
add_include_file("kernel" "kernel/bitpattern.h")
add_include_file("kernel" "kernel/cellaigs.h")
add_include_file("kernel" "kernel/celledges.h")
add_include_file("kernel" "kernel/celltypes.h")
add_include_file("kernel" "kernel/consteval.h")
add_include_file("kernel" "kernel/constids.inc")
add_include_file("kernel" "kernel/cost.h")
add_include_file("kernel" "kernel/drivertools.h")
add_include_file("kernel" "kernel/ff.h")
add_include_file("kernel" "kernel/ffinit.h")
add_include_file("kernel" "kernel/ffmerge.h")
add_include_file("kernel" "kernel/fmt.h")
add_include_file("kernel" "kernel/hashlib.h")
add_include_file("kernel" "kernel/json.h")
add_include_file("kernel" "kernel/log.h")
add_include_file("kernel" "kernel/macc.h")
add_include_file("kernel" "kernel/modtools.h")
add_include_file("kernel" "kernel/mem.h")
add_include_file("kernel" "kernel/qcsat.h")
add_include_file("kernel" "kernel/register.h")
add_include_file("kernel" "kernel/rtlil.h")
add_include_file("kernel" "kernel/satgen.h")
add_include_file("kernel" "kernel/scopeinfo.h")
add_include_file("kernel" "kernel/sexpr.h")
add_include_file("kernel" "kernel/sigtools.h")
add_include_file("kernel" "kernel/timinginfo.h")
add_include_file("kernel" "kernel/utils.h")
add_include_file("kernel" "kernel/yosys.h")
add_include_file("kernel" "kernel/yosys_common.h")
add_include_file("kernel" "kernel/yw.h")
add_include_file("libs/ezsat" "libs/ezsat/ezsat.h")
add_include_file("libs/ezsat" "libs/ezsat/ezminisat.h")
add_include_file("libs/sha1" "libs/sha1/sha1.h")
add_include_file("libs/json11" "libs/json11/json11.hpp")
add_include_file("passes/fsm" "passes/fsm/fsmdata.h")
add_include_file("frontends/ast" "frontends/ast/ast.h")
add_include_file("frontends/ast" "frontends/ast/ast_binding.h")
add_include_file("frontends/blif" "frontends/blif/blifparse.h")
add_include_file("backends/rtlil" "backends/rtlil/rtlil_backend.h")
if (ENABLE_ZLIB)
add_include_file("kernel" "kernel/fstdata.h")
add_include_file("libs/fst" "libs/fst/fstapi.h")
endif()
And some others scattered across other cmake files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the rest of the headers aren't installed and thus aren't available to external projects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates on a refactor, I spent some time turning kernel into an isolated static library as well as did some reorganizing to make it a bit easier to follow: link. I tried to get it to a point where if another library needed to use kernel, it could just link it in, and have no other config to use the kernel.
The problem I've ran into and I'm quite stuck on, is that when another library needs to use the kernel, the other library isn't properly registering commands:
flex_target(RTLIL_LEXER "rtlil_lexer.l" "${CMAKE_CURRENT_BINARY_DIR}/rtlil_lexer.cc")
bison_target(RTLIL_PARSER "rtlil_parser.y" "${CMAKE_CURRENT_BINARY_DIR}/rtlil_parser.tab.cc" DEFINES_FILE "${CMAKE_CURRENT_BINARY_DIR}/rtlil_parser.tab.hh" COMPILE_FLAGS "-d -r all")
add_library(yosys_frontends_rtlil STATIC
rtlil_frontend.cc
)
add_library(yosys_frontends_rtlil_gen STATIC
${FLEX_RTLIL_LEXER_OUTPUTS}
${BISON_RTLIL_PARSER_OUTPUTS}
)
target_link_libraries(yosys_obj PUBLIC yosys_frontends_rtlil yosys_frontends_rtlil_gen)
target_link_libraries(yosys_frontends_rtlil_gen PUBLIC yosys_kernel)
target_link_libraries(yosys_frontends_rtlil PUBLIC yosys_kernel)
nick@DESKTOP-OJ2V1NK:/mnt/c/Users/Nick/Projects/CPP/yosys$ ./build/yosys -p "rtlil"
/----------------------------------------------------------------------------\
| yosys -- Yosys Open SYnthesis Suite |
| Copyright (C) 2012 - 2025 Claire Xenia Wolf <[email protected]> |
| Distributed under an ISC-like license, type "license" to see terms |
\----------------------------------------------------------------------------/
Yosys 0.50+1 (git sha1 0119e6b15, GNU 14.3.0)
-- Running command `rtlil' --
ERROR: No such command: rtlil (type 'help' for a command overview)
I have a suspicion that linking it statically creates a fully different translation unit between the libraries so it doesn't register in the same kernel. That being said, I'm not sure how to verify this or resolve it.
This is a lot closer to the proper way to use cmake IMO and would be really nice to get it working properly but I'm not quite sure how to make it work. Perhaps a shared library would work, but that would need some refactoring to deal with the circular dependencies between kernel & some of the other libs:
CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
"yosys_kernel" of type SHARED_LIBRARY
depends on "yosys_frontends_ast" (weak)
depends on "yosys_passes_cmds" (weak)
depends on "yosys_backends_verilog" (weak)
depends on "yosys_passes_techmap" (weak)
depends on "yosys_frontends_rtlil" (weak)
depends on "yosys_frontends_rtlil_gen" (weak)
depends on "yosys_frontends_verilog" (weak)
depends on "yosys_backends_rtlil" (weak)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't fully read this, but rtlil
has never been a command, try read_rtlil
or write_rtlil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, this may be a better example:
yosys> help
anlogic_eqn Anlogic: Calculate equations for luts
anlogic_fixcarry Anlogic: fix carry chain
assertpmux adds asserts for parallel muxes
async2sync convert async FF inputs to sync circuits
cd a shortcut for 'select -module <name>'
clean remove unused cells and wires
clk2fflogic convert clocked FFs to generic $ff cells
connect_rpc connect to RPC frontend
coolrunner2_fixup insert necessary buffer cells for CoolRunner-II architecture
coolrunner2_sop break $sop cells into ANDTERM/ORTERM cells
cutpoint adds formal cut points to the design
design save, restore and reset current design
dump print parts of the design in RTLIL format
echo turning echoing back of commands on and off
efinix_fixcarry Efinix: fix carry chain
equiv_add add a $equiv cell
equiv_induct proving $equiv cells using temporal induction
equiv_make prepare a circuit for equivalence checking
equiv_mark mark equivalence checking regions
equiv_miter extract miter from equiv circuit
equiv_opt prove equivalence for optimized circuit
equiv_purge purge equivalence checking module
equiv_remove remove $equiv cells
equiv_simple try proving simple $equiv instances
equiv_status print status of equivalent checking module
equiv_struct structural equivalence checking
eval evaluate the circuit given an input
expose convert internal signals to module ports
fmcombine combine two instances of a cell into one
fminit set init values/sequences for formal
formalff prepare FFs for formal
freduce perform functional reduction
fsm extract and optimize finite state machines
fsm_detect finding FSMs in design
fsm_expand expand FSM cells by merging logic into it
fsm_export exporting FSMs to KISS2 files
fsm_extract extracting FSMs in design
fsm_info print information on finite state machines
fsm_map mapping FSMs to basic logic
fsm_opt optimize finite state machines
fsm_recode recoding finite state machines
fst2tb generate testbench out of fst file
gatemate_foldinv fold inverters into Gatemate LUT trees
greenpak4_dffinv merge greenpak4 inverters and DFF/latches
help display help messages
hierarchy check, expand and clean up design hierarchy
history show last interactive commands
ice40_braminit iCE40: perform SB_RAM40_4K initialization from file
ice40_dsp iCE40: map multipliers
ice40_opt iCE40: perform simple optimizations
ice40_wrapcarry iCE40: wrap carries
jny write design and metadata
json write design in JSON format
keep_hierarchy selectively add the keep_hierarchy attribute
lattice_gsr Lattice: handle GSR
license print license terms
ls list modules or objects in modules
memory translate memories to basic cells
memory_bmux2rom convert muxes to ROMs
memory_bram map memories to block rams
memory_collect creating multi-port memory cells
memory_dff merge input/output DFFs into memory read ports
memory_libmap map memories to cells
memory_map translate multiport memories to basic cells
memory_memx emulate vlog sim behavior for mem ports
memory_narrow split up wide memory ports
memory_nordff extract read port FFs from memories
memory_share consolidate memory ports
memory_unpack unpack multi-port memory cells
microchip_dffopt MICROCHIP: optimize FF control signal usage
microchip_dsp MICROCHIP: pack resources into DSPs
miter automatically create a miter circuit
mutate generate or apply design mutations
muxpack $mux/$pmux cascades to $pmux
nx_carry NanoXplore: create carry cells
onehot optimize $eq cells for onehot signals
opt perform simple optimizations
opt_clean remove unused cells and wires
opt_demorgan Optimize reductions with DeMorgan equivalents
opt_dff perform DFF optimizations
opt_expr perform const folding and simple expression rewriting
opt_ffinv push inverters through FFs
opt_lut optimize LUT cells
opt_lut_ins discard unused LUT inputs
opt_mem optimize memories
opt_mem_feedback convert memory read-to-write port feedback paths to write enables
opt_mem_priority remove priority relations between write ports that can never collide
opt_mem_widen optimize memories where all ports are wide
opt_merge consolidate identical cells
opt_muxtree eliminate dead trees in multiplexer trees
opt_reduce simplify large MUXes and AND/OR gates
opt_share merge mutually exclusive cells of the same type that share an input signal
peepopt collection of peephole optimizers
plugin load and list loaded plugins
pmux2shiftx transform $pmux cells to $shiftx cells
proc translate processes to netlists
proc_arst detect asynchronous resets
proc_clean remove empty parts of processes
proc_dff extract flip-flops from processes
proc_dlatch extract latches from processes
proc_init convert initial block to init attributes
proc_memwr extract memory writes from processes
proc_mux convert decision trees to multiplexers
proc_prune remove redundant assignments
proc_rmdead eliminate dead trees in decision trees
proc_rom convert switches to ROMs
qbfsat solve a 2QBF-SAT problem in the circuit
ql_bram_merge Infers QuickLogic k6n10f BRAM pairs that can operate independently
ql_bram_types Change TDP36K type to subtypes
ql_dsp_io_regs change types of QL_DSP2 depending on configuration
ql_dsp_macc infer QuickLogic multiplier-accumulator DSP cells
ql_dsp_simd merge QuickLogic K6N10f DSP pairs to operate in SIMD mode
ql_ioff Infer I/O FFs for qlf_k6n10f architecture
read load HDL designs
read_aiger read AIGER file
read_blif read BLIF file
read_json read JSON file
read_liberty read cells from liberty file
read_verilog read modules from Verilog file
read_verilog_file_list parse a Verilog file list
read_xaiger2 (experimental) read XAIGER file
recover_names Execute a lossy mapping command and recover original netnames
rmports remove module ports with no connections
sat solve a SAT problem in the circuit
script execute commands from file or wire
select modify and view the list of selected objects
share perform sat-based resource sharing
shell enter interactive command mode
sim simulate the circuit
simplemap mapping simple coarse-grain cells
submod moving part of a module to a new submodule
supercover add hi/lo cover cells for each wire bit
synth_achronix synthesis for Achronix Speedster22i FPGAs.
synth_anlogic synthesis for Anlogic FPGAs
synth_coolrunner2 synthesis for Xilinx Coolrunner-II CPLDs
synth_easic synthesis for eASIC platform
synth_ecp5 synthesis for ECP5 FPGAs
synth_efinix synthesis for Efinix FPGAs
synth_fabulous FABulous synthesis script
synth_gatemate synthesis for Cologne Chip GateMate FPGAs
synth_gowin synthesis for Gowin FPGAs
synth_greenpak4 synthesis for GreenPAK4 FPGAs
synth_ice40 synthesis for iCE40 FPGAs
synth_intel synthesis for Intel (Altera) FPGAs.
synth_intel_alm synthesis for ALM-based Intel (Altera) FPGAs.
synth_lattice synthesis for Lattice FPGAs
synth_microchip synthesis for Microchip FPGAs
synth_nanoxplore synthesis for NanoXplore FPGAs
synth_nexus synthesis for Lattice Nexus FPGAs
synth_quicklogic Synthesis for QuickLogic FPGAs
synth_sf2 synthesis for SmartFusion2 and IGLOO2 FPGAs
synth_xilinx synthesis for Xilinx FPGAs
synthprop synthesize SVA properties
tcl execute a TCL script file
test_abcloop automatically test handling of loops in abc command
test_autotb generate simple test benches
test_cell automatically test the implementation of a cell type
test_generic test the generic compute graph
test_pmgen test pass for pmgen
uniquify create unique copies of modules
verific load Verilog and VHDL designs using Verific
verilog_defaults set default options for read_verilog
verilog_defines define and undefine verilog defines
wreduce reduce the word size of operations if possible
write_aiger write design to AIGER file
write_aiger2 (experimental) write design to AIGER file
write_blif write design to BLIF file
write_btor write design to BTOR file
write_cxxrtl convert design to C++ RTL simulation
write_edif write design to EDIF netlist file
write_firrtl write design to a FIRRTL file
write_functional_cxx convert design to C++ using the functional backend
write_functional_rosette Generate Rosette compatible Racket from Functional IR
write_functional_smt2 Generate SMT-LIB from Functional IR
write_intersynth write design to InterSynth netlist file
write_jny generate design metadata
write_json write design to a JSON file
write_rtlil write design to RTLIL file
write_simplec convert design to simple C code
write_smt2 write design to SMT-LIBv2 file
write_smv write design to SMV file
write_spice write design to SPICE netlist file
write_table write design as connectivity table
write_xaiger write design to XAIGER file
write_xaiger2 (experimental) write module to XAIGER file
xilinx_dffopt Xilinx: optimize FF control signal usage
xilinx_dsp Xilinx: pack resources into DSPs
xilinx_srl Xilinx shift register extraction
Everything that was statically linked into kernel (& vice versa) seems to have not registered, no rtlil commands at all, no extract pass, and quite a few others are missing
install(TARGETS yosys_exe | ||
RUNTIME DESTINATION .) | ||
|
||
#### yosys-config setup #### |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this section to be split off into a separate file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also Done: ad800f7
CMakeLists.txt
Outdated
#target_include_directories(yosys PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) | ||
#target_include_directories(yosys PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) | ||
#target_compile_definitions(yosys PRIVATE _YOSYS_) | ||
add_library(yosys OBJECT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With cmake -G Ninja
the build.ninja
file ends up with two targets named yosys
which errors out, I think you have to rename the object one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 8051724, however the shared lib needed to be renamed as well to get Ninja to work
|
||
install(PROGRAMS ${yosys_BINARY_DIR}/yosys-config DESTINATION .) | ||
|
||
#### TESTING #### |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow, the tests run 2.6x longer on my 24 threads than with make, and I saw a test fail indeterministically. I'll see what I can find out on my end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, there was a top level GNU Make running the tests so these make invocations connected to its job server and shared a pool of job threads. Now each make gets only one thread, maybe we could work around that for make, but Ninja doesn't provide a job server (ninja-build/ninja#1139). I think we should rewrite tests/gen-tests-makefile.sh
to emit cmake files that call add_test
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the issue you linked indicates that the next Ninja release should include a jobserver client, and I believe there's a jobserver server being discussed. So perhaps continuing to use Ninja will work?
After a good chunk of time, I don't think it is possible to do much refactoring and separating out sub components into their own libraries without breaking things, or at least I don't know how to fix the things being broken. I got kernel into it's own static (and shared) library, but either yosys would only register passes in the kernel library, or it would crash from every library getting registered twice. Except for #5204 (comment), this is probably the best & most organized the CMake files can get at the moment without a semi significant refactor |
One more update here, in regards to why the static library was not working. (It still isn't working but if it was ever wanted in the future perhaps this will prove helpful). It seems to me that only symbols that are marked extern are able to be used properly when linked into another binary: Shows all symbols:
Shows only extern symbols:
Actual linked output of help pass:
If there was ever any interest, this is the small bit I wrote up to use the linked yosys lib: https://github.com/nickrallison/ryosys |
Yeah, I'm still interested in picking this up later and see where I can get it. We have a couple changes planned that touch the build system so I'd prefer we build them on top of CMake |
What are the reasons/motivation for this change?
Got tests working with CMake after rebasing from main
Explain how this is achieved.
see lines 293-305 of CMakeLists.txt:
If applicable, please suggest to reviewers how they can test the change.
This is a cherry-pick of the changes after merging main into this branch, merge this commit id into the changes to test: 7b0c1fe
Then see build.sh, this file can likely be removed / placed elsewhere but it seemed reasonable to provide the build sequence used