opt_signif: carry operand ranges across module boundaries - #284
Closed
akashlevy wants to merge 1 commit into
Closed
Conversation
The pass analysed one module at a time, so an instance was an opaque driver and
its output port contributed its declared width. A dot-product lane behind a
module boundary therefore priced its accumulate tree at the full datapath bus
even though the lane can only ever reach a fraction of it, which is where the
sawtooth in block-scaled MX designs came from: the narrower the format, the
larger the dead fraction, so e2m1 over-counted about twice as much as e4m3.
HierCache computes and caches the output-port ranges of instantiated modules and
sig_range now consults it for instances. Driver grew cell/port/offset so built-in
cells and instances take one path rather than special-casing ports named Y.
Flows that blackbox modules before narrowing them (parallel synthesis hands each
worker only its own batch) have no body left to read, so -summarize stamps the
ranges onto output ports as signif_range_signed/signif_range_unsigned attributes
while the bodies are still present, and a later run reads them back off the
blackbox. -no-cross-hier turns the whole thing off.
Two fixes fall out of exercising this on real datapaths:
- cell_range judged every range by its signed width, which discarded any
non-negative range reaching into the top half of its word. A 6-bit word
holding [0,45] is a block-scaled lane's exponent sum, and it was being
thrown away. Judge by unsigned width when the range cannot go negative;
sig_range still re-checks per reader, so a signed reader of such a word
falls back to the declared range.
- -min-bits <n> skips narrowings that save fewer than n bits. arith_tree only
chains a cell whose output is read whole, so shaving one bit off a parent's
operand port can drop the sum below carry_save_min_width and cost an entire
compressor level -- a real regression on tile_mul_shift, where a 29-bit
adder stopped forming a 30-bit compressor.
Tests cover the hierarchy cases (single and two-level, unbounded inputs as a
negative control, blackboxes, repeated instances, and an instance with a port
named Y), the attribute round-trip including stale-attribute removal, wide
constant words under -cross-flops, the -min-bits threshold, and a formal
equivalence proof of hierarchical narrowing via the gold/gate flatten idiom.
Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryThis PR extends
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code failure identified. Hierarchical ranges are computed with unconstrained child inputs, selected by reader signedness, checked against consumed widths, and conservatively fall back to declared ranges when analysis is unavailable; the added tests cover the principal new execution paths.
|
| Filename | Overview |
|---|---|
| passes/opt/opt_signif.cc | Adds conservative hierarchical range propagation, persisted blackbox summaries, wide-constant handling, and a minimum-savings threshold; no changed-code defect was established. |
| tests/opt/opt_signif.ys | Expands regression coverage for hierarchy traversal, summary attributes, repeated instances, boundary constants, narrowing thresholds, and formal equivalence. |
Sequence Diagram
sequenceDiagram
participant Parent as Parent SignifWorker
participant Cache as HierCache
participant Child as Child SignifWorker
Parent->>Cache: Request instance output range
alt Summary not cached
Cache->>Child: Analyze output port with unconstrained inputs
Child-->>Cache: Signed and unsigned ranges
Cache->>Cache: Cache ranges by module and port
end
Cache-->>Parent: Range for reader signedness
Parent->>Parent: Validate range fits consumed bits
Parent->>Parent: Narrow eligible operand if savings meet min-bits
Reviews (1): Last reviewed commit: "opt_signif: carry operand ranges across ..." | Re-trigger Greptile
akashlevy
marked this pull request as draft
August 21, 2026 13:51
Author
|
Closing for now, as we need a better test case before we can continue here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
opt_signifanalysed one module at a time, so an instance was an opaque driver whose output port contributed its full declared width. A dot-product lane behind a module boundary therefore had its accumulate tree priced at the whole datapath bus, even though the lane's exponent range can only reach a fraction of it.That is the root cause of the sawtooth we have been chasing in a customer's block-scaled MX dot products. The dead fraction scales with how narrow the format is, so the error was strongly family-dependent: on a 64-bit bus an
e2m1lane occupies 11 bits and over-counted by about 2x, whilee4m3occupies 39 bits and over-counted by about 1.2x. Adjacent points in their sweep alternate between families, which is what produced teeth rather than a uniform offset.Four changes:
HierCachecomputes and caches output-port ranges of instantiated modules;sig_rangeconsults it for instances.Drivergained cell/port/offset so built-in cells and instances share one path instead of special-casing ports namedY.-summarize. Flows that blackbox modules before narrowing them have no body left to read. This stamps ranges onto output ports assignif_range_signed/signif_range_unsignedwhile the bodies still exist, and a later run reads them back off the blackbox.-no-cross-hierdisables the feature entirely.cell_rangejudged every range by its signed width, discarding any non-negative range that reaches into the top half of its word. A 6-bit word holding[0,45]is a block-scaled lane's exponent sum, and it was being thrown away.sig_rangestill re-checks per reader, so a signed reader of such a word falls back to the declared range.-min-bits <n>. Skips narrowings saving fewer than n bits.arith_treeonly chains a cell whose output is read whole, so shaving one bit off a parent's operand port can drop a sum belowcarry_save_min_widthand cost a whole compressor level. This was a real regression ontile_mul_shift, where a 29-bit adder stopped forming a 30-bit compressor.Measured effect
Reproduced the customer's sweep locally (50 designs, asap7). Comparing predicted area against their reported real area, with
-no-cross-hierstanding in for the shipped behaviour:Flattening the design reaches the same place (0.96 / 1.88x / 0.961), which is a useful workaround for users today but costs runtime and defeats parallel batching on large designs.
Test plan
tests/opt/opt_signif.yspasses; new groups cover single-level and two-level hierarchy, unbounded submodule inputs as a negative control, blackboxes with narrow and wide ports, repeated instances of one module, an instance with a port namedY, the attribute round-trip including stale-attribute removal, wide constant words under-cross-flops, and the-min-bitsthresholdmain(42 commits past the original base)Made with Cursor