Skip to content

Commit

Permalink
fix(ssa): Fix slice intrinsic handling in the capacity tracker (#4643)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves
#4395 (comment)

## Summary\*

We were not accurately accounting for situations where the slice
capacity tracker was expecting a capacity from slice intrinsic results.
I have added a case for handling the slice capacity of `AsSlice`.

I also tested this against the `aztec-packages` work and we can
successfully compile now.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
vezenovm authored Mar 26, 2024
1 parent 10f182a commit 1b50ce1
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ impl<'a> SliceCapacityTracker<'a> {
Intrinsic::SlicePopFront => (Some(1), results.len() - 1),
// The slice capacity of these intrinsics is not determined by the arguments of the function.
Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) => (None, 1),
Intrinsic::AsSlice => (Some(0), 1),
_ => return,
};
let result_slice = results[result_index];
Expand All @@ -90,6 +91,7 @@ impl<'a> SliceCapacityTracker<'a> {
self.compute_slice_capacity(*arg, slice_sizes);
}
}

if let Some(contents_capacity) = slice_sizes.get(&slice_contents) {
let new_capacity = *contents_capacity + 1;
slice_sizes.insert(result_slice, new_capacity);
Expand All @@ -102,9 +104,6 @@ impl<'a> SliceCapacityTracker<'a> {
.expect("ICE: Should have an argument index for slice intrinsics");
let slice_contents = arguments[argument_index];

// We do not decrement the size on intrinsics that could remove values from a slice.
// This is because we could potentially go back to the smaller slice and not fill in dummies.
// This pass should be tracking the potential max that a slice ***could be***
if let Some(contents_capacity) = slice_sizes.get(&slice_contents) {
let new_capacity = *contents_capacity - 1;
slice_sizes.insert(result_slice, new_capacity);
Expand All @@ -121,6 +120,15 @@ impl<'a> SliceCapacityTracker<'a> {
slice_sizes
.insert(result_slice, FieldElement::max_num_bytes() as usize);
}
Intrinsic::AsSlice => {
let argument_index = argument_index
.expect("ICE: Should have an argument index for AsSlice builtin");
let array_size = self
.dfg
.try_get_array_length(arguments[argument_index])
.expect("ICE: Should be have an array length for AsSlice input");
slice_sizes.insert(result_slice, array_size);
}
_ => {}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_capacity_tracker"
type = "bin"
authors = [""]
compiler_version = ">=0.26.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
expected = "10"
first = "10"
input = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Reference https://github.com/noir-lang/noir/issues/4395#issuecomment-2018948631
// for context.
// We were not accurately accounting for situations where the slice capacity tracker
// was expecting a capacity from slice intrinsic results.
fn main(expected: pub Field, first: Field, input: [Field; 20]) {
let mut hasher_slice = input.as_slice();
hasher_slice = hasher_slice.push_front(first);
assert(hasher_slice[0] == expected);
// We need a conditional based upon witnesses
// to force a store of the slice.
// If this successfully compiles it means we have stored
// the results of the slice intrinsics used above.
if expected as u32 > 10 {
hasher_slice[expected - 10] = 100;
} else {
hasher_slice[expected] = 100;
}
assert(hasher_slice[0] == expected);
}

0 comments on commit 1b50ce1

Please sign in to comment.