Scalarize uninitialized nested POD records by shankarapailoor · Pull Request #754 · project-llzk/llzk-lib

shankarapailoor · 2026-09-22T19:35:04Z

Summary

Keep unread nested POD records in explicit pod.new storage while mem2reg promotes their parent allocation. This lets later SROA and mem2reg rounds recursively scalarize the nested POD instead of leaving a residual POD-typed llzk.nondet.

Related issues

None.

Changes

Materialize an uninitialized POD record as pod.new in NewPodOp::getDefaultValue.
Add a focused PodToScalar regression for reading an uninitialized nested POD leaf.
Add a changelog entry for the corrected scalarization behavior.

Testing

nix develop --command bash -c "build/bin/llzk-opt --llzk-pod-to-scalar test/Transforms/PodToScalar/late_pod_nondet.llzk | FileCheck test/Transforms/PodToScalar/late_pod_nondet.llzk"
nix develop --command bash -c "cmake --build build --target check-lit" — 434 passed, 3 expected failures, 4 unsupported.
nix build -L — passed, including release build, lit tests, and unit/C API tests.

Submission checklist

If I am an external contributor, this PR has a linked issue marked approved; otherwise, this does not apply.
I added or updated tests for all relevant behavior, or explained above why tests are not needed.
I updated the relevant TableGen or other documentation, or explained above why documentation is not needed.
I added a changelog entry describing user-visible changes.
I enabled Allow edits from maintainers if this PR comes from a fork.

No TableGen documentation changes are needed; the implementation comment documents why nested POD defaults remain explicit storage.

AI assistance

No AI tools contributed to this PR.
AI tools contributed to this PR.

Tools used: OpenAI Codex.

How the tools contributed: Investigated the residual POD IR, reduced it to a focused regression, implemented the targeted fix, and drafted the changelog and PR description.

How I verified the contribution: Rebuilt llzk-opt, ran the focused FileCheck regression, ran the complete lit suite, and completed nix build -L successfully.

github-actions · 2026-09-22T19:47:30Z

Test Results

2 files ±0 2 suites ±0 3m 7s ⏱️ +49s
445 tests +3 441 ✅ +3 4 💤 ±0 0 ❌ ±0
890 runs +6 882 ✅ +6 8 💤 ±0 0 ❌ ±0

Results for commit 8ee913d. ± Comparison against base commit 24250ae.

♻️ This comment has been updated with latest results.

tim-hoffman · 2026-09-22T19:56:25Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d0b8c59ed

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

chatgpt-codex-connector · 2026-09-22T20:06:26Z

+  // can recursively scalarize it. A POD-typed `llzk.nondet` would no longer be visible to the
+  // allocation-based scalarization fixpoint.
+  if (auto podType = llvm::dyn_cast<PodType>(slot.elemType)) {
+    return builder.create<NewPodOp>(getLoc(), podType);


Preserve affine instantiations on the nested allocation

When the uninitialized nested POD contains affine-parameterized array or struct records, the outer single-record pod.new carries the required map operands, but this builder overload creates the replacement NewPodOp with empty mapOperands and numDimsPerMap. NewPodOp::verifyAffineMapOperands requires one instantiation group per recursively nested affine map, so llzk-pod-to-scalar produces invalid IR and fails verification for these valid inputs instead of scalarizing them; propagate the original allocation's map operand groups and dimension counts to the nested allocation.

Useful? React with 👍 / 👎.

Fixed in 1cb6895. The nested default now forwards the outer single-record allocation affine-map operand groups and dimension counts. Added an affine nested-POD regression to late_pod_nondet.llzk.

chatgpt-codex-connector · 2026-09-22T20:06:26Z

+  // can recursively scalarize it. A POD-typed `llzk.nondet` would no longer be visible to the
+  // allocation-based scalarization fixpoint.
+  if (auto podType = llvm::dyn_cast<PodType>(slot.elemType)) {
+    return builder.create<NewPodOp>(getLoc(), podType);


Preserve copy semantics for repeated nested-record reads

When an uninitialized nested record is read more than once and one returned copy is mutated, mem2reg replaces every pre-store read of the outer slot with this same NewPodOp value. POD reads are documented as value copies rather than aliases, so code that reads %outer[@inner] twice, writes a leaf through the first result, and then reads that leaf through the second must still observe the original nondeterministic value; after this change, later SROA/mem2reg rounds see one shared allocation and incorrectly forward the write to the second copy. Materialize independent snapshots for POD-valued reads or reject this mutable-copy case rather than using one allocation as the shared default.

Useful? React with 👍 / 👎.

Fixed in 1cb6895. getPromotableSlots now declines mem2reg promotion when a POD-valued read is mutated, preserving independent value-copy semantics. Added a focused two-read and one-write mem2reg regression.

chatgpt-codex-connector · 2026-09-22T20:06:26Z

+  // can recursively scalarize it. A POD-typed `llzk.nondet` would no longer be visible to the
+  // allocation-based scalarization fixpoint.
+  if (auto podType = llvm::dyn_cast<PodType>(slot.elemType)) {
+    return builder.create<NewPodOp>(getLoc(), podType);


Erase an unused nested default after promotion

When --mem2reg promotes an outer POD whose nested-record read has no live users, this newly created default becomes unused, but handlePromotionComplete only erases unused defaults whose defining op is llzk.nondet. The original outer allocation is erased while this effectful pod.new remains, so a standalone mem2reg invocation now leaves a dead allocation that the previous nondeterministic-default path explicitly removed; include NewPodOp in the unused-default cleanup (the PodToScalar pipeline's later cleanup masks this only for that pipeline).

Useful? React with 👍 / 👎.

Fixed in 1cb6895. Promotion cleanup now erases an unused nested pod.new default as well as llzk.nondet. Added a standalone mem2reg regression verifying no dead allocation remains.

chatgpt-codex-connector · 2026-09-22T20:06:30Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-23T15:45:57.291265Z	`8ee913d`	Manual request

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

shankarapailoor · 2026-09-22T22:48:03Z

@codex review

chatgpt-codex-connector · 2026-09-22T22:52:28Z

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 1d925a1d8a

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

tim-hoffman · 2026-09-23T00:14:05Z

+// CHECK-LABEL: module attributes {llzk.lang} {
+// CHECK-NEXT:    function.def @preserve_mutated_read_copy(%arg0: index) -> index {
+// CHECK-NEXT:      %pod = pod.new : <[@inner: !pod.type<[@value: index]>]>
+// CHECK-NEXT:      %0 = pod.read %pod[@inner] : <[@inner: !pod.type<[@value: index]>]>, !pod.type<[@value: index]>
+// CHECK-NEXT:      %1 = pod.read %pod[@inner] : <[@inner: !pod.type<[@value: index]>]>, !pod.type<[@value: index]>
+// CHECK-NEXT:      pod.write %0[@value] = %arg0 : <[@value: index]>, index
+// CHECK-NEXT:      %2 = pod.read %1[@value] : <[@value: index]>, index
+// CHECK-NEXT:      function.return %2 : index
+// CHECK-NEXT:    }
+// CHECK-NEXT:  }


use lit variables as generated by the python script

Fixed in fc8850c. The nested POD checks now use the FileCheck variables emitted by scripts/generate-test-checks.py while keeping the checks minimized and omitting the autogenerated disclaimer. The focused regression and full check-lit suite pass.

shankarapailoor · 2026-09-23T05:31:39Z

Addressed the latest external review in 0a468ed:

Nested default pod.new creation now uses an insertion guard and is placed immediately before the original allocation, so block-local affine map operands dominate it.
The transitive read-only proof now follows array-valued POD reads and array.extract results through ArrayAccessOpInterface; array writes/inserts, forwarding, escapes, and unrecognized aggregates reject promotion.
Added regressions for a block-local arith.constant affine operand and mutation through a copied extracted subarray.
Added the map-order identity rationale for the single-record case.

I did not broaden the promotion gate to every flat aggregate record in this PR; that is the larger pre-existing case. I also checked the suggested plain --mem2reg positive cascade: it advances one nested level rather than fully scalarizing the chain in one run, while the PodToScalar driver supplies the repeated rounds, so I did not add a test asserting otherwise.

Focused checks, full check-lit, and nix build -L pass.

shankarapailoor · 2026-09-23T12:10:56Z

@codex review

chatgpt-codex-connector · 2026-09-23T12:15:02Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 0a468ed00a

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

shankarapailoor · 2026-09-23T14:13:17Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52d4836b69

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

chatgpt-codex-connector · 2026-09-23T14:16:55Z

+      } else if (auto writeOp = llvm::dyn_cast<WritePodOp>(user)) {
+        if (writeOp.getPodRef() == aggregateValue) {
+          if (!user->isBeforeInBlock(ownedRead)) {
+            return false;
+          }


Reject untransferred mutations of earlier snapshots

When an earlier snapshot is mutated before ownedRead but that changed record is not subsequently transferred into ownedValue, this branch still declares promotion safe. For example, snapshot = read outer; write snapshot.value = replacement; owner = read outer; write outer.inner = owner; read owner.value must return the original nondeterministic value because POD reads are copies, but promotion aliases both reads to the same nested pod.new, so the later scalarization returns replacement. Only permit a mutation of an earlier snapshot when the analysis proves the mutated aggregate is copied into the owner before writeback; otherwise decline promotion.

Useful? React with 👍 / 👎.

Fixed in 82f208b. Snapshot-use analysis now rejects a directly mutated aggregate unless that aggregate is explicitly transferred into the owned value before the first writeback. I added a regression for the untransferred earlier-snapshot case while retaining the positive Circom-style array mutation/transfer coverage. Focused FileCheck tests and the full nix build -L (including all 445 lit tests and unit tests) pass.

shankarapailoor · 2026-09-23T15:40:35Z

@codex review.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ee913d6ad

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

chatgpt-codex-connector · 2026-09-23T15:45:54Z

+  return llvm::all_of(reads, [ownedRead, ownedValue, firstWriteback](ReadPodOp readOp) {
+    return readOp == ownedRead ||
+           hasOnlySnapshotUses(readOp.getResult(), ownedRead, ownedValue, firstWriteback);
+  });


Reject promotions that alias interacting snapshots

When there are multiple snapshots before ownedRead, this checks each snapshot independently and misses interactions between them. If snapshot A mutates a nested leaf and later transfers that leaf into ownedValue, while snapshot B reads the same leaf after A's mutation but before ownedRead, both snapshots pass this predicate; mem2reg then replaces A and B with the same default NewPodOp, so B observes A's mutation even though pod.read returns an independent value copy. Reject such promotion whenever another snapshot is observed after an accepted snapshot mutation, or model the cross-snapshot ordering explicitly.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-09-23T15:45:54Z

+    return builder.create<NewPodOp>(
+        getLoc(), podType, mapOperands, getNumDimsPerMapAttr(), InitializedRecords {}
+    );


Keep affine nested defaults destructurable

For an uninitialized outer record whose nested POD has multiple records and contains an affine-sized array, this creates a multi-record pod.new with nonempty map operands. NewPodOp::getDestructurableSlots() rejects every such allocation when getMapOperands() is nonempty, while mem2reg rejects it because it has more than one record, so the scalarization loop reaches a fixed point with residual POD IR and --llzk-pod-to-scalar fails on otherwise valid input. The affine operand groups need to remain available while allowing this nested allocation to be split record-by-record.

Useful? React with 👍 / 👎.

shankarapailoor · 2026-09-23T15:51:54Z

I'm going to close this PR as I think pod-to-scalar needs a more fundamental refactor because this stuff is getting too complicated.

Scalarize uninitialized nested POD records

ec16dab

shankarapailoor requested a review from a team as a code owner September 22, 2026 19:35

Merge branch 'main' into shankara/pod-nondet-fix

0d0b8c5

tim-hoffman reviewed Sep 22, 2026

View reviewed changes

Comment thread test/Transforms/PodToScalar/late_pod_nondet.llzk Outdated

shankarapailoor added 2 commits September 22, 2026 12:58

Move POD regression checks after input

3da673b

Merge branch 'main' into shankara/pod-nondet-fix

cb7dc09

shankarapailoor requested a review from tim-hoffman September 22, 2026 20:02

chatgpt-codex-connector Bot reviewed Sep 22, 2026

View reviewed changes

shankarapailoor added 2 commits September 22, 2026 14:03

Preserve nested POD promotion semantics

1cb6895

Conservatively promote read-only nested PODs

1d925a1

tim-hoffman requested changes Sep 23, 2026

View reviewed changes

shankarapailoor added 2 commits September 22, 2026 22:10

Use FileCheck variables for nested POD tests

fc8850c

Fix nested POD default dominance and array copies

0a468ed

shankarapailoor added 2 commits September 23, 2026 05:25

Merge branch 'main' into shankara/pod-nondet-fix

5722aad

Scalarize owned nested POD writebacks

52d4836

Reorder nested POD FileCheck assertions

f2f66bf

chatgpt-codex-connector Bot reviewed Sep 23, 2026

View reviewed changes

shankarapailoor added 2 commits September 23, 2026 08:28

Reject untransferred snapshot mutations

82f208b

Use explicit lambda captures in POD promotion

8ee913d

chatgpt-codex-connector Bot reviewed Sep 23, 2026

View reviewed changes

shankarapailoor closed this Sep 23, 2026

Conversation

shankarapailoor commented Sep 22, 2026

Summary

Related issues

Changes

Testing

Submission checklist

AI assistance

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

tim-hoffman commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

shankarapailoor commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 22, 2026

Uh oh!

tim-hoffman Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor commented Sep 23, 2026

Uh oh!

shankarapailoor commented Sep 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026

Uh oh!

shankarapailoor commented Sep 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

shankarapailoor commented Sep 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Sep 23, 2026

github-actions Bot commented Sep 22, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 22, 2026 •

edited

Loading