Skip to content

Commit

Permalink
fix: Ssa typing for array & slice indexes (noir-lang#4278)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Partial work towards noir-lang#4275

## Summary\*

We were mixing Field and u64 types when accessing arrays. Since we index
arrays by u64s, always cast array/slice addressing values to u64.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
sirasistant authored Feb 7, 2024
1 parent 7e139de commit 4074bab
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 19 deletions.
5 changes: 5 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,11 @@ impl<'a> FunctionContext<'a> {
address
}

/// Array indexes are u64s. This function casts values used as indexes to u64.
pub(super) fn make_array_index(&mut self, index: ValueId) -> ValueId {
self.builder.insert_cast(index, Type::unsigned(64))
}

/// Define a local variable to be some Values that can later be retrieved
/// by calling self.lookup(id)
pub(super) fn define(&mut self, id: LocalId, value: Values) {
Expand Down
26 changes: 7 additions & 19 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ use noirc_frontend::{

use crate::{
errors::{InternalError, RuntimeError},
ssa::{
function_builder::data_bus::DataBusBuilder,
ir::{instruction::Intrinsic, types::NumericType},
},
ssa::{function_builder::data_bus::DataBusBuilder, ir::instruction::Intrinsic},
};

use self::{
Expand Down Expand Up @@ -390,8 +387,9 @@ impl<'a> FunctionContext<'a> {
length: Option<super::ir::value::ValueId>,
) -> Result<Values, RuntimeError> {
// base_index = index * type_size
let index = self.make_array_index(index);
let type_size = Self::convert_type(element_type).size_of_type();
let type_size = self.builder.field_constant(type_size as u128);
let type_size = self.builder.numeric_constant(type_size as u128, Type::unsigned(64));
let base_index =
self.builder.set_location(location).insert_binary(index, BinaryOp::Mul, type_size);

Expand Down Expand Up @@ -428,20 +426,10 @@ impl<'a> FunctionContext<'a> {
index: super::ir::value::ValueId,
length: Option<super::ir::value::ValueId>,
) {
let array_len = length.expect("ICE: a length must be supplied for indexing slices");
// Check the type of the index value for valid comparisons
let array_len = match self.builder.type_of_value(index) {
Type::Numeric(numeric_type) => match numeric_type {
// If the index itself is an integer, keep the array length as a Field
NumericType::Unsigned { .. } | NumericType::Signed { .. } => array_len,
// If the index and the array length are both Fields we will not be able to perform a less than comparison on them.
// Thus, we cast the array length to a u64 before performing the less than comparison
NumericType::NativeField => self
.builder
.insert_cast(array_len, Type::Numeric(NumericType::Unsigned { bit_size: 64 })),
},
_ => unreachable!("ICE: array index must be a numeric type"),
};
let index = self.make_array_index(index);
// We convert the length as an array index type for comparison
let array_len = self
.make_array_index(length.expect("ICE: a length must be supplied for indexing slices"));

let is_offset_out_of_bounds = self.builder.insert_binary(index, BinaryOp::Lt, array_len);
let true_const = self.builder.numeric_constant(true, Type::bool());
Expand Down

0 comments on commit 4074bab

Please sign in to comment.