Repository navigation
fix(types): correct Felt252DictEntry and U96LimbsLessThanGuarantee layouts - #1671
Merged
Merged
Conversation
…youts, add a type layout check Two Sierra types reported a `TypeBuilder::layout` that disagreed with the MLIR type `TypeBuilder::build` emits for them. Codegen bakes `layout()` into array strides, enum payload offsets and argument marshalling while the generated code reads and writes memory according to the emitted LLVM type, so the mismatch let valid programs access aggregate elements at the wrong offsets. - `Felt252DictEntry<T>` is emitted as two pointers but its layout reported a felt252 followed by two pointers (48 bytes / align 16 instead of 16 / 8). An array or struct of entries then used the wrong stride and offsets. - `U96LimbsLessThanGuarantee<N>` is emitted as 2 * N u96 limbs but was declared zero-sized with an empty layout. Boxing it allocated zero bytes in the arena and the store clobbered the previous allocation, which right after `eval_circuit` is the outputs array. To stop this class of drift from recurring, add `find_type_layout_mismatches`, which builds every type a program declares, lowers it to LLVM IR and compares LLVM's ABI size and alignment against `layout()`, and a test that runs it over every Sierra snippet in the cairo repo's e2e libfunc test data. New types are covered as soon as any snippet declares them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
orizi
approved these changes
Oct 26, 2026
orizi
left a comment
Collaborator
There was a problem hiding this comment.
@orizi reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
11.048 ± 0.052 | 10.964 | 11.138 | 5.99 ± 0.05 |
cairo-native (embedded AOT) |
1.844 ± 0.014 | 1.819 | 1.862 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.872 ± 0.017 | 1.846 | 1.900 | 1.02 ± 0.01 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
534.4 ± 8.3 | 523.8 | 546.8 | 1.00 |
cairo-native (embedded AOT) |
1659.9 ± 11.1 | 1645.8 | 1681.7 | 3.11 ± 0.05 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1691.4 ± 16.1 | 1671.5 | 1712.5 | 3.17 ± 0.06 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.930 ± 0.017 | 4.892 | 4.952 | 2.84 ± 0.04 |
cairo-native (embedded AOT) |
1.734 ± 0.023 | 1.701 | 1.778 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.755 ± 0.011 | 1.741 | 1.780 | 1.01 ± 0.02 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.842 ± 0.023 | 4.814 | 4.887 | 2.92 ± 0.02 |
cairo-native (embedded AOT) |
1.658 ± 0.012 | 1.634 | 1.672 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.689 ± 0.019 | 1.665 | 1.724 | 1.02 ± 0.01 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
582.5 ± 9.0 | 570.2 | 596.7 | 1.00 |
cairo-native (embedded AOT) |
1703.0 ± 11.6 | 1687.4 | 1721.7 | 2.92 ± 0.05 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1737.5 ± 16.6 | 1718.8 | 1775.0 | 2.98 ± 0.05 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
482.8 ± 7.6 | 469.6 | 493.9 | 1.00 |
cairo-native (embedded AOT) |
1675.2 ± 12.0 | 1645.7 | 1688.3 | 3.47 ± 0.06 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1697.6 ± 11.0 | 1676.7 | 1715.5 | 3.52 ± 0.06 |
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two Sierra types reported a
TypeBuilder::layoutthat disagreed with the MLIR typeTypeBuilder::buildemits for them. Codegen bakeslayout()into array strides, enum payload offsets and argument marshalling, while the generated code reads and writes memory according to the emitted LLVM type. The mismatch let valid programs access aggregate elements at the wrong offsets.Felt252DictEntry<T>is emitted as two pointers but its layout reported afelt252followed by two pointers (48 bytes / align 16 instead of 16 / 8). An array or struct of entries then used the wrong stride and offsets.U96LimbsLessThanGuarantee<N>is emitted as2 * Nu96 limbs but was declared zero-sized with an empty layout. Boxing it allocated zero bytes in the arena and the store clobbered the previous allocation, which right aftereval_circuitis the outputs array.Regression guard
Both bugs are the same class:
buildandlayoutare maintained by hand and drifted apart. This addsfind_type_layout_mismatches, which builds every type a program declares, lowers it to LLVM IR and compares LLVM's ABI size and alignment againstlayout(). A new test runs it over every Sierra snippet in the cairo repo's e2e libfunc test data, so any type a libfunc touches is checked, and new types are covered as soon as any snippet declares them.Also factors the repeated LLVM target initialization into a shared
ffi::initialize_llvm_targetshelper.Testing
e2e_libfunc_type_layouts_match_llvmfails on the old layouts and passes with the fixes.🤖 Generated with Claude Code
This change is