Skip to content

perf(sv): bind class signatures once per scope during type discovery - #2421

Merged
mergify[bot] merged 1 commit into
masterfrom
stack/tignear/perf/sv-shared-scope-tables/bind-class-signatures-once-per-scope-type--a9baaf70
Oct 28, 2026
Merged

mergify[bot] merged 1 commit into
masterfrom
stack/tignear/perf/sv-shared-scope-tables/bind-class-signatures-once-per-scope-type--a9baaf70

Conversation

@tignear

@tignear tignear commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Each $bits, $size or other array query that validation binds discovers
the types of its owners' declarations, iterating over the scope's
declarations until they settle. Every pass bound the signature of every
method of every class specialization of the scope again, copying the whole
discovery context for each class. In uvm_pkg one query took 60 to 110 seconds
in three passes, and UVM elaboration did not finish in 25 minutes.

A class's signatures are bound in the scope that declares it, not in the
declarations around a query. Bind them once by discovering the scope's own
declarations from its base context until they settle, binding the class
signatures in every pass as before, so a method range that uses a scope
declaration (logic [$bits(data)-1:0]) still sees it. Later queries and
their passes publish the class methods' entries again while that context
(its constants, types and function results) and the set of specializations
are unchanged; the scope's other subroutines are still discovered with each
query, where a generate block may hide them. The type, function
result and parameter tables of a discovery context are shared until
written, so copying a context no longer copies them.

With the ref and unpacked-struct gates bypassed locally, UVM elaboration
of uvm_test_run_test now reaches its next unsupported construct after 177
seconds instead of running for more than 25 minutes.

A regression counts the bindings for 2 and 16 query-bearing methods: they
are equal, while rebinding on every discovery pass gives 30 and 198. The
shared case class::method_ranges_of_scope_declarations sizes a module
variable by a class method whose range uses $bits of a module variable;
Verilator 5.052 passes, Icarus 13.0 rejects the call in a declaration range
and the pinned xezim reads 1 (new sv_class-X16); both exact failures are
retained with adapter-free Top/Wrapped observations.
Analyzer/frontend units: 881 / 11; full SV suite: 6,247 passed, 20 skipped;
sv-tests verdicts unchanged.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T10:55:04.151187Z 3330429 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b61e93e1e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/celox-sv-analyzer/src/ast/dimensions/discovery.rs Outdated
Each `$bits`, `$size` or other array query that validation binds discovers
the types of its owners' declarations, iterating over the scope's
declarations until they settle. Every pass bound the signature of every
method of every class specialization of the scope again, copying the whole
discovery context for each class. In uvm_pkg one query took 60 to 110 seconds
in three passes, and UVM elaboration did not finish in 25 minutes.

A class's signatures are bound in the scope that declares it, not in the
declarations around a query. Bind them once by discovering the scope's own
declarations from its base context until they settle, binding the class
signatures in every pass as before, so a method range that uses a scope
declaration (`logic [$bits(data)-1:0]`) still sees it. Later queries and
their passes publish the class methods' entries again while that context
(its constants, types and function results) and the set of specializations
are unchanged; the scope's other subroutines are still discovered with each
query, where a generate block may hide them. The type, function
result and parameter tables of a discovery context are shared until
written, so copying a context no longer copies them.

With the `ref` and unpacked-struct gates bypassed locally, UVM elaboration
of uvm_test_run_test now reaches its next unsupported construct after 177
seconds instead of running for more than 25 minutes.

A regression counts the bindings for 2 and 16 query-bearing methods: they
are equal, while rebinding on every discovery pass gives 30 and 198. The
shared case `class::method_ranges_of_scope_declarations` sizes a module
variable by a class method whose range uses `$bits` of a module variable;
Verilator 5.052 passes, Icarus 13.0 rejects the call in a declaration range
and the pinned xezim reads 1 (new sv_class-X16); both exact failures are
retained with adapter-free Top/Wrapped observations.
Analyzer/frontend units: 881 / 11; full SV suite: 6,247 passed, 20 skipped;
sv-tests verdicts unchanged.

Change-Id: Ia9baaf704906c2ed6f06f6ffec193051824d5ed2
@tignear
tignear force-pushed the stack/tignear/perf/sv-shared-scope-tables/bind-class-signatures-once-per-scope-type--a9baaf70 branch from 5b61e93 to 3330429 Compare October 28, 2026 10:52
@tignear

tignear commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Revision history

# Type Changes Reason Date
1 initial 5b61e93 2026-10-28 10:52 UTC
2 content 5b61e93 → 3330429 Bind class signatures from a settled discovery of the scope's own declarations instead of the fixed base, and publish only class method entries, per review 4399250380; add class::method_ranges_of_sco… 2026-10-28 10:52 UTC

@mergify

mergify Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

@tignear tignear added the merge-ready Request merging through Mergify after required checks label Oct 28, 2026
@mergify mergify Bot added the queued Mergify queue entry label Oct 28, 2026
@mergify

mergify Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

✅ Merged into master as 2f5f265 at 12:13 UTC

28 min in the queue, 10 min of it CI.

So far

Merge conditions: 7 of 7 met

The batch, tested on draft #2437

Condition Required by
✅ check-success = Conventional Commit title —
✅ check-success = Queue validation —
✅ any of: check-success = @github-actions/Lint & Format, check-neutral = @github-actions/Lint & Format, check-skipped = @github-actions/Lint & Format GitHub repository ruleset rule master
✅ any of: check-success = @github-actions/Rust Test & NAPI Build, check-neutral = @github-actions/Rust Test & NAPI Build, check-skipped = @github-actions/Rust Test & NAPI Build GitHub repository ruleset rule master
✅ any of: check-success = @github-actions/JS Test (Ubuntu), check-neutral = @github-actions/JS Test (Ubuntu), check-skipped = @github-actions/JS Test (Ubuntu) GitHub repository ruleset rule master
✅ any of: check-success = @github-actions/Conventional Commit title, check-neutral = @github-actions/Conventional Commit title, check-skipped = @github-actions/Conventional Commit title GitHub repository ruleset rule master
✅ any of: check-success = @github-actions/Queue validation, check-neutral = @github-actions/Queue validation, check-skipped = @github-actions/Queue validation GitHub repository ruleset rule master

Times in UTC · dashboard · how the merge queue works

@mergify
mergify Bot merged commit 2f5f265 into master Oct 28, 2026
24 checks passed
@mergify
mergify Bot deleted the stack/tignear/perf/sv-shared-scope-tables/bind-class-signatures-once-per-scope-type--a9baaf70 branch October 28, 2026 12:13
@mergify mergify Bot removed the queued Mergify queue entry label Oct 28, 2026
tignear added a commit that referenced this pull request Oct 28, 2026
Allow modules, packages and compilation-unit classes to request package-class
specializations that the declaring package never names. This enables the UVM
registry pattern `typedef Registry#(User) type_id`, including calls through
`type_id::create()` that construct the supplied compilation-unit class.

Bind explicit actuals in the requesting scope and defaults, inherited member
types/constants and method bodies in their declaration scopes. Reuse one
canonical specialization and package state across importing scopes, preserving
nominal type identity, equivalent defaults, constructor defaults, static class
handles and distinct per-file compilation-unit classes. Discover signatures
before binding executable bodies with the supplied classes' metadata.

Keep module-local class actuals unsupported until their identity can distinguish
module instances. This change is independent of built-in process control and
virtual-interface support. It does not remove the existing restriction on
nonconstant package declaration initializers.

Add nine portable shared cases and API regressions for native images and
serialized checkpoints. Retain all 19 observed external-simulator failures with
unchanged expectations, exact diagnostics, standard review and six standalone
probes. xezim validation uses pinned source d491f948/core a25655a4 with a
regenerated dependency lock in an isolated Nix package; no simulator or
repository lock sources are changed.

Validation:
- All five backends: 50 foreign-specialization tests passed; class/package
  and concurrent-assertion regressions: 539 passed after syncing master.
  Package regressions: 70 passed after syncing master. Earlier full SV run: 6250 passed,
  20 ignored.
- Analyzer/frontend unit tests on master e7ec496: 896 passed, 3 ignored.
- NAPI/JS: 435 tests, playground 132 tests, and plugin/watch integration passed.
- Affected-crate Clippy, formatting and retained-report/gate checks passed.
- Nine external cases: Verilator 5 pass/4 retained failures, Icarus 9 retained
  failures, xezim 3 pass/6 retained failures. Standalone Celox probes pass in
  both two-state and four-state modes.

After syncing master 8c81192 (with #2322 and #2421): a scope without
classes of its own discovers queries as before, since it has no class
signatures to bind once; resolution keeps the built-in `process` fallback and
its type spelling. Analyzer/frontend units: 886 passed, 3 ignored; full SV:
6,421 passed, 20 skipped; retained-report and gate tests pass. Retained
Verilator/xezim diagnostics of the foreign cases follow their new case lines.

Change-Id: I94cfcf52749ba3ad9d7e7d893e795c86bc146939
tignear added a commit that referenced this pull request Oct 28, 2026
Allow modules, packages and compilation-unit classes to request package-class
specializations that the declaring package never names. This enables the UVM
registry pattern `typedef Registry#(User) type_id`, including calls through
`type_id::create()` that construct the supplied compilation-unit class.

Bind explicit actuals in the requesting scope and defaults, inherited member
types/constants and method bodies in their declaration scopes. Reuse one
canonical specialization and package state across importing scopes, preserving
nominal type identity, equivalent defaults, constructor defaults, static class
handles and distinct per-file compilation-unit classes. Discover signatures
before binding executable bodies with the supplied classes' metadata.

Keep module-local class actuals unsupported until their identity can distinguish
module instances. This change is independent of built-in process control and
virtual-interface support. It does not remove the existing restriction on
nonconstant package declaration initializers.

Add nine portable shared cases and API regressions for native images and
serialized checkpoints. Retain all 19 observed external-simulator failures with
unchanged expectations, exact diagnostics, standard review and six standalone
probes. xezim validation uses pinned source d491f948/core a25655a4 with a
regenerated dependency lock in an isolated Nix package; no simulator or
repository lock sources are changed.

Validation:
- All five backends: 50 foreign-specialization tests passed; class/package
  and concurrent-assertion regressions: 539 passed after syncing master.
  Package regressions: 70 passed after syncing master. Earlier full SV run: 6250 passed,
  20 ignored.
- Analyzer/frontend unit tests on master e7ec496: 896 passed, 3 ignored.
- NAPI/JS: 435 tests, playground 132 tests, and plugin/watch integration passed.
- Affected-crate Clippy, formatting and retained-report/gate checks passed.
- Nine external cases: Verilator 5 pass/4 retained failures, Icarus 9 retained
  failures, xezim 3 pass/6 retained failures. Standalone Celox probes pass in
  both two-state and four-state modes.

After syncing master 8c81192 (with #2322 and #2421): a scope without
classes of its own discovers queries as before, since it has no class
signatures to bind once; resolution keeps the built-in `process` fallback and
its type spelling. Analyzer/frontend units: 886 passed, 3 ignored; full SV:
6,421 passed, 20 skipped; retained-report and gate tests pass. Retained
Verilator/xezim diagnostics of the foreign cases follow their new case lines.

Review follow-ups: a request supplies only the caller's qualified routines
(the methods of the classes its actual type arguments name), not its own
unqualified functions; an unresolved name in a package class body binding
to an importing module's function also happens without importing-scope
specializations and is tracked in #2447. A compilation-unit typedef is named
with its source file, as compilation-unit classes are, so the same enum
typedef in two files gives two specializations; a Celox-only multi-file
test covers it, since IEEE 1800-2023 3.12.1 also allows one compilation unit
for all files (Verilator rejects the duplicate typedef). Analyzer/frontend
886, full SV 6,422 passed.

Change-Id: I94cfcf52749ba3ad9d7e7d893e795c86bc146939
tignear added a commit that referenced this pull request Oct 28, 2026
Allow modules, packages and compilation-unit classes to request package-class
specializations that the declaring package never names. This enables the UVM
registry pattern `typedef Registry#(User) type_id`, including calls through
`type_id::create()` that construct the supplied compilation-unit class.

Bind explicit actuals in the requesting scope and defaults, inherited member
types/constants and method bodies in their declaration scopes. Reuse one
canonical specialization and package state across importing scopes, preserving
nominal type identity, equivalent defaults, constructor defaults, static class
handles and distinct per-file compilation-unit classes. Discover signatures
before binding executable bodies with the supplied classes' metadata.

Keep module-local class actuals unsupported until their identity can distinguish
module instances. This change is independent of built-in process control and
virtual-interface support. It does not remove the existing restriction on
nonconstant package declaration initializers.

Add nine portable shared cases and API regressions for native images and
serialized checkpoints. Retain all 19 observed external-simulator failures with
unchanged expectations, exact diagnostics, standard review and six standalone
probes. xezim validation uses pinned source d491f948/core a25655a4 with a
regenerated dependency lock in an isolated Nix package; no simulator or
repository lock sources are changed.

Validation:
- All five backends: 50 foreign-specialization tests passed; class/package
  and concurrent-assertion regressions: 539 passed after syncing master.
  Package regressions: 70 passed after syncing master. Earlier full SV run: 6250 passed,
  20 ignored.
- Analyzer/frontend unit tests on master e7ec496: 896 passed, 3 ignored.
- NAPI/JS: 435 tests, playground 132 tests, and plugin/watch integration passed.
- Affected-crate Clippy, formatting and retained-report/gate checks passed.
- Nine external cases: Verilator 5 pass/4 retained failures, Icarus 9 retained
  failures, xezim 3 pass/6 retained failures. Standalone Celox probes pass in
  both two-state and four-state modes.

After syncing master 8c81192 (with #2322 and #2421): a scope without
classes of its own discovers queries as before, since it has no class
signatures to bind once; resolution keeps the built-in `process` fallback and
its type spelling. Analyzer/frontend units: 886 passed, 3 ignored; full SV:
6,421 passed, 20 skipped; retained-report and gate tests pass. Retained
Verilator/xezim diagnostics of the foreign cases follow their new case lines.

Review follow-ups: a request supplies only the caller's qualified routines
(the methods of the classes its actual type arguments name), not its own
unqualified functions; an unresolved name in a package class body binding
to an importing module's function also happens without importing-scope
specializations and is tracked in #2447. A compilation-unit typedef is named
with its source file, as compilation-unit classes are, so the same enum
typedef in two files gives two specializations; a Celox-only multi-file
test covers it, since IEEE 1800-2023 3.12.1 also allows one compilation unit
for all files (Verilator rejects the duplicate typedef). Analyzer/frontend
886, full SV 6,422 passed.

Second review round: a handle chain such as `a.next.next...` resolves each
receiver once for both the instance and the static property, instead of
doubling the walk per field; and a request binds against the template its
owner package declares, not a same-named class of another package in that
source (`class::foreign_generic_template_of_the_named_package`: Verilator
and xezim pass, Icarus rejects the class). Analyzer/frontend 887, full SV
6,427 passed.

Change-Id: I94cfcf52749ba3ad9d7e7d893e795c86bc146939
tignear added a commit that referenced this pull request Oct 28, 2026
Allow modules, packages and compilation-unit classes to request package-class
specializations that the declaring package never names. This enables the UVM
registry pattern `typedef Registry#(User) type_id`, including calls through
`type_id::create()` that construct the supplied compilation-unit class.

Bind explicit actuals in the requesting scope and defaults, inherited member
types/constants and method bodies in their declaration scopes. Reuse one
canonical specialization and package state across importing scopes, preserving
nominal type identity, equivalent defaults, constructor defaults, static class
handles and distinct per-file compilation-unit classes. Discover signatures
before binding executable bodies with the supplied classes' metadata.

Keep module-local class actuals unsupported until their identity can distinguish
module instances. This change is independent of built-in process control and
virtual-interface support. It does not remove the existing restriction on
nonconstant package declaration initializers.

Add nine portable shared cases and API regressions for native images and
serialized checkpoints. Retain all 19 observed external-simulator failures with
unchanged expectations, exact diagnostics, standard review and six standalone
probes. xezim validation uses pinned source d491f948/core a25655a4 with a
regenerated dependency lock in an isolated Nix package; no simulator or
repository lock sources are changed.

Validation:
- All five backends: 50 foreign-specialization tests passed; class/package
  and concurrent-assertion regressions: 539 passed after syncing master.
  Package regressions: 70 passed after syncing master. Earlier full SV run: 6250 passed,
  20 ignored.
- Analyzer/frontend unit tests on master e7ec496: 896 passed, 3 ignored.
- NAPI/JS: 435 tests, playground 132 tests, and plugin/watch integration passed.
- Affected-crate Clippy, formatting and retained-report/gate checks passed.
- Nine external cases: Verilator 5 pass/4 retained failures, Icarus 9 retained
  failures, xezim 3 pass/6 retained failures. Standalone Celox probes pass in
  both two-state and four-state modes.

After syncing master 8c81192 (with #2322 and #2421): a scope without
classes of its own discovers queries as before, since it has no class
signatures to bind once; resolution keeps the built-in `process` fallback and
its type spelling. Analyzer/frontend units: 886 passed, 3 ignored; full SV:
6,421 passed, 20 skipped; retained-report and gate tests pass. Retained
Verilator/xezim diagnostics of the foreign cases follow their new case lines.

Review follow-ups: a request supplies only the caller's qualified routines
(the methods of the classes its actual type arguments name), not its own
unqualified functions; an unresolved name in a package class body binding
to an importing module's function also happens without importing-scope
specializations and is tracked in #2447. A compilation-unit typedef is named
with its source file, as compilation-unit classes are, so the same enum
typedef in two files gives two specializations; a Celox-only multi-file
test covers it, since IEEE 1800-2023 3.12.1 also allows one compilation unit
for all files (Verilator rejects the duplicate typedef). Analyzer/frontend
886, full SV 6,422 passed.

Second review round: a handle chain such as `a.next.next...` resolves each
receiver once for both the instance and the static property, instead of
doubling the walk per field; and a request binds against the template its
owner package declares, not a same-named class of another package in that
source (`class::foreign_generic_template_of_the_named_package`: Verilator
and xezim pass, Icarus rejects the class). Analyzer/frontend 887, full SV
6,427 passed.

Change-Id: I94cfcf52749ba3ad9d7e7d893e795c86bc146939
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-ready Request merging through Mergify after required checks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant