Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new MIR rvalue to load a single element from a constant array via code/data copy, avoiding materializing the entire const array in memory for indexed reads (especially beneficial in loops and for large arrays).
Changes:
- Add
Rvalue::ConstArrayElemLoadand thread it through MIR lowering, transforms, hashing/formatting, and borrowck. - Emit per-element
CODECOPYfor const-array indexing in both Yul and Sonatina backends. - Add/refresh fixtures + snapshots covering looped indexing and signed loads.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mir/src/transform/lower_capability_to_repr.rs | Rewrites dynamic index value roots inside ConstArrayElemLoad. |
| crates/mir/src/transform.rs | Stabilization, ZST canonicalization, and use-count tracking updated for ConstArrayElemLoad. |
| crates/mir/src/lower/prepass.rs | Avoid eagerly lowering const array paths; add const_array_data_for_expr for element-load path. |
| crates/mir/src/lower/mod.rs | Capability-space + “first unlowered expr” tracking updated for new rvalue. |
| crates/mir/src/lower/expr.rs | Emit ConstArrayElemLoad for indexing const array paths (when eligible). |
| crates/mir/src/ir.rs | Introduce Rvalue::ConstArrayElemLoad in MIR. |
| crates/mir/src/hash.rs | Hashing support for ConstArrayElemLoad. |
| crates/mir/src/fmt.rs | Formatting support for ConstArrayElemLoad. |
| crates/mir/src/analysis/borrowck.rs | Borrowck/value-operand/local-use plumbing updated for ConstArrayElemLoad. |
| crates/fe/tests/fixtures/fe_test/const_array_large_loop.fe | New Fe-level regression fixture for large const arrays + loop/index + signed loads. |
| crates/codegen/tests/fixtures/sonatina_ir/const_nested_u8_array.snap | Snapshot update reflecting reduced const-array materialization. |
| crates/codegen/tests/fixtures/sonatina_ir/const_array_loop.snap | New Sonatina IR snapshot for per-element const array loads in a loop. |
| crates/codegen/tests/fixtures/sonatina_ir/const_array_branch.snap | Snapshot update: per-element copy instead of full-array copy. |
| crates/codegen/tests/fixtures/sonatina_ir/const_array_addmod.snap | Snapshot update: per-element copy instead of full-array copy. |
| crates/codegen/tests/fixtures/sonatina_ir/const_array_add.snap | Snapshot update: per-element copy instead of full-array copy. |
| crates/codegen/tests/fixtures/sonatina_ir/const_array.snap | Snapshot update: fewer full-array copies; per-element loads. |
| crates/codegen/tests/fixtures/const_nested_u8_array.snap | Yul snapshot update reflecting reduced const-array materialization. |
| crates/codegen/tests/fixtures/const_array_loop.snap | New Yul snapshot for per-element const array loads in a loop. |
| crates/codegen/tests/fixtures/const_array_loop.fe | New codegen fixture that exercises repeated dynamic indexing. |
| crates/codegen/tests/fixtures/const_array_branch.snap | Yul snapshot update: fixed-size datacopy for indexed element. |
| crates/codegen/tests/fixtures/const_array_addmod.snap | Yul snapshot update: fixed-size datacopy for indexed element. |
| crates/codegen/tests/fixtures/const_array_add.snap | Yul snapshot update: fixed-size datacopy for indexed element. |
| crates/codegen/tests/fixtures/const_array.snap | Yul snapshot update: fixed-size datacopy for indexed elements. |
| crates/codegen/src/yul/emitter/statements.rs | Add Yul lowering for ConstArrayElemLoad. |
| crates/codegen/src/yul/emitter/expr.rs | Expose apply_from_word_conversion for reuse by new lowering path. |
| crates/codegen/src/sonatina/lower.rs | Add Sonatina lowering for ConstArrayElemLoad + const-data global dedup helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ced8a1 to
a89364f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a89364f3b6
ℹ️ 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".
ae6c5a0 to
f85e8e3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f85e8e3124
ℹ️ 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: c3f655d52f
ℹ️ 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: c4836aa5b2
ℹ️ 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".
c4836aa to
c323a08
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c323a0813e
ℹ️ 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".
13a51cb to
c820427
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c820427629
ℹ️ 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".
c820427 to
6a14174
Compare
Previously, each read from a const array would copy the entire array into memory. Ok for small arrays, horrible for big ones. We now do a code copy for each element. Sometime later we'll decide what to do based on the size.
This includes a special case for so that arrays of u8 and bool are backed, but for now all other int sizes are stored as 32 byte words. This isn't ideal of course, and needs to be addressed with some properly defined layout rules. Strings also need layout attention.