opt_modred: admit bounded nonlinear slots as opaque digits (-opaque-slots) - #308
Closed
akashlevy wants to merge 2 commits into
Closed
opt_modred: admit bounded nonlinear slots as opaque digits (-opaque-slots)#308akashlevy wants to merge 2 commits into
akashlevy wants to merge 2 commits into
Conversation
…lots) A balanced tree of residue LUTs is not linear in its raw inputs: a 6->3 leaf table decodes the canonical digits only and answers 0, not the residue, for the 2^k-1 its own inputs can reach. The proof therefore failed at the leaves and the whole tree was left serial, even though the tables *above* the leaves do add mod M over whatever the leaves emit. Under -opaque-slots (default off, so this is a no-op on its own), a slot whose own proof failed is admitted as an opaque k-bit digit once a sweep shows its image misses 2^k-1, and the levels above it are proven linear in those digits. That re-brackets the upper levels of the tree as one end-around-carry carry-save tree. Two guards keep it from firing where it cannot pay: - Two opaque digits minimum. One is a serial cascade's own state, where the tree just re-adds the digit the step already produced while the chain stays alive to feed the later steps -- area for no depth (measured +9.5 LoL on a mod-61 cascade before this guard). - Cell-level region depth is only skipped in favour of the bit-level arrival test when the match actually uses opaque digits, since an opaque-slot region keeps the leaf tables that the depth above is counted over. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryThe PR adds opt-in support for treating bounded nonlinear reduction slots as opaque digits, enabling higher reduction-tree levels to be rewritten as carry-save trees.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| passes/opt/opt_modred.cc | Adds bounded opaque-slot proof composition, associated profitability guards and options, and correctly clamps the previously unsafe shift-count arguments. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Candidate reduction root] --> B[Build and sweep cut]
B --> C{Cut linear?}
C -->|Yes| D[Compose proven slot weights]
C -->|No, bounded and opaque enabled| E[Record image bound]
E --> F[Admit slot as opaque digit at upper level]
D --> G{Profitable rewrite?}
F --> G
G -->|Yes| H[Emit carry-save tree and normalize]
G -->|No| I[Keep original region]
Reviews (2): Last reviewed commit: "opt_modred: keep user-supplied bit count..." | Re-trigger Greptile
-max-bound-bits and -max-cut-bits both reach `int64_t(1) << n`, where a negative or >=63 value read straight off the command line is undefined behavior: the sweep limit comes out wrong, or a sanitizer build aborts. Clamp both to [0, 62] at the parse point, which covers all three shift sites rather than guarding each one. Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
@greptileai review |
Author
|
The motivating win on Companion Preqorsor PR: Silimate/preqorsor#2293 (closing as well). |
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.
Problem
A mod-7 checksum spelled as a balanced 8→4→2→1 tree of 6→3 residue LUTs was
left fully serial by
opt_modred.The leaf tables are not linear in the raw bits: a 6→3 table decodes the
canonical digits only, so it answers
0— not the residue — for the2^k-1its own inputs can reach. The proof therefore died at the leaves, even though
the tables above the leaves genuinely do add mod M over whatever the leaves
emit.
opt_modadd_treedoes not own this shape either, since it is a treerather than a serial cascade.
Change
New
-opaque-slots(default off). A slot whose own reduction proof failedis admitted as an opaque k-bit digit once a sweep shows its image misses
2^k-1; the levels above it are then proven linear in those digits andre-emitted as one end-around-carry carry-save tree.
A leaf is never treated as a residue, only as a bounded unknown digit — that is
what keeps this sound on tables that are merely mod-M-shaped.
Supporting pieces:
check_lineargains asweep_maxout-parameter and keeps sweeping past amismatch when the cut is cheap enough (
-max-bound-bits, default 10 bits),purely to get that image bound.
bound_memokeeps the tightest bound any cutof a node yielded.
Proof::opaquecounts admitted digits through the whole composition.Guards
tree merely re-adds the digit the step already produced while the chain stays
alive to feed the later steps — area for no depth. Measured +9.5 LoL on a
mod-61 cascade before this guard existed.
test when the match actually uses opaque digits, since an opaque-slot region
keeps the leaf tables that the depth above is counted over.
Validation
Consumer-side measurements are in the companion PR. Summary: 3 cases win
(−4.75, −6.25 and −1.50 LoL, each also 10–15% smaller), 1 trades 0.75 LoL for
6.5% area, and 20 of 24 A/B'd designs — barrel shifters, add trees, add chains,
multipliers — are bit-identical.
equiv_opt -assertpasses on the three cases where the new path fires and thedesign is small enough to prove, including the one whose leaf tables differ
from
A % 7on ~74% of inputs.Because the flag defaults off, this is a no-op on its own and safe to merge
first: with
-opaque-slotsabsent, every A/B'd case reproduces its baselinemetrics exactly. The behavior change lands atomically in the preqorsor PR.
Made with Cursor