PR #7902 uncovered a pre-existing bug in multi-threaded scheduling,
where we can end up with an un-ordered R-W hazard in the MTask graph,
resulting in non-deterministic runtime behaviour.
This is extremely hard to actually trigger on a small example, so using
ThreadSanitizer to flag it, which can identify the race reliably.
In this patch:
- Add configure and `verilator --get-supported TSAN` to check if the
configured compiler supports ThreadSanitizer
- Add a --tsan option to the test driver.py which builds the test with
thread sanitizer (similar idea to --gdbsim).
- Add a tests.enable_tsan() method to allow turning on TSan in the test
Python file.
- Add a suppressions file that waives TSan errors in the runtime library
- Finally add `t_sched_hybrid_hazard` that demonstrates the data race
triggered after #7902. This is currently expected failing, fix later.
With the suppression, there are 17 vltmt tests failing due races in the
generated code. (Using `driver.py --vltmt --tsan --quiet -j0`)
Restrict VL_RESTORER to be usable only with trivially copyable types.
Introduce VL_RESTORER_COPY and VL_RESTORER_CLEAR, which are more
efficient versions usable for non trivially copyable types.
VL_RESTORER_COPY semantically behaves the same as VL_RESTORER, but only
does one copy at initialization, and on scope exit restores via a move.
VL_RESTORER_CLEAR swaps the variable with an new one constructed via
the no-args constructor (e.g. empty collection), which does not require
any copying at any point.
Static assertions enforce picking one of the new flavours when copying
might be expensive.
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.
Fix scheduling of writes in virtual interfaces, there were missing triggers (see added test).
Make V3SchedVirtIface handle writes done inside methods called through a virtual interface. The pass first records direct vif.member writes, VIF method calls, and candidate interface member VarScopes. It then walks the methods reachable from those VIF calls, writes to persistent interface variables in those method bodies are treated as VIF writes, and nested calls are followed with the same interface context. Function locals, temps, and events are ignored because they are not persistent interface storage observable through a later VIF read. Triggers are still created only from the intersection of (interface type, member name) writes and matching VarScopes, so unrelated interface variables and interfaces with no virtual access do not get extra triggers.
Recognize the common single-bit scan loop idioms in V3Unroll (before it
unrolls) and lower them to bit-reduction primitives, replacing a literal
W-iteration loop with one intrinsic-backed expression:
target=0; for (i=0;i<W;i++) if (vec[i]) target = i + 1; -> $mostsetbitp1(vec)
target=0; for (i=0;i<W;i++) if (vec[i]) target = target + 1; -> $countones(vec)
The leading-one form lowers to a new AstMostSetBitP1 node, emitted as
VL_MOSTSETBITP1_{I,Q,W}; those runtime helpers now use __builtin_clz where
available (same pattern as VL_REDXOR's __builtin_parity), with the existing
bit scan as fallback. The count-ones form reuses AstCountOnes ($countones,
popcount); as the DFG requires a 32-bit countones result it is built at 32
bits and narrowed to the accumulator width with a select.
Matching is structural to stay sound: the index must start at 0, increment
by exactly 1, and scan all W==width(vec) bits via a single 1-bit select of a
distinct vector, with the target pre-zeroed and no else branch. The loop
bound is accepted as a strict ascending 'idx < W' written either way and
signed or unsigned (Gt/GtS/Lt/LtS). Gated by -fbit-scan-loops (on at -O).
Adds t_bit_scan_loops (I/Q/W, count-ones and unsigned-index positives;
step-2, start-1, idx*2+1, vec[idx+1], target=idx and W!=width negatives, all
self-checked and asserted via --stats not to lower) plus t_bit_scan_loops_off
for the disable flag.
Motivated by a transformer inference design whose 80-bit leading-one detector
ran every cycle (~37% of runtime); the lowering is worth ~39% there.
Yilou Wang
Geza Lore
Yogish Sekhar
Artur Bieniek
Kornel Uriasz
github action
Jaeuk Lee
Wilson Snyder
Patrick Creighton
Jakub Michalski
Matthew Ballance
Igor Zaworski
Pawel Klopotek
Eryk Szpotański
Sergey Chusov
Joshua Leahy
Nick Brereton
Wolfgang Mayerwieser
Ryszard Rozak
Thomas Santerre
Tom Jackson
Jakub Wasilewski