[pull] main from B-Lang-org:main#185
Merged
Merged
Conversation
Also, currently no attempt is made to track positions through arrays.
Retain the previous cross-referencing code as comments in case it is useful for improving future position-tracking.
Add targeted strictness in ISyntaxSubst so that it does not introduce unnecessary thunks.
The basic pattern is to have separate equations for tracing and non-tracing code so that it is easy for GHC to see that non-tracing code does not retain references to input expressions after evaluation.
The strict evaluation means that they will not hold on to references to their source expression in thunks for calculating their type or position information (which would cause a leak).
… bounds. Note: this originally increased the stack by adding -K20 when no -K flag appeared; but it was rebased on 'main' which had -K30, so now it decreases the stack.
The three heap-allocation sites in IExpandUtils (addHeapUnev, addHeapWHNF, addHeapPred) each built an IRefT and deepseq'd it to avoid retaining the original expression through thunks in the type and position fields. Move that pattern (and its explanatory comment) into one utility, mkIRefT, and use it at all three sites. The separate deepseq of the cell name in addHeapWHNF is kept, since it is about the name, not the reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0163DheA2134fySFQW71MfBe Co-authored-by: Claude <noreply@anthropic.com>
The ClockCheck* tests in bsc.mcd/Misc xfail their comparison against the desired output (with positions on the cross-domain method calls). Cite GitHub issue #863 as the bug id instead of old Bluespec Inc bug 1238, and drop the mention of the -cross-info flag, which has been removed. Also add a plain compare_file against a .bad-expected snapshot of the current position-less output for each test, so that any change in the output is noticed. For ClockCheckCond, repurpose the orphaned nocrossinfo expected file (nothing referenced it) as that snapshot; the snapshots for the other four tests still need to be captured from a testsuite run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0163DheA2134fySFQW71MfBe Co-authored-by: Claude <noreply@anthropic.com>
Captured from an actual run of the compiler (with upstream main merged in), for the compare_file checks added alongside the xfailed compare_file_bug checks: these pin the current position-less G0007 output so unrelated drift fails outright while issue #863 stays open. ClockCheckCond's snapshot already exists (renamed from the orphaned nocrossinfo expected file, verified byte-identical to current output). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0163DheA2134fySFQW71MfBe Co-authored-by: Claude <noreply@anthropic.com>
Cells enter the heap only through addHeapCell and updHeap. Force the name at allocation, matching updHeap's existing deepseq on update, so that the heap never stores a name thunk. This subsumes the deepseq that addHeapWHNF did on its own name and extends the same guarantee to all other cell allocations, in particular HUnev cells, where an unforced inferName-derived name could retain a stale snapshot of another heap cell (via inferName's IRefT clause) or arbitrary expression-derived structure. Also update the commented-out addHeapNF to use mkIRefT. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BaLzQyHtX6CvXty92voyM8 Co-authored-by: Claude <noreply@anthropic.com>
…efer through log-depth play calls
Add a shared idQuality scoring function in Id (isKeepId beats non-bad beats bad beats Nothing) and use it from two CSE-style sites that previously had their own ad-hoc preferences: * ITransform.runCSE's rename_map: the per-CSE-group list of replaced defs is now stored as a quality-ranked Set so picking the best Id is S.findMax in O(log n). The DefP_NoCSE filter is applied at build time; missing keys map to the cse_name identity via iTransRenameId's findWithDefault. * IExpand.eqPtrs pass 2: after the first-wins canonical selection, walk each equivalence class and swap in the highest-quality pointer as canonical. Without this, the canonical was whichever pointer pass 1 happened to visit first, leading to generated code that preferred method-derived names (e.g. "_read__h47") over user-source variable names (e.g. "idx__h115") for the same expression. The pass-2 work uses strict folds and bang patterns on the (dsm, ptrm) tuple to stay stack-safe at scale.
Three synthetic modules that catch the InlineCReg / AVerilog / AExpand quadratics and the strict-state stack-overflow fixes introduced earlier on this branch. Each test's .exp documents which fixes it primarily exercises and which others it implicitly requires (the bsc compile would overflow or time out without them). - CregInlineBlowup: 4096 CRegs with trivial rules. Catches the InlineCReg O(svars * creg_ports) scan, AVerilog ds7 quadratic and AExpand.aoptExpandTest. Compiles in ~25 s with all fixes. - CregMemBlowup: 256 CRegs plus heavy per-rule arithmetic. Primarily catches the AVerilog ds7 quadratic (verilog phase 60x faster with the fix). Needs +RTS -K3G -RTS. - AoptScanBlowup: 8192 plain Regs + 512 rules with deep arithmetic. Catches the AExpand.aoptExpandTest O(defs * submodule-vars) scan in the aopt phase (~10x faster with the fix). Needs +RTS -K3G -RTS.
Extend check_intermediate_files to take an optional list of
synthesised modules for which a .ba file is expected, and check that
each one exists. The existing .bo check is unchanged.
Callers pass the modules list whenever bsc is expected to produce
.ba files:
- sim compiles (compile_object_pass[_warning]) always produce .ba,
so the module is passed unconditionally.
- verilog compiles only produce .ba when -elab is in the options;
a small helper, verilog_ba_modules, returns either the module
name or an empty list. Used by compile_verilog_pass,
compile_verilog_pass_warning[_bug],
compile_verilog_pass_no_warning[_bug],
compile_verilog_schedule_pass, and compile_synthesize_verilog_pass.
The non-prof default stays at 10m. Profiling builds get a separate 256m default because -fprof-auto adds a cost-centre stack frame around every closure entry, which on the perf regression suite multiplies peak stack usage by roughly an order of magnitude. Without this, BSC_BUILD=PROF could not run the regression tests using their non-prof bounds.
Replace the legacy -K3G overrides (which were left over from before the strictness/quadratic fixes) with tight per-test caps. Each cap is roughly 2x the measured peak so that a regression growing non-prof stack usage will be caught: CregInlineBlowup peak ~6M cap -K16m CregMemBlowup peak ~6M cap -K16m AoptScanBlowup peak ~12M cap -K40m (3x to absorb CI variance)
Add which_dumpba/dumpba/dumpba_pass analogs of the existing dumpbo machinery in config/unix.exp, plumb DUMPBA through suitemake.mk, and run dumpba on each .ba file that check_intermediate_files verifies (internal-check runs only, matching dumpbo). Update the README paragraph that described .ba sanity-checking as a future extension. dumpba already builds as part of UTILEXES and is installed by install-utils. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0163DheA2134fySFQW71MfBe Co-authored-by: Claude <noreply@anthropic.com>
The eqPtrs performance rework on this branch (foldl' over a strict tuple) also switched the topological sort from GraphWrapper.tSortInt to SCC.tsort, and added a second pass that re-picks the best-named pointer per CSE equivalence class. Both changes could flip defs from hidden to visible in the Bluesim symbol table and VCD. This commit fixes both while keeping the performance rework: 1. Restore GraphWrapper.tSortInt in place of SCC.tsort. The two sorts break ties between independent nodes differently, and the tie order decides which of two equal-expression heap cells pass 1 sees first and makes the CSE canonical. When one twin carries a name hint and the other does not, that choice is user-visible: pDef keeps well-named defs (setKeepId) and hides nameless ones (setBadId), so the different tie order silently turned previously hidden defs into visible ones. Generated-code behavior must not depend on a topological sort's tie order; keep upstream's sort so the pass-1 canonical choice matches upstream. 2. Guard the best-named-canonical pass (pass 2) so it only re-picks among visibly named pointers (idQuality >= 1). Unguarded, it could promote a named pointer over a nameless pass-1 canonical -- the same hidden-to-visible flip, this time deterministic. With the guard, a hidden canonical stays canonical; pass 2 may only improve WHICH visible name is used, never create a newly visible def. Also update the now-stale SCC.tsort reference in the perf-creg-blowup test comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011X4pjSzD37mKTcPUG9Z7Gw
eqTypeNum was calling satisfy (dvs=Nothing), which prevented genAddInsts "last resort" rules from firing. For types with abstract type variables (e.g., SizeOf on PrimPair), Add t1 t2 ?v couldn't resolve because the rule requires dvs=Just to produce TAdd t1 t2. Switching to satisfyFV passes the bound type variables, enabling these resolutions.
Replace the old -trace-eq-witnesses flag and scattered commented-out traces with a single -trace-icheck flag that covers eqType0, eqType1, eqTypeNum, tCheck, addDict, and addT. Remove dead code (commented-out pattern, stale type comment).
Previously, fixupDefs replaced a package's ATF cache with the union of its own entries and the caches of all imported packages, and genBinFile then serialized that merged cache into the .bo file. Since every imported .bo already carried its own merged cache, each .bo stored the transitive union of the caches of its entire dependency cone, growing monotonically down the import graph. Every compile then paid to deserialize those unions from every import, hold them live through the middle-end (inflating every major GC), redundantly re-merge them once per synthesized module (via updDef -> fixupDefs), and re-serialize the whole union into its own .bo. Measured on the shipped libraries: unrelated leaf packages (FShow, RWire, DefaultValue, ...) each carried an identical ~1.8 KB inherited payload; the deepest package (Math) grew 21 KB (1.57x). With this change the leaf payload drops to ~34 bytes (an empty own cache) and Math carries only its own ~300 bytes of entries. Instead, keep only the package's own typecheck/ctxreduce-derived entries in the IPackage (and thus the .bo), and build the union needed for elaboration once in compilePackage from the caches of the loaded imports. Because the loaded imports always cover the package's full transitive closure (each .bo's ipkg_depends records its writer's entire loaded closure), the elaboration-time cache contents are unchanged, so fullTypeNormalizer's hit rate is identical. Bump the .bo format tag since the stored contents changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011X4pjSzD37mKTcPUG9Z7Gw
An incoherent match is an information-dependent choice (it varies with -allow-incoherent-matches, in-scope instances, and import order), so persisting its ATF projection into the .bo cache would deliver a context-dependent answer to importing compiles as if it were canonical. Drop the recordATFs call from the incoherent arm; coherent full discharges still record. The cache previously had only miss-path tests, so a lost recording site would go unnoticed by a green suite. Add hit tests: a same-package hit (ground resolution recorded by a monomorphic definition, consumed when elaborating a polymorphic body) and a cross-package hit delivered through the .bo cache, both asserted via -trace-atf-cache. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cqQdWKp7jr73GHQYnvhiq
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )