Update llzk-pod-to-scalar to split pods within arrays to multiple arrays#565
Update llzk-pod-to-scalar to split pods within arrays to multiple arrays#565tim-hoffman wants to merge 37 commits into
llzk-pod-to-scalar to split pods within arrays to multiple arrays#565Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 477c5322c0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45632675f9
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca87a4e2b6
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR updates the llzk-pod-to-scalar transformation to handle pod.type values that appear as array element types by splitting them into parallel leaf arrays (one per POD leaf record). This aligns pod-to-scalar with the previously unsupported case “pods within arrays”, and adjusts transformation pipeline ordering + adds extensive lit coverage.
Changes:
- Extend
PodToScalarPassto split arrays-of-POD into parallel arrays across relevant ops (array ops, constrain ops, function signatures/calls/returns, bool quantifiers, struct member accesses) and then continue POD scalarization. - Introduce
flattenArrayElementTypeas a shared utility and reuse it from the polymorphic flattening pass. - Add/refresh many
test/Transforms/PodToScalar/*lit tests covering nested POD arrays, affine shapes, containment/equality, quantifiers, and attribute propagation; update pipeline ordering to run pod-to-scalar before array-to-scalar.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Transforms/PodToScalar/static_unwritten_nested_pod_array.llzk | New coverage for static array-of-POD unwritten field handling and struct member splitting. |
| test/Transforms/PodToScalar/return_array_from_pod_field.llzk | New coverage for returning array-of-POD fields becoming multiple leaf arrays in function results. |
| test/Transforms/PodToScalar/nonstatic_nested_pod_array.llzk | New coverage for dynamic array-of-POD creation and struct member splitting. |
| test/Transforms/PodToScalar/member_with_nested_pod_array.llzk | New coverage for splitting nested POD arrays in struct members and access patterns. |
| test/Transforms/PodToScalar/function_signatures_with_nested_pod_array.llzk | New coverage for argument/result name attr propagation through POD-array signature splitting. |
| test/Transforms/PodToScalar/function_result_attrs_array_of_pod.llzk | New coverage for function result attrs when returning arrays-of-POD. |
| test/Transforms/PodToScalar/function_calls_with_pod.llzk | Update existing expectations for calls involving arrays-of-POD after new splitting behavior. |
| test/Transforms/PodToScalar/constrain_array_of_pod.llzk | New coverage for constrain.eq / constrain.in behavior on arrays-of-POD after splitting. |
| test/Transforms/PodToScalar/circom_decomp_prod.llzk | Large golden update verifying the new splitting logic in a realistic “circom” module. |
| test/Transforms/PodToScalar/bool_quantifiers.llzk | New coverage for lowering quantifiers over arrays-of-POD after splitting. |
| test/Transforms/PodToScalar/array_write_direct_pod_dynamic_nested_array.llzk | New coverage for writing POD elements into arrays where POD contains dynamic nested arrays. |
| test/Transforms/PodToScalar/array_read_from_unwritten_pod_field.llzk | New coverage for reads from unwritten dynamic array-of-POD fields. |
| test/Transforms/PodToScalar/array_read_from_pod_field.llzk | New coverage for array reads returning POD elements, now split into leaf arrays. |
| test/Transforms/PodToScalar/array_new_affine_leaf_array.llzk | New coverage for affine-instantiated leaf arrays within POD arrays and wildcard fallback behavior. |
| test/Transforms/PodToScalar/array_new_affine_leaf_array_conflict.llzk | New diagnostic coverage for conflicting affine instantiations across POD-array elements. |
| test/Transforms/PodToScalar/array_length.llzk | New coverage ensuring array.len rank semantics are preserved after POD-array splitting. |
| test/Transforms/PodToScalar/array_leaf_in_pod_array.llzk | New coverage for POD arrays whose leaves include arrays + tag fields. |
| test/Transforms/PodToScalar/array_extract_insert.llzk | New coverage for extract/insert on arrays-of-POD becoming per-leaf extract/insert. |
| lib/Util/TypeHelper.cpp | Adds flattenArrayElementType helper shared by multiple transformations. |
| include/llzk/Util/TypeHelper.h | Declares flattenArrayElementType utility. |
| lib/Dialect/Polymorphic/Transforms/FlatteningPass.cpp | Switches to shared flattenArrayElementType helper. |
| lib/Transforms/LLZKTransformationPassPipelines.cpp | Reorders passes so pod-to-scalar runs before array-to-scalar. |
| lib/Dialect/POD/Transforms/PodToScalarPass.cpp | Main implementation: split arrays-of-POD to parallel arrays + integrate with POD scalarization pipeline. |
| include/llzk/Util/Walk.h | Minor lambda cleanup (avoid unused parameter warning). |
| include/llzk/Dialect/POD/Transforms/TransformationPasses.td | Updates pass documentation to reflect new splitting behavior + pass ordering guidance. |
| changelogs/unreleased/th__make_pod_to_scalar_split_arrays.yaml | Changelog entry for new behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac379dd4f7
ℹ️ 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".
No description provided.