Rewrite byte_extract from multi-dimensional array#8705
Rewrite byte_extract from multi-dimensional array#8705tautschnig merged 1 commit intodiffblue:developfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8705 +/- ##
========================================
Coverage 80.47% 80.48%
========================================
Files 1704 1704
Lines 188762 188774 +12
Branches 73 73
========================================
+ Hits 151908 151936 +28
+ Misses 36854 36838 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0ae1c7e to
9a19114
Compare
|
Now includes a minified version of the sample provided in #8796 to confirm the desired effect of the simplification improvement. |
15e460c to
4f514db
Compare
|
Can we push this to the top of the queue to be reviewed please? This is blocking progress on mldsa-native. |
4f514db to
81a05fd
Compare
|
Regression testing with mlkem-native and mldsa-native (all parameter sets) is good at the HEAD of this branch now. I have also tested the specific test case added for Issue #8796 with good results. Please rebase and merge. |
81a05fd to
a6572b2
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves handling of byte-level extraction and array rewrites for multi-dimensional arrays, and fixes SMT array-theory behavior for let-bound arrays by ensuring fresh let symbols remain connected to their bound arrays.
Changes:
- Enhance
get_subexpression_at_offsetto better decompose scaled offsets for multi-dimensional array layouts. - In the bit-blaster, record array equalities for
letbindings over unbounded arrays so element constraints propagate through the fresh symbol. - Add regression tests for SMT2
letover arrays and a DFCC performance/regression scenario involving multi-dimensional arrays.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/pointer_offset_size.cpp | Adds a new decomposition path for scaled offsets into multi-dimensional arrays; factors out elem_size_bytes. |
| src/solvers/flattening/boolbv_let.cpp | Adds element-wise connection between fresh let symbols and bound unbounded arrays via recorded array equality. |
| regression/smt2_solver/let-array/let-array.smt2 | New SMT2 regression demonstrating required propagation through let-bound arrays. |
| regression/smt2_solver/let-array/let-array.desc | Expected output for the new SMT2 regression. |
| regression/contracts-dfcc/multi_dimensional_array_performance/test.desc | New DFCC regression entry for the multi-dimensional array scenario. |
| regression/contracts-dfcc/multi_dimensional_array_performance/program.desc | Checks produced program output matches expected byte_extract pattern. |
| regression/contracts-dfcc/multi_dimensional_array_performance/main.c | New C harness intended to exercise the multi-dimensional array rewrite path under DFCC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a6572b2 to
fbfe16e
Compare
diffblue/cbmc#8705 Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
diffblue/cbmc#8705 Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
As a follow-up to 78839a9: add support for rewriting multiple-of-element size access to arrays when working with multi-dimensional arrays. Fixes: diffblue#8796
fbfe16e to
d5b671e
Compare
When byte_extract is applied to a union-typed expression with a non-constant computed offset, the byte_extract lowering creates a massive expression because it must consider all possible byte layouts. For unions where the widest member covers the entire union, we can instead decompose the access through that member, avoiding the expensive lowering. Add union handling to the non-constant-offset overload of get_subexpression_at_offset: for symbol and member expressions of union type, recurse into the widest member. Guard this on the widest member's size equalling the union's size (no trailing padding). The constant-offset overload is left unchanged to preserve existing simplification behavior (e.g., byte_extract(byte_update(...)) patterns used during constant propagation). On the reproducer from diffblue#8813, the union version now takes 1.0s (previously 94s), matching the struct version at 1.0s. On the union_performance2 test (poly vec[8] with int32_t coeffs[256], accessed through a union with --z3), the union version completes in 0.08s matching the struct equivalent. Without this fix combined with the multi-dimensional array rewrite from diffblue#8705, the byte_extract lowering would not terminate within reasonable time. Fixes: diffblue#8813 Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
diffblue/cbmc#8705 Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
As a follow-up to 78839a9: add support for rewriting multiple-of-element size access to arrays when working with multi-dimensional arrays.
Fixes: #8796