Before this patch, the code passed around std::string objects by
value. It's probably not a hot-spot, but it can't hurt to avoid the
copying.
Removing the copy and clean-up code means the resulting code is ~6.1kb
smaller when compiled with GCC 9.3 and standard settings.
The existing code does a search to figure out whether id is in the
dict (with the call to count()), and then looks it up again to get the
result (with the call to at()). This version calls find() instead,
avoiding the double lookup.
Code size increases slightly (6kb). I think this is because the
contents of find() are getting inlined, and then inlined into lots of
the callsites for cell() and wire().
Looking at the compiled code before this patch, you just get
a (non-inlined) call to count() followed by a call to at(). After the
patch, the contents of find() have been inlined (so you see do_hash,
then do_lookup). The result for each function is about 30 bytes / 40%
bigger, which presumably also enlarges call-sites that inline it.
There was a handwritten copy constructor, which I'm not sure was
actually legal C++ (it unconditionally read from the 'data' member of
a union, which wouldn't have been written if wire was true). It was
also a bit less efficient than the constructor you get from the
compiler by default (which is allowed to just copy the memory).
This gives a marginal (~0.25%) decrease in code size when compiled
with GCC 9.3.
These operators work by fetching the string from the global string
table and then comparing with the std::string that was passed in as
rhs.
Using str() means that we create a std::string (strlen; malloc;
memcpy), compare for equality (another memcmp if they have the same
length) and then finally free the string.
Using c_str() means that we pass the const char* straight to
std::string's equality operator. This ends up as a call to
std::string::compare (the const char* flavour), which is essentially
strcmp.
The workflow of debugging fatal pass errors in Yosys is flawed in
three ways:
1. Running Yosys under a debugger is sufficient for the debugger
to catch some fatal errors (segfaults, aborts, STL exceptions)
but not others (`log_error()`, `log_cmd_error()`). This is
neither obvious nor easy to remember.
2. To catch Yosys-specific fatal errors, it is necessary to set
a breakpoint at `logv_error_with_prefix()`, or at least,
`logv_error()`. This is neither obvious nor easy to remember,
and GDB's autocomplete takes many seconds to suggest function
names due to the large amount of symbols in Yosys.
3. If a breakpoint is not set and Yosys encounters with such
a fatal error, the process terminates. When debugging a crash
that takes a long time to reproduce (or a nondeterministic crash)
this can waste a significant amount of time.
To solve this problem, add a macro `YS_DEBUGTRAP` that acts as a hard
breakpoint (if available), and a macro `YS_DEBUGTRAP_IF_DEBUGGING`
that acts as a hard breakpoint only if debugger is present.
Then, use `YS_DEBUGTRAP_IF_DEBUGGING` in `logv_error_with_prefix()`
to obviate the need for a breakpoint on nearly every platform.
Co-Authored-By: Alberto Gonzalez <boqwxp@airmail.cc>
This includes the following significant changes:
* Patching ezsat and minisat to disable resource limiting code
on WASM/WASI, since the POSIX functions they use are unavailable.
* Adding a new definition, YOSYS_DISABLE_SPAWN, present if platform
does not support spawning subprocesses (i.e. Emscripten or WASI).
This definition hides the definition of `run_command()`.
* Adding a new Makefile flag, DISABLE_SPAWN, present in the same
condition. This flag disables all passes that require spawning
subprocesses for their function.
As per suggestion made in https://github.com/YosysHQ/yosys/pull/1987, now:
RTLIL::wire holds an is_signed field.
This is exported in JSON backend
This is exported via dump_rtlil command
This is read in via ilang_parser
A few passes included the same list of FF cell types. Make it a global
const instead.
The zinit pass also seems to include a list like that, but given that
it seems to be completely broken at the time (see #1568 discussion),
I'm going to pretend I didn't see that.
This patch should support things like
`define foo(a, b = 3, c) a+b+c
`foo(1, ,2)
which will evaluate to 1+3+2. It also spots mistakes like
`foo(1)
(the 3rd argument doesn't have a default value, so a call site is
required to set it).
Most of the patch is a simple parser for the format in preproc.cc, but
I've also taken the opportunity to wrap up the "name -> definition"
map in a type, rather than use multiple std::map's.
Since this type needs to be visible to code that touches defines, I've
pulled it (and the frontend_verilog_preproc declaration) out into a
new file at frontends/verilog/preproc.h and included that where
necessary.
Finally, the patch adds a few tests in tests/various to check that we
are parsing everything correctly.
whitequark
Rupert Swarbrick
Miodrag Milanović
N. Engelhardt
Alberto Gonzalez
Martin
Marcelina Kościelnicka
Eddie Hung
Peter Crozier
Claire Wolf
Claire Wolf
Xiretza
Vamsi K Vytla
Rupert Swarbrick
Waldir Pimenta
jiegec
David Shah