Stateful POD and aggregate storage - #649
Conversation
305ed85 to
7cef052
Compare
7cef052 to
8c1fbaa
Compare
10496f8 to
447bd82
Compare
447bd82 to
7f5319e
Compare
7f5319e to
b927c8f
Compare
b927c8f to
c9ce071
Compare
c965086 to
e845f58
Compare
e845f58 to
232e94f
Compare
969d4e2 to
a3cb1e6
Compare
010c330 to
f841b5d
Compare
|
@codex review |
![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: f841b5d819
ℹ️ 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".
f841b5d to
7c0b886
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c0b8864b6
ℹ️ 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".
7c0b886 to
31f6931
Compare
31f6931 to
1e92ace
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e92ace677
ℹ️ 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".
| const llvm::DenseMap<SourceRef, SourceRefLatticeValue> storedValues = | ||
| materializeStoredValues(before, aliases); |
There was a problem hiding this comment.
When an array is created uninitialized, or an initialized element is overwritten, and a later array.read feeds a constraint, materializeStoredValues() has no entry for the array.write/array.insert: the array-access branch in visitOperation() returns without calling recordStorageWrite. Consequently the new dependency view leaves the read as its allocation address or stale initializer instead of the written operand, so the CDG can omit the actual witness dependency; record the rvalue against getWriteTargetState() with the same conditional-write semantics used for POD writes.
Useful? React with 👍 / 👎.
| void SourceRefAnalysis::StorageState::recordAggregateAlias( | ||
| Operation *op, size_t aliasIndex, const SourceRefLatticeValue &source, | ||
| const SourceRefLatticeValue &target | ||
| ) { |
There was a problem hiding this comment.
When an aggregate assignment occurs inside an scf.if branch or zero-trip loop, this alias event has no mayBeSkipped state and is therefore materialized unconditionally. For example, if a POD record initially contains aggregate A and is conditionally overwritten with aggregate B, B's alias—and its initializer write, which is also recorded as definite—canonicalizes B's fields onto the target even when the branch does not execute, causing the initial dependency to be dropped. Preserve both alias alternatives for potentially skipped regions and mark nested initializer events optional as well.
Useful? React with 👍 / 👎.
| Operation *defOp = sourceRoot->getDefiningOp(); | ||
| if (defOp == nullptr || !llvm::isa<CallOp, NewPodOp, CreateArrayOp, NonDetOp>(defOp)) { | ||
| continue; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier aggregate-initializer report is that this new filter explicitly rejects block-argument roots (getDefiningOp() == nullptr) and aggregate member values rooted at struct.new. When such an aggregate initializes a POD record and a nested child is later read, no alias rebases the requested suffix; resolution matches the root write and returns the source aggregate root, potentially including sibling dependencies, rather than the requested child. Support these source forms or transfer the queried suffix onto the stored aggregate value during resolution.
Useful? React with 👍 / 👎.
1 participant
Summary
Separates physical SourceRef identity from logical dependency resolution on top of #648. Raw lattices continue to identify POD and aggregate storage, while an explicit dependency view resolves stored values for clients that need logical dependencies. This is PR 2 of 6 in the stacked series.
Related issues
No standalone issue; depends on #648.
Changes
SourceRefAnalysis::getValueState()address-oriented for POD/array reads and aggregate allocation roots.SourceRefAnalysis::getDependencyState(DataFlowSolver &, Value), backed by aggregate aliases and definite/possible storage-write summaries.Testing
git diff --checknix build -Lon this exact stacked commit: 371 lit tests and 1,311 unit/CAPI tests passedSubmission checklist
If I am an external contributor, this PR has a linked issue markedapproved; otherwise, this does not apply.create-changelogin the Nix development shell creates a template.)I enabled Allow edits from maintainers if this PR comes from a fork.AI assistance
Tools used: OpenAI Codex.
How the tools contributed: Codex helped partition the original working-tree change, implement the raw-versus-resolved SourceRef architecture, add focused regressions, and prepare this draft PR.
How I verified the contribution: Reviewed the isolated diff and ran
git diff --check, the focused analysis regressions through the project test suite, andnix build -Lon this exact commit.