Skip to content

feat(sv): pass unpacked structures by reference - #2473

Merged
mergify[bot] merged 1 commit into
masterfrom
stack/tignear/feat/sv-uvm-unpacked-structs/pass-unpacked-structures-ref--41a3098e
Oct 28, 2026
Merged

mergify[bot] merged 1 commit into
masterfrom
stack/tignear/feat/sv-uvm-unpacked-structs/pass-unpacked-structures-ref--41a3098e

Conversation

@tignear

@tignear tignear commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

A ref or const ref formal of unpacked structure type was rejected,
which blocks UVM: uvm_reg_adapter::reg2bus(const ref uvm_reg_bus_op rw)
and bus2reg(..., ref uvm_reg_bus_op rw) are elaborated with every
uvm_pkg. A structure is a set of member variables, so its reference
formal is now one reference per member, bound to the matching member of
the actual (IEEE 1800-2023 13.5.2): writes through the formal are seen at
once in both directions, also across task suspensions.

  • The analyzer records the members of a structure formal in
    SubroutineParam::leaves. A reference formal gets no copy-in or
    copy-out. The body that dispatches a virtual method forwards the members,
    so a call through a pure virtual method binds the override's members to
    the caller's.
  • Actuals: structure variables, members that are structures, class
    properties of structure type and their members. A scalar member of a
    structure is now also an ordinary reference actual (bump(x.a)).
  • Type equivalence (6.22.2) compares structure types by identity, also
    for containers of structures (ref s_t q[$]).
  • const ref writes, nonblocking writes and fork uses are checked per
    member.
  • The frontend binds each member like a scalar reference. A concatenation
    assignment to members bound to class properties writes temporaries in
    their place, so every destination position is taken before any write,
    then stores each property. A formal bound to a property and passed on
    to an output or inout structure argument copies each member back.
  • A receiver with a side effect (a pop, $urandom, a call) is rejected
    for a structure property actual, as for its whole reads, since each
    member would evaluate it again.

Tests cover callee whole assignment, patterns, comparisons and passing
on by reference or value; enumeration, string, class handle and 4-state
members; handle members across heap collections; pure virtual
reg2bus/bus2reg; and a task holding the reference across delays.
Verilator 5.052 rejects two cases that bind a class property in a
function that constructs an object (%Error-IMPURE, also with a scalar
ref). xezim 0.11.3 shows its known copy semantics and padded string
properties. Icarus does not support unpacked structures. The docs state
that structure members may not be dynamic containers.

Depends-On: #2424

@tignear

tignear commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

This pull request is part of a Mergify stack:

# Pull Request Link
1 feat(sv): support unpacked structures #2415
2 feat(sv): support class properties of unpacked structure type #2419
3 feat(sv): support unpacked structures as dynamic container elements #2422
4 feat(sv): support nested dynamic containers and class handle keys #2424
5 feat(sv): pass unpacked structures by reference #2473 👈

@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-28T22:34:53.536298Z e0c060f 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: c841a15f04

ℹ️ 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".

@mergify

mergify Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

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

@tignear
tignear force-pushed the stack/tignear/feat/sv-uvm-unpacked-structs/support-nested-dynamic-containers-class-handle--54576c9f branch from dc72e4c to 6484baa Compare October 28, 2026 19:23
@tignear
tignear force-pushed the stack/tignear/feat/sv-uvm-unpacked-structs/pass-unpacked-structures-ref--41a3098e branch from c841a15 to 31d6b3f Compare October 28, 2026 19:23
@tignear

tignear commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Revision history

# Type Changes Reason Date
1 initial c841a15 2026-10-28 19:23 UTC
2 content c841a15 → 31d6b3f Review fixes: property-bound structure references forwarded to output/inout copy each member back; concatenations with property-bound members write temporaries first, keeping destination positions; e… 2026-10-28 19:23 UTC
3 content 31d6b3f → 43e8dc3 2026-10-28 20:25 UTC
4 content 43e8dc3 → e0c060f 2026-10-28 22:32 UTC

@tignear
tignear force-pushed the stack/tignear/feat/sv-uvm-unpacked-structs/support-nested-dynamic-containers-class-handle--54576c9f branch from 6484baa to ac30379 Compare October 28, 2026 20:25
@tignear
tignear force-pushed the stack/tignear/feat/sv-uvm-unpacked-structs/pass-unpacked-structures-ref--41a3098e branch from 31d6b3f to 43e8dc3 Compare October 28, 2026 20:25
A `ref` or `const ref` formal of unpacked structure type was rejected,
which blocks UVM: `uvm_reg_adapter::reg2bus(const ref uvm_reg_bus_op rw)`
and `bus2reg(..., ref uvm_reg_bus_op rw)` are elaborated with every
`uvm_pkg`. A structure is a set of member variables, so its reference
formal is now one reference per member, bound to the matching member of
the actual (IEEE 1800-2023 13.5.2): writes through the formal are seen at
once in both directions, also across task suspensions.

- The analyzer records the members of a structure formal in
  `SubroutineParam::leaves`. A reference formal gets no copy-in or
  copy-out. The body that dispatches a virtual method forwards the members,
  so a call through a pure virtual method binds the override's members to
  the caller's.
- Actuals: structure variables, members that are structures, class
  properties of structure type and their members. A scalar member of a
  structure is now also an ordinary reference actual (`bump(x.a)`).
- Type equivalence (6.22.2) compares structure types by identity, also
  for containers of structures (`ref s_t q[$]`).
- `const ref` writes, nonblocking writes and fork uses are checked per
  member.
- The frontend binds each member like a scalar reference. A concatenation
  assignment to members bound to class properties writes temporaries in
  their place, so every destination position is taken before any write,
  then stores each property. A formal bound to a property and passed on
  to an `output` or `inout` structure argument copies each member back.
- A receiver with a side effect (a pop, `$urandom`, a call) is rejected
  for a structure property actual, as for its whole reads, since each
  member would evaluate it again.

Tests cover callee whole assignment, patterns, comparisons and passing
on by reference or value; enumeration, string, class handle and 4-state
members; handle members across heap collections; pure virtual
`reg2bus`/`bus2reg`; and a task holding the reference across delays.
Verilator 5.052 rejects two cases that bind a class property in a
function that constructs an object (`%Error-IMPURE`, also with a scalar
`ref`). xezim 0.11.3 shows its known copy semantics and padded string
properties. Icarus does not support unpacked structures. The docs state
that structure members may not be dynamic containers.

Change-Id: I41a3098eb3b4359ca618580fb4de6bff25676be4
@tignear tignear added merge-priority Prioritize queue admission and waiting order without interrupting running checks merge-ready Request merging through Mergify after required checks and removed merge-priority Prioritize queue admission and waiting order without interrupting running checks labels Oct 28, 2026
@tignear
tignear force-pushed the stack/tignear/feat/sv-uvm-unpacked-structs/pass-unpacked-structures-ref--41a3098e branch from 43e8dc3 to e0c060f Compare October 28, 2026 22:32
@tignear
tignear force-pushed the stack/tignear/feat/sv-uvm-unpacked-structs/support-nested-dynamic-containers-class-handle--54576c9f branch from ac30379 to 9fd32cc Compare October 28, 2026 22:32
@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 56777f4 at 23:20 UTC

Merged together with #2424. 14 min in the queue, 11 min of it CI.

So far

Merge conditions: 8 of 8 met

This pull request

Condition
✅ depends-on = celox-sim/celox#2424

The batch, tested on draft #2496

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

Base automatically changed from stack/tignear/feat/sv-uvm-unpacked-structs/support-nested-dynamic-containers-class-handle--54576c9f to master October 28, 2026 23:19
@mergify
mergify Bot merged commit 56777f4 into master Oct 28, 2026
25 of 44 checks passed
@mergify
mergify Bot deleted the stack/tignear/feat/sv-uvm-unpacked-structs/pass-unpacked-structures-ref--41a3098e branch October 28, 2026 23:20
@mergify mergify Bot removed the queued Mergify queue entry label Oct 28, 2026
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