This change set contains a number of bug fixes and improvements related to
scoping and resolution in generate and procedural blocks. While many of the
frontend changes are interdependent, it may be possible bring the techmap
changes in under a separate PR.
Declarations within unnamed generate blocks previously encountered issues
because the data declarations were left un-prefixed, breaking proper scoping.
The LRM outlines behavior for generating names for unnamed generate blocks. The
original goal was to add this implicit labelling, but doing so exposed a number
of issues downstream. Additional testing highlighted other closely related scope
resolution issues, which have been fixed. This change also adds support for
block item declarations within unnamed blocks in SystemVerilog mode.
1. Unlabled generate blocks are now implicitly named according to the LRM in
`label_genblks`, which is invoked at the beginning of module elaboration
2. The Verilog parser no longer wraps explicitly named generate blocks in a
synthetic unnamed generate block to avoid creating extra hierarchy levels
where they should not exist
3. The techmap phase now allows special control identifiers to be used outside
of the topmost scope, which is necessary because such wires and cells often
appear in unlabeled generate blocks, which now prefix the declarations within
4. Some techlibs required modifications because they relied on the previous
invalid scope resolution behavior
5. `expand_genblock` has been simplified, now only expanding the outermost
scope, completely deferring the inspection and elaboration of nested scopes;
names are now resolved by looking in the innermost scope and stepping outward
6. Loop variables now always become localparams during unrolling, allowing them
to be resolved and shadowed like any other identifier
7. Identifiers in synthetic function call scopes are now prefixed and resolved
in largely the same manner as other blocks
before: `$func$\func_01$tests/simple/scopes.blk.v:60$5$\blk\x`
after: `\func_01$func$tests/simple/scopes.v:60$5.blk.x`
8. Support identifiers referencing a local generate scope nested more
than 1 level deep, i.e. `B.C.x` while within generate scope `A`, or using a
prefix of a current or parent scope, i.e. `B.C.D.x` while in `A.B`, `A.B.C`,
or `A.B.C.D`
9. Variables can now be declared within unnamed blocks in SystemVerilog mode
Addresses the following issues: 656, 2423, 2493
The only difference between "RTLIL" and "ILANG" is that the latter is
the text representation of the former, as opposed to the in-memory
graph representation. This distinction serves no purpose but confuses
people: it is not obvious that the ILANG backend writes RTLIL graphs.
Passes `write_ilang` and `read_ilang` are provided as aliases to
`write_rtlil` and `read_rtlil` for compatibility.
This parameter will resolve to the name of the cell being mapped. The
first user of this parameter will be synth_intel_alm's Quartus output,
which requires a unique (and preferably descriptive) name passed as
a cell parameter for the memory cells.
The new types include:
- FFs with async reset and enable (`$adffe`, `$_DFFE_[NP][NP][01][NP]_`)
- FFs with sync reset (`$sdff`, `$_SDFF_[NP][NP][01]_`)
- FFs with sync reset and enable, reset priority (`$sdffs`, `$_SDFFE_[NP][NP][01][NP]_`)
- FFs with sync reset and enable, enable priority (`$sdffce`, `$_SDFFCE_[NP][NP][01][NP]_`)
- FFs with async reset, set, and enable (`$dffsre`, `$_DFFSRE_[NP][NP][NP][NP]_`)
- latches with reset or set (`$adlatch`, `$_DLATCH_[NP][NP][01]_`)
The new FF types are not actually used anywhere yet (this is left
for future commits).
Previously this was tagged only with YS_ATTRIBUTE(noreturn), but not
YS_NORETURN, so it got lost in #2173, resulting in warnings in
frontends/ast/simplify.cc:
frontends/ast/simplify.cc:267:1: warning: function declared 'noreturn' should not return [-Winvalid-noreturn]
}
^
frontends/ast/simplify.cc:379:1: warning: function declared 'noreturn' should not return [-Winvalid-noreturn]
}
^
Upgrading to WASI SDK 11.0 caused the WASM build to fail because WASM
does not have signals. (Arguably Yosys was broken even before, it was
just broken silently.)
The $div and $mod cells use truncating division semantics (rounding
towards 0), as defined by e.g. Verilog. Another rounding mode, flooring
(rounding towards negative infinity), can be used in e.g. VHDL. The
new $divfloor cell provides this flooring division.
This commit also fixes the handling of $div in opt_expr, which was
previously optimized as if it was $divfloor.
The $div and $mod cells use truncating division semantics (rounding
towards 0), as defined by e.g. Verilog. Another rounding mode, flooring
(rounding towards negative infinity), can be used in e.g. VHDL. The
new $modfloor cell provides this flooring modulo (also known as "remainder"
in several languages, but this name is ambiguous).
This commit also fixes the handling of $mod in opt_expr, which was
previously optimized as if it was $modfloor.
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.
Robert Baruch
Zachary Snow
umarcor
whitequark
Marcelina Kościelnicka
nitz
Miodrag Milanovic
N. Engelhardt
Xiretza
clairexen
Alberto Gonzalez
Anonymous Maarten
Rupert Swarbrick
Martin
Eddie Hung
Peter Crozier
Claire Wolf
Vamsi K Vytla
Rupert Swarbrick