Repository navigation
feat(sv): support unpacked structures as dynamic container elements - #2422
Conversation
|
This pull request is part of a Mergify stack:
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79d36ee85f
ℹ️ 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".
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
c3f54de to
c71b11a
Compare
79d36ee to
437b350
Compare
Revision history
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 437b350222
ℹ️ 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".
c71b11a to
ecce9b8
Compare
437b350 to
53dedc6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53dedc6898
ℹ️ 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".
Queues, dynamic arrays and associative arrays of an unpacked structure type were rejected. A structure of at most 64 bits whose members are all 2-state or all 4-state and hold no handle is kept as its flat value, so the container's default element is the structure's default. Any other structure is held as an immutable heap record: `ObjectOp::Box` builds a record from the flat value on `push`, `insert` or an assignment, and `ObjectOp::Unbox` reads it back. Each leaf is a record field, so the collector traces string and handle members through the record's handle list, and 4-state members keep their X/Z bits. Record fields are `nullable`: the new `OBJECT_LOAD_NULLABLE` heap operation reads a null record as zeros without a fault, which is the default structure for a missing associative entry or a new dynamic array element. Records are not seeded, so they do not draw from the process RNG. `e.m` reads a member of an element `q[i]`, `aa[k]` or a popped element directly from the record or the flat value, including a handle member's properties (`q[0].orig.m_type.n`). Patterns, typed patterns and values of the structure type initialize elements. Comparing containers of records and sorting structures without `with` are rejected, as is writing a member of an element. Assignment patterns passed to a method argument now receive the formal's shape, which method calls taking a structure need. Change-Id: Iac820f46cbee8d792f4b0fbde28baccdd54ecf31
ecce9b8 to
9c93687
Compare
53dedc6 to
15e6872
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15e6872922
ℹ️ 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".
| .expect("a leaf of the structure"); | ||
| match &target.record { | ||
| Some(record) => { | ||
| classes::object_call(ObjectOp::Load(record.fields[index]), vec![raw.clone()]) |
There was a problem hiding this comment.
In four-state simulation, a record-backed element such as struct { string name; logic flag; } can return X from this load, but case::expr_is_two_state currently classifies every non-container object operation as two-state. Consequently, if (q[0].flag || !q[0].flag) is folded to unconditional true by statements::simplify_condition, even when flag is X and neither branch condition should be true. Update the two-state classifier to account for the field's four_state metadata when introducing these record loads.
Useful? React with 👍 / 👎.
| let object = self.heap_call( | ||
| ops::encode(ops::OBJECT_NEW, u64::from(sv::ir::Record::CLASS)), | ||
| &[size], | ||
| ); |
There was a problem hiding this comment.
For record-backed structures, assigning an element its existing value now spuriously triggers whole-container events. For example, after initializing a module-level queue of struct { string name; int value; }, executing q[0] = q[0] allocates a fresh record here. Heap::finish_container canonicalizes containers by their stored element handles, not the records' contents, so the unchanged queue receives a different handle and wakes @(q). This breaks the equal-update event behavior already covered by containers::equal_updates_preserve_event_waits. Canonicalize immutable records by value, or otherwise preserve container identity for equal record updates.
Useful? React with 👍 / 👎.
Queues, dynamic arrays and associative arrays of an unpacked structure
type were rejected. A structure of at most 64 bits whose members are all
2-state or all 4-state and hold no handle is kept as its flat value, so
the container's default element is the structure's default. Any other
structure is held as an immutable heap record:
ObjectOp::Boxbuilds arecord from the flat value on
push,insertor an assignment, andObjectOp::Unboxreads it back. Each leaf is a record field, so thecollector traces string and handle members through the record's handle
list, and 4-state members keep their X/Z bits. Record fields are
nullable: the newOBJECT_LOAD_NULLABLEheap operation reads a nullrecord as zeros without a fault, which is the default structure for a
missing associative entry or a new dynamic array element. Records are not
seeded, so they do not draw from the process RNG.
e.mreads a member of an elementq[i],aa[k]or a popped elementdirectly from the record or the flat value, including a handle member's
properties (
q[0].orig.m_type.n). Patterns, typed patterns and values ofthe structure type initialize elements. Comparing containers of records
and sorting structures without
withare rejected, as is writing amember of an element.
Assignment patterns passed to a method argument now receive the formal's
shape, which method calls taking a structure need.
Depends-On: #2419