Rewrite module inlining decision to be based on a bipartite Module/Cell
graph, similar to V3InlineCFuncs. Preserved all old heuristics, but
added 2 new ones:
- If a module, and all the sub-hierarchy below it, is less than 10% the
total flattened size of the design, then flatten the contents of that
module (but the module itself is not necessarily inlined).
- If the flattened size of all instances of a module is less than 20% of
the total flattened size of the design, then inline all instances of
that module.
These are both relative to the total size of the design, so they
auto-scale with complexity. The net effect is that large shared
instances are preserved, but their contents are flattened out. E.g. in a
multi-core CPU this would keep the cores non-inlined but flatten out
most everything else. This still enables V3Combining and sharing those
later, but avoids potentially big overheads e.g. with small widely used
library modules.
Empirically this yields less generated C++ than the previous version
(due to removing lots of small functions), and can improve performance
10-20% while still having meaningful combining relative to the size of
the design.
When a lot of combinational logic is driven from top level inputs,
work can be wasted evaluating that logic if the top level inputs don't
change.
This change adds an optimization by performing a change detect on the
top level inputs, and evaluate 'ico' logic only if the top level input
actually changed. This especially helps with --hierarchical/--lib-create
which runs the 'ico' of each sub-model in the eval settle loop.
This was observed to yield 40%+ run-time speedup on some partitioned
designs.
The added change detection is cheap, so it is emitted even if the 'ico'
region is small, and is on by default.
The optimization is only sound if the model itself does not write to the
top level inputs (otherwise the 'previous value' variables would be out
of sync, which are not updated by internal writes.). If we can detect a
top level input is written within the design, then for that input, we
fall back on always running the relevant logic. With --vpi we cannot
prove safety statically, so --vpi will disable this optimisation unless
explicitly enabled. (In which case it's the user's responsibility to not
write to top level inputs via the VPI.)
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.
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.
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.
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.
This functionality used to be distributed in the removeVars pass and the
final dfgToAst conversion. Instead added a new 'regularize' pass to
convert DFGs into forms that can be trivially converted back to Ast, and
a new 'eliminateVars' pass to remove/repalce redundant variables. This
simplifies dfgToAst significantly and makes the code a bit easier to
follow.
The new 'regularize' pass will ensure that every sub-expression with
multiple uses is assigned to a temporary (unless it's a trivial memory
reference or constant), and will also eliminate or replace redundant
variables. Overall it is a performance neutral change but it does
enable some later improvements which required the graph to be in this
form, and this also happens to be the form required for the dfgToAst
conversion.
Geza Lore
Wilson Snyder
Yogish Sekhar
Bartłomiej Chmiel
Igor Zaworski
Veripool API Bot
Oleh Maksymenko
Todd Strader
Yilou Wang
Geza Lore
Ryszard Rozak
Igor Zaworski
Zhou Shen
Andrew Nolte
Szymon Gizler