From 4074babef6e25c0f723f2bc9b1b2c89302f8e0b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Wed, 7 Feb 2024 10:06:42 +0100 Subject: [PATCH] fix: Ssa typing for array & slice indexes (#4278) # Description ## Problem\* Partial work towards https://github.com/noir-lang/noir/issues/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. --- .../src/ssa/ssa_gen/context.rs | 5 ++++ .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 26 +++++-------------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 4dc355ae51f..e23a1dc3235 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -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) { diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 1758e1785d3..76fefae9864 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -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::{ @@ -390,8 +387,9 @@ impl<'a> FunctionContext<'a> { length: Option, ) -> Result { // 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); @@ -428,20 +426,10 @@ impl<'a> FunctionContext<'a> { index: super::ir::value::ValueId, length: Option, ) { - 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());