Internals: Refactor AstNodeBlock representation (#6280)
AstNodeBlock now has 2 child lists: 'declsp' to hold declarations within
the block, and 'stmtsp' to hold the procedural statements.
AstBegin is then just a simple subtype of AstNodeBlock.
AstFork is a proper superset of AstNodeBlock (and also AstBegin), and
adds 'forksp' which hold the parallel statements. Having the sequential
'stmtsp' in AstFork is required to properly implement variable
initializers in fork blocks (IEEE 1800-2023 9.3.2), this makes that
clear, while also separating the non AstNodeStmt declarations
(for #6280). The actual fork branches in 'AstFork::forkps()' are all
AstBegin nodes. This is required as lowering stages will introduce
additional statements in each parallel branch. (We used to wrap AstFork
statements into AstBegin in 3 different places, now they always are
AstBegin and this is enforced via the type checker/V3Broken).
Also fixes incorrect disabling of forked processes from within the `fork`.
- Delete 'finalsp'. It was used in one place, basically unnecessary and
safe to remove.
- Make 'argsp' a 'List[AstVar]'. This held before. It holds the function
argument and return variables.
- Replace 'intitsp' with 'varsp' and make it into 'List[AstVar]' to hold
the function local variables. This was most of its use before. The few
places we inserted statements here now moved into 'stmtsp' by
inserting at the front of the list.
Remove the large variety of ways raw "text" is represented in the Ast.
Particularly, the only thing that represents a string to be emitted in
the output is AstText.
There are 5 AstNodes that can contain AstText, and V3Emit will throw an
error if an AstText is encountered anywhere else:
- AstCStmt: Internally generated procedural statements involving raw
text.
- AstCStmtUser: This is the old AstUCStmt, renamed so it sorts next to
AstCStmt, as it's largely equivalent. We should never create this
internally unless used to represent user input. It is used for $c,
statements in the input, and for some 'systemc_* blocks.
- AstCExpr: Internally generaged expression involving raw text.
- AstCExprUser: This is the old AstUCFunc, renamed so it sorts next to
AstCExpr. It is largely equivalent, but also has more optimizations
disabled. This should never be created internally, it is only used for
$c expressions in the input.
- AstTextBlock: Use by V3ProtectLib only, to generate the hierarchical
wrappers.
Text "tracking" for indentation is always on for AstCStmt, AstCExpr, and
AstTextBlock, as these are always generated by us, and should always be
well formed.
Tracking is always off for AstCStmtUser and AstCExprUser, as these
contain arbitrary user input that might not be safe to parse for
indentation.
Remove subsequently redundant AstNodeSimpleText and AstNodeText types.
This patch also fixes incorrect indentation in emitted waveform tracing
functions, and makes the output more readable for hier block SV stubs.
With that, all raw text nodes are handled as a proper AstNodeStmt or
AstNodeExpr as required for #6280.
There was exactly one place in V3Task, handling DPI arguments when we
relied on cleanOut of AstCExpr being false for masking. Made that code
do the relevant masking via a few new run-time functions, which also
eliminates some special cases in the relevant V3Task functions.
Initial idea was to remodel AssignW as Assign under Alway. Trying that
uncovered some issues, the most difficult of them was that a delay
attached to a continuous assignment behaves differently from a delay
attached to a blocking assignment statement, so we need to keep the
knowledge of which flavour an assignment was until V3Timing.
So instead of removing AstAssignW, we always wrap it in an AstAlways,
with a special `keyword()` type. This makes it into a proper procedural
statement, which is almost equivalent to AstAssign, except for the case
when they contain a delay. We still gain the benefits of #6280 and can
simplify some code. Every AstNodeStmt should now be under an
AstNodeProcedure - which we should rename to AstProcess, or an
AstNodeFTask). As a result, V3Table can now handle AssignW for free.
Also uncovered and fixed a bug in handling intra-assignment delays if
a function is present on the RHS of an AssignW.
There is more work to be done towards #6280, and potentially simplifying
AssignW handing, but this is the minimal change required to tick it off
the TODO list for #6280.
I stared this because the emitted makefiles for hierarchical verilation
were non-deterministic (iterating unordered_map indexed by pointers).
Then I realized that the V3HierPlan is just a dependency graph encoded
in a slightly idiosyncratic way. We do have a data structure to use for
that instead.
With that the output should always be deterministic + have nicer dumps.
All code is built as C++ via CXX, but we still have some references to
CC. Trying to make sure we don't add plain C later by hiding the C
compiler. (So it's always enough to override CXX=... in configure)
This patch gets rid of over 80% of temporary dynamic memory allocations
(when a malloced node is immediately freed with no other malloc in
between). It also gets rid of over 20% of all calls to malloc.
It's worth ~3% average verilation speed up with tcmalloc, and more
without tcmalloc.
This patch implements #6480. All loop statements are represented using
AstLoop and AstLoopTest.
This necessitates rework of the loop unroller to handle loops of
arbitrary form. To enable this, I have split the old unroller used for
'generate for' statements and moved it into V3Param, and subsequently
rewrote V3Unroll to handle the new representation. V3Unroll can now
unroll more complex loops, including with loop conditions containing
multiple variable references or inlined functions.
Handling the more generic code also requires some restrictions. If a
loop contains any of the following, it cannot be unrolled:
- A timing control that might suspend the loop
- A non-inlined call to a non-pure function
These constructs can change the values of variables in the loop, so are
generally not safe to unroll if they are present. (We could still unroll
if all the variables needed for unrolling are automatic, however we
don't do that right now.)
These restrictions seem ok in the benchmark suite, where the new
unroller can generally unroll many more loops than before.
Internals: Refactor generate construct Ast handling (#6280)
We introduce AstNodeGen, the common base class of AstGenBlock,
AstGenCase, AstGenFor, and AstGenIf, which together represent all SV
generate constructs. Subsequently remove AstNodeFor, AstNodeCase
(AstCase is now directly derived from AstNodeStmt) and adjust internals
to work on the new representation.
Output is identical modulo hashes do to changed AstNode type ids, no
functional change intended.
Step towards #6280.
Rename AstAssignAlias to AstAlias and make it derive from AstNode
instead of AstNodeStmt.
Replace AstAlias with AstAssignW in V3LinkDot::linkDotScope, which is
the last place we need to be aware of the alias construct. Using
AstAssignW dowstream enables further optimization while preserving the
same functionality.
The only use for the clocker attribute and the AstVar::isUsedClock that
is actually necessary today for correctness is to mark top level inputs
of --lib-create blocks as being (or driving) a clock signal. Correctness
of --lib-create (and hence hierarchical blocks) actually used to depend
on having the right optimizations eliminate intermediate clocks (e.g.:
V3Gate), when the top level port was not used directly in a sensitivity
list, or marking top level signals manually via --clk or the clocker
attribute. However V3Sched::partition already needs to trace through the
logic to figure out what signals might drive a sensitivity list, so it
can very easily mark all top level inputs as such.
In this patch we remove the AstVar::attrClocker and AstVar::isUsedClock
attributes, and replace them with AstVar::isPrimaryClock, automatically
set by V3Sched::partition. This eliminates all need for manual
annotation so we are deprecating the --clk/--no-clk options and the
clocker/no_clocker attributes.
This also eliminates the opportunity for any further mis-optimization
similar to #6453.
Regarding the other uses of the removed AstVar attributes:
- As of 5.000, initial edges are triggered via a separate mechanism
applied in V3Sched, so the use in V3EmitCFunc.cpp is redundant
- Also as of 5.000, we can handle arbitrary sensitivity expressions, so
the restriction on eliminating clock signals in V3Gate is unnecessary
- Since the recent change when Dfg is applied after V3Scope, it does
perform the equivalent of GateClkDecomp, so we can delete that pass.
These are no longer required for correct scheduling. They are still
accepted for backward compatibility, but have no effect on simulation
and are dropped in the front-end. Also removed the then redundant
AstAlwaysPublic class.
Fixes#6442
Added a lock around the table used to detect memory leaks. Note the only
part that is multi-threaded at the moment is V3Emit where we create
AstCFiles and the like, so the lock should be almost always uncontested.
This code is not thread safe. Specifically AstNode constructors are not
thread safe, as they may create entries in the shared Dtype table via
which can be racy.
Replace std::exit with v3Global.exit, and make V3Error::vlAbort call
v3Global.shutdown. This gives us an opportunity to release resources to
facilitate leak checking even when exiting early on an error.
Note we still don't release most resources by default without
VL_LEAK_CHECKS, so there is no behaviour change there.
Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
Signed-off-by: Artur Bieniek <abieniek@internships.antmicro.com>
Co-authored-by: Krzysztof Bieganski <kbieganski@antmicro.com>
Combined Dfg variable elimination into the regularization pass that runs
before converting back to Ast. This avoids introducing some unnecessary
temporaries.
Added replacing of variables with constants in the Ast if after the
Dfg passes they are known to be constants. This is only done in final
scoped Dfg application.
Avoid introducing temporaries for common sub-expressions that are
cheaper to re-compute than store in a temporary variable.
Enable removal of redundant unpacked array variables.
Also fixes#6394 as this patch involved changes to that code.
Do not apply V3Const in V3Expand after a function if nothing was
expanded in it.
Also fix statistics counter while at it.
Inspired by #6379, follow up from #6111
1. Move class V3ParseGrammar into V3ParseGrammar.h so editors understand
it as c++ code
2. Fix #line directives in the bison output file
This together enables us to gdb through V3ParseGrammar, verilog.y, and
the bison generated C code step by step, with all source annotations in
the debug info pointing to the right place (e.g.: you will step to the
right place in verilog.y, then step back to the bison generated switch
statement/loop, and then step into calls in V3ParseGrammar as kind of
expected.
These are all genuine bugs, brief descriptions.
1. V3OrderCFuncEmitter.h used to delete a node early that was still
reference in a graph dump later. Not a big deal, it can be deleted
later at the end of V3Order.
2. V3Param.cpp: this one is tricky. The variable referenced by
AstVarXRef was deleted at the end of `visit(AstGenCase*)`, but then
`visit(AstVarXRef*)` checks `nodep->varp()` (already deleted) to see
if it's in an interface.
3. V3String::wildMatch is sometimes called with an empty 's' (the string
we are matching against tha pattern 'p'), in which case it used to go
off into the woods. Added check on call. An arbitrary number of `*`
will still match the empty string.
4. V3Task.cpp: There was an error reported for an unsupported construct,
then a subsequent SEGV. Just signal the error upward so we bail on an
error in a more graceful way.
5. verylog.y: Some unsupported constructs failed to set the parsed node,
so some memory thrash made it into some code downstream. Just parse
these into nullptr.
Also increased the timeout on one test, which sometimes tripped with
asan on GCC during heavy host load.
Added a mini type system for Dfg using DfgDataType to replace Dfg's use
of AstNodeDType. This is much more restricted and represents only the
types Dfg can handle in a canonical form. This will be needed when
adding more support for unpacked arrays and maybe unpacked structs one
day.
Also added an internal type checker for DfgGraphs which encodes all the
assumptions the code makes about type relationships in the graph. Run
this in a few places with --debug-check. Fix resulting fallout.
Reduce set of synthesized logic to be more in-line with what Dfg used to
handle before + drivers of circular variables. This was always the
intention but the previous algorithm was both a bit too eager, and also
missed some circular variables. We can add back more heuristics based on
performance measurements for non-circular logic later.
Parts of this algorithm were distributed over many files some
masquerading as re-usable APIs, they were not. Move everything into one
file and avoid unnecessary virtual functions.
- Remove _ENUM_END, so -Wswitch does not demand it's covered. Use the
new NUM_TYPES constexpr member instead.
- Remove 'at' prefix. This seems historical and is not particularly useful.
- Fix some cppcheck warts while at it
Add DfgUserMap as a handle around the one pointer worth of algorithm
specific 'user' storage in each DfgVertex. This reduces verbosity,
improves type safety and correctness. Also enables us to remove one
pointer from DfgVertex to reduce memory use. No functional change.
Large scale refactoring to simplify some of the more obtuse internals of
DFG. Remove multiple redundant internal APIs, simplify representation of
variables, fix potential unsoundness in circular decomposition. No
functional change intended.
* Fix broken support of unassigned virtual interfaces
Unassigned virtual interface support added by #6245 is broken - PR marks
dead module as alive - we can't do that as once a module is dead it
needs to remain dead because earlier steps (e.g. port resolution) have
already been skipped.
This commit handles unassigned virtual interfaces at the beginning of
first pass of LinkDot (so it is never marked as dead, and no linking
steps are getting skipped).
Fixes#6253.
* Apply suggestions from code review
Co-authored-by: Wilson Snyder <wsnyder@wsnyder.org>
* Apply 'make format'
* Revert add of redundant iterateChildren() call
* Add original test case
---------
Co-authored-by: Wilson Snyder <wsnyder@wsnyder.org>
Co-authored-by: github action <action@example.com>
The previous algorithm was designed to handle the general case where a
full control flow path predicate is required to select which value to
use when synthesizing control flow join point in an always block.
Here we add a better algorithm that tries to use the predicate of
the closest dominating branch if the branch paths dominate the joining
paths. This is almost universally true in synthesizable logic (RTLMeter
has no exceptions), however there are cases where this is not
applicable, for which we fall back on the previous generic algorithm.
Overall this significantly simplifies the synthesized Dfg graphs and
enables further optimization.
There were a couple corner case bugs in V3Inline, and one in Dfg when
dealing with inlining of modules/variables.
V3Inline:
- Invalid code generated when inlining an input that also had an
assignment to it (Throws an ASSIGNIN, but this is sometimes reasonable
to do, e.g. hiererchical reference to an unonnected input port)
- Inlining (aliasing) publicly writeable input port.
- Inlining forcable port connected to constant.
Dfg:
- Inining publicly writeable variables
The tests that cover these are the same and fixing one will trigger the
other bug, so fixing them all in one go. Also cleanup V3Inline to be less
out of order and rely less on unique APIs only used by V3Inine (will
remove those in follow up patch).
Small step towards #6280.
Wilson Snyder
Geza Lore
Bartłomiej Chmiel
Igor Zaworski
Ryszard Rozak
Todd Strader
Artur Bieniek
github action
Szymon Gizler
Krzysztof Bieganski
Fabian Keßler-Schulz
أحمد المحمودي
Jakub Wasilewski
Lan Zongwei
Aleksander Kiryk