Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ssa_refactor)!: Add Slices #1728

Merged
merged 60 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
913bc9b
initial slices work on frontend
vezenovm Jun 16, 2023
5ec2c67
cargo clippy
vezenovm Jun 16, 2023
8a6f188
develop comment cleanup
vezenovm Jun 16, 2023
cf309a0
working push_back and len slices commands
vezenovm Jun 16, 2023
b442f88
cargo clippy
vezenovm Jun 16, 2023
5934644
cleanup
vezenovm Jun 16, 2023
f9d7921
fix clippy
vezenovm Jun 16, 2023
c253625
empty slice being processed
vezenovm Jun 16, 2023
990121e
cargo clippy
vezenovm Jun 16, 2023
2070e5e
Merge branch 'master' into mv/slices-2
vezenovm Jun 16, 2023
6717d9d
delete old comment
vezenovm Jun 16, 2023
8f17fda
remove unused import
vezenovm Jun 16, 2023
3366bb9
add enable_slices flag to avoid slices in old SSA
vezenovm Jun 16, 2023
c8dd8cd
add new SSA type for slices
vezenovm Jun 16, 2023
88261b3
missing Slice conversion
vezenovm Jun 16, 2023
0f3e9fd
Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
vezenovm Jun 16, 2023
e5e8eef
PR comments, fix slice/array subtyping
vezenovm Jul 3, 2023
fe29949
origin merge
vezenovm Jul 3, 2023
0f3fa79
hack to handle duplicate methods and mismatched types when compiling …
vezenovm Jul 3, 2023
fdd3266
cleanup enable_slices tech debt to be part of the NodeInterner
vezenovm Jul 3, 2023
18061d8
reference enable_slices flag issue
vezenovm Jul 3, 2023
1de2354
cleanup name
vezenovm Jul 3, 2023
eecd581
remove SSA Value::Slice
vezenovm Jul 3, 2023
b4960f9
merge master and use simplify_call method
vezenovm Jul 3, 2023
cc04a7a
remove dbg
vezenovm Jul 3, 2023
c5d9d9c
fix array len from slice params
vezenovm Jul 4, 2023
0be7871
remove old debug
vezenovm Jul 4, 2023
525cfd6
unwrap_array_element_type method in monomorphization pass
vezenovm Jul 4, 2023
a8033c8
cargo clippy
vezenovm Jul 4, 2023
b33dea2
mark issue in TODO comment for tech debt that removes slice module in…
vezenovm Jul 4, 2023
bc49ec3
Update crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs
vezenovm Jul 4, 2023
a51d70d
fix stdlib crate check
vezenovm Jul 4, 2023
901c973
Update noir_stdlib/src/slice.nr
vezenovm Jul 4, 2023
8fe073b
Update noir_stdlib/src/slice.nr
vezenovm Jul 4, 2023
64ecced
Update noir_stdlib/src/slice.nr
vezenovm Jul 4, 2023
2d14285
Update noir_stdlib/src/slice.nr
vezenovm Jul 4, 2023
5d8151f
PR comments and stdlib crate assert
vezenovm Jul 4, 2023
99bd62c
master merge
vezenovm Jul 4, 2023
b6fd1b8
Merge remote-tracking branch 'origin/mv/slices-2' into mv/slices-2
vezenovm Jul 4, 2023
dd3be16
cargo fmt
vezenovm Jul 4, 2023
b7f4bb2
keep enable slices check in expr type check
vezenovm Jul 5, 2023
4344361
increase timeout time
vezenovm Jul 5, 2023
f2b9cbb
increase timeout to 60m
vezenovm Jul 5, 2023
c47f7fb
back to 30m timeout
vezenovm Jul 5, 2023
ebf36fb
excldue 6_array from old ssa test and move 6_array into ssa_refactor …
vezenovm Jul 5, 2023
8493267
merge conflicts w/ master
vezenovm Jul 5, 2023
2938b47
unwrap_or_else for first_elem_type
vezenovm Jul 5, 2023
fa29eb4
only test new ssa :D
vezenovm Jul 6, 2023
34d6755
add back old ssa tests
vezenovm Jul 6, 2023
f0d0539
remove Vec type
vezenovm Jul 6, 2023
cd25582
remove Vec type check
vezenovm Jul 6, 2023
5e6bf97
Merge branch 'master' into mv/slices-2
vezenovm Jul 6, 2023
691a35b
remove Vec from stdlib as we have no primitive type until we add in V…
vezenovm Jul 6, 2023
f0f0a71
try w/ a pub input to 6_array
vezenovm Jul 6, 2023
b55a45a
im dumb and misplaced the pub
vezenovm Jul 6, 2023
80d28e0
master merge after no more driver
vezenovm Jul 7, 2023
b746a96
remove pub
vezenovm Jul 7, 2023
daac684
temp: remove unused tests for debugging
kevaundray Jul 7, 2023
6d1a45b
Revert "temp: remove unused tests for debugging"
kevaundray Jul 7, 2023
49c7610
Merge branch 'master' into mv/slices-2
jfecher Jul 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FileManager {
// Unwrap as we ensure that all file_id's map to a corresponding file in the file map
self.file_map.get_file(file_id).unwrap()
}
fn path(&mut self, file_id: FileId) -> &Path {
pub fn path(&mut self, file_id: FileId) -> &Path {
// Unwrap as we ensure that all file_ids are created by the file manager
// So all file_ids will points to a corresponding path
self.id_to_path.get(&file_id).unwrap().0.as_path()
Expand Down
4 changes: 2 additions & 2 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ fn on_code_lens_request(

// We ignore the warnings and errors produced by compilation for producing codelenses
// because we can still get the test functions even if compilation fails
let _ = check_crate(&mut state.context, false);
let _ = check_crate(&mut state.context, false, false);

let fm = &state.context.file_manager;
let files = fm.as_simple_files();
Expand Down Expand Up @@ -224,7 +224,7 @@ fn on_did_save_text_document(

let mut diagnostics = Vec::new();

let file_diagnostics = match check_crate(&mut state.context, false) {
let file_diagnostics = match check_crate(&mut state.context, false, false) {
Ok(warnings) => warnings,
Err(errors_and_warnings) => errors_and_warnings,
};
Expand Down
5 changes: 3 additions & 2 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn check_from_path<B: Backend>(
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings, compile_options.experimental_ssa)?;

// XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
if let Some((parameters, return_type)) = compute_function_signature(&context) {
Expand Down Expand Up @@ -214,7 +214,8 @@ d2 = ["", "", ""]
pub(crate) fn check_crate_and_report_errors(
context: &mut Context,
deny_warnings: bool,
enable_slices: bool,
) -> Result<(), ReportedErrors> {
let result = check_crate(context, deny_warnings).map(|warnings| ((), warnings));
let result = check_crate(context, deny_warnings, enable_slices).map(|warnings| ((), warnings));
super::compile_cmd::report_errors(result, context, deny_warnings)
}
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ mod tests {
let mut context = Context::default();
create_local_crate(&mut context, &root_file, CrateType::Binary);

let result = check_crate(&mut context, false);
let result = check_crate(&mut context, false, false);
let success = result.is_ok();

let errors = match result {
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn run_tests<B: Backend>(
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings, compile_options.experimental_ssa)?;

let test_functions = context.get_all_test_functions_in_crate_matching(&LOCAL_CRATE, test_name);
println!("Running {} test functions...", test_functions.len());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@ t = "10"

#7128
#15309
#16349

#16349
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,3 @@ fn main(x: [u32; 5], y: [u32; 5], mut z: u32, t: u32) {
}
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.6.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "5"
y = "10"
23 changes: 23 additions & 0 deletions crates/nargo_cli/tests/test_data_ssa_refactor/slices/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use dep::std::slice;

fn main(x : Field, y : pub Field) {

let mut slice: [Field] = [0; 2];

assert(slice[0] == 0);
assert(slice[0] != 1);
slice[0] = x;
assert(slice[0] == x);

let slice_plus_10 = slice.push_back(y);
assert(slice_plus_10[2] == 10);
assert(slice_plus_10[2] != 8);
assert(slice_plus_10.len() == 3);

let mut new_slice: [Field] = [];
jfecher marked this conversation as resolved.
Show resolved Hide resolved
for i in 0..5 {
new_slice = new_slice.push_back(i);
}
assert(new_slice.len() == 5);
}

7 changes: 5 additions & 2 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ pub fn propagate_dep(
pub fn check_crate(
context: &mut Context,
deny_warnings: bool,
enable_slices: bool,
) -> Result<Warnings, ErrorsAndWarnings> {
// Add the stdlib before we check the crate
// TODO: This should actually be done when constructing the driver and then propagated to each dependency when added;
Expand All @@ -167,6 +168,8 @@ pub fn check_crate(
let std_crate = create_non_local_crate(context, path_to_std_lib_file, CrateType::Library);
propagate_dep(context, std_crate, &CrateName::new(std_crate_name).unwrap());

context.def_interner.enable_slices = enable_slices;

let mut errors = vec![];
CrateDefMap::collect_defs(LOCAL_CRATE, context, &mut errors);

Expand Down Expand Up @@ -195,7 +198,7 @@ pub fn compile_main(
is_opcode_supported: &impl Fn(&Opcode) -> bool,
options: &CompileOptions,
) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
let warnings = check_crate(context, options.deny_warnings)?;
let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;

let main = match context.get_main_function(&LOCAL_CRATE) {
Some(m) => m,
Expand Down Expand Up @@ -226,7 +229,7 @@ pub fn compile_contracts(
is_opcode_supported: &impl Fn(&Opcode) -> bool,
options: &CompileOptions,
) -> Result<(Vec<CompiledContract>, Warnings), ErrorsAndWarnings> {
let warnings = check_crate(context, options.deny_warnings)?;
let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;

let contracts = context.get_all_contracts(&LOCAL_CRATE);
let mut compiled_contracts = vec![];
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ impl SsaContext {
Type::Function(..) => ObjectType::Function,
Type::Tuple(_) => todo!("Conversion to ObjectType is unimplemented for tuples"),
Type::String(_) => todo!("Conversion to ObjectType is unimplemented for strings"),
Type::Vec(_) => todo!("Conversion to ObjectType is unimplemented for Vecs"),
Type::Slice(_) => todo!("Conversion to ObjectType is unimplemented for slices"),
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl Value {
Type::Unit
| Type::Function(..)
| Type::Array(..)
| Type::Vec(..)
| Type::Slice(..)
| Type::String(..)
| Type::Integer(..)
| Type::Bool
Expand Down
8 changes: 8 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,9 @@ impl Context {
(_, Type::Array(..)) | (Type::Array(..), _) => {
unreachable!("Arrays are invalid in binary operations")
}
(_, Type::Slice(..)) | (Type::Slice(..), _) => {
unreachable!("Arrays are invalid in binary operations")
}
// If either side is a Field constant then, we coerce into the type
// of the other operand
(Type::Numeric(NumericType::NativeField), typ)
Expand Down Expand Up @@ -732,6 +735,7 @@ impl Context {

Self::convert_vars_to_values(out_vars, dfg, result_ids)
}
_ => todo!("expected a black box function"),
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -743,6 +747,10 @@ impl Context {
assert_eq!(elements.len(), 1);
(&elements[0]).into()
}
Type::Slice(elements) => {
assert_eq!(elements.len(), 1);
(&elements[0]).into()
}
_ => unreachable!("Expected array type"),
}
}
Expand Down
24 changes: 22 additions & 2 deletions crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,16 @@ impl DataFlowGraph {

/// Gets or creates a ValueId for the given Intrinsic.
pub(crate) fn import_intrinsic(&mut self, intrinsic: Intrinsic) -> ValueId {
if let Some(existing) = self.intrinsics.get(&intrinsic) {
if let Some(existing) = self.get_intrinsic(intrinsic) {
return *existing;
}
self.values.insert(Value::Intrinsic(intrinsic))
let intrinsic_value_id = self.values.insert(Value::Intrinsic(intrinsic));
self.intrinsics.insert(intrinsic, intrinsic_value_id);
intrinsic_value_id
}

pub(crate) fn get_intrinsic(&mut self, intrinsic: Intrinsic) -> Option<&ValueId> {
self.intrinsics.get(&intrinsic)
}

/// Attaches results to the instruction, clearing any previous results.
Expand Down Expand Up @@ -360,6 +366,20 @@ impl DataFlowGraph {
}
}

/// Returns the Type::Array associated with this ValueId if it refers to an array parameter.
/// Otherwise, this returns None.
pub(crate) fn get_array_parameter_type(
&self,
value: ValueId,
) -> Option<(Rc<CompositeType>, usize)> {
match &self.values[self.resolve(value)] {
Value::Param { typ: Type::Array(element_type, size), .. } => {
Some((element_type.clone(), *size))
}
_ => None,
}
}

/// Sets the terminator instruction for the given basic block
pub(crate) fn set_block_terminator(
&mut self,
Expand Down
62 changes: 49 additions & 13 deletions crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub(crate) type InstructionId = Id<Instruction>;
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub(crate) enum Intrinsic {
Sort,
ArrayLen,
jfecher marked this conversation as resolved.
Show resolved Hide resolved
SlicePushBack,
Println,
ToBits(Endian),
ToRadix(Endian),
Expand All @@ -43,6 +45,8 @@ impl std::fmt::Display for Intrinsic {
match self {
Intrinsic::Println => write!(f, "println"),
Intrinsic::Sort => write!(f, "arraysort"),
Intrinsic::ArrayLen => write!(f, "array_len"),
Intrinsic::SlicePushBack => write!(f, "slice_push_back"),
Intrinsic::ToBits(Endian::Big) => write!(f, "to_be_bits"),
Intrinsic::ToBits(Endian::Little) => write!(f, "to_le_bits"),
Intrinsic::ToRadix(Endian::Big) => write!(f, "to_be_radix"),
Expand All @@ -59,6 +63,8 @@ impl Intrinsic {
match name {
"println" => Some(Intrinsic::Println),
"arraysort" => Some(Intrinsic::Sort),
"array_len" => Some(Intrinsic::ArrayLen),
"slice_push_back" => Some(Intrinsic::SlicePushBack),
"to_le_radix" => Some(Intrinsic::ToRadix(Endian::Little)),
"to_be_radix" => Some(Intrinsic::ToRadix(Endian::Big)),
"to_le_bits" => Some(Intrinsic::ToBits(Endian::Little)),
Expand Down Expand Up @@ -276,7 +282,6 @@ impl Instruction {
Instruction::ArrayGet { array, index } => {
let array = dfg.get_array_constant(*array);
let index = dfg.get_numeric_constant(*index);

if let (Some((array, _)), Some(index)) = (array, index) {
let index =
index.try_to_u64().expect("Expected array index to fit in u64") as usize;
Expand All @@ -290,7 +295,6 @@ impl Instruction {
Instruction::ArraySet { array, index, value } => {
let array = dfg.get_array_constant(*array);
let index = dfg.get_numeric_constant(*index);

if let (Some((array, element_type)), Some(index)) = (array, index) {
let index =
index.try_to_u64().expect("Expected array index to fit in u64") as usize;
Expand Down Expand Up @@ -375,23 +379,55 @@ fn simplify_call(func: ValueId, arguments: &[ValueId], dfg: &mut DataFlowGraph)
Value::Intrinsic(intrinsic) => *intrinsic,
_ => return None,
};

let constant_args: Option<Vec<_>> =
arguments.iter().map(|value_id| dfg.get_numeric_constant(*value_id)).collect();
let constant_args = match constant_args {
Some(constant_args) => constant_args,
Option::None => return None,
};

match intrinsic {
Intrinsic::ToBits(endian) => {
let field = constant_args[0];
let limb_count = constant_args[1].to_u128() as u32;
SimplifiedTo(constant_to_radix(endian, field, 2, limb_count, dfg))
if let Some(constant_args) = constant_args {
let field = constant_args[0];
let limb_count = constant_args[1].to_u128() as u32;
SimplifiedTo(constant_to_radix(endian, field, 2, limb_count, dfg))
} else {
None
}
}
Intrinsic::ToRadix(endian) => {
let field = constant_args[0];
let radix = constant_args[1].to_u128() as u32;
let limb_count = constant_args[2].to_u128() as u32;
SimplifiedTo(constant_to_radix(endian, field, radix, limb_count, dfg))
if let Some(constant_args) = constant_args {
let field = constant_args[0];
let radix = constant_args[1].to_u128() as u32;
let limb_count = constant_args[2].to_u128() as u32;
SimplifiedTo(constant_to_radix(endian, field, radix, limb_count, dfg))
} else {
None
}
}
Intrinsic::ArrayLen => {
let slice = dfg.get_array_constant(arguments[0]);
if let Some((slice, _)) = slice {
let slice_len =
dfg.make_constant(FieldElement::from(slice.len() as u128), Type::field());
SimplifiedTo(slice_len)
} else if let Some((_, slice_len)) = dfg.get_array_parameter_type(arguments[0]) {
let slice_len = dfg.make_constant(
FieldElement::from(slice_len as u128),
Type::Numeric(NumericType::NativeField),
);
SimplifiedTo(slice_len)
} else {
None
}
}
Intrinsic::SlicePushBack => {
let slice = dfg.get_array_constant(arguments[0]);
if let (Some((mut slice, element_type)), elem) = (slice, arguments[1]) {
slice.push_back(elem);
let new_slice = dfg.make_array(slice, element_type);
SimplifiedTo(new_slice)
} else {
None
}
}
Intrinsic::BlackBox(_) | Intrinsic::Println | Intrinsic::Sort => None,
}
Expand Down
7 changes: 7 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ pub(crate) enum Type {
/// An immutable array value with the given element type and length
Array(Rc<CompositeType>, usize),

/// An immutable slice value with a given element type
Slice(Rc<CompositeType>),

/// A function that may be called directly
Function,
}
Expand Down Expand Up @@ -74,6 +77,10 @@ impl std::fmt::Display for Type {
let elements = vecmap(element.iter(), |element| element.to_string());
write!(f, "[{}; {length}]", elements.join(", "))
}
Type::Slice(element) => {
let elements = vecmap(element.iter(), |element| element.to_string());
write!(f, "[{}]", elements.join(", "))
}
Type::Function => write!(f, "function"),
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ impl<'f> Context<'f> {
then_value,
else_value,
),
Type::Slice(_) => panic!("Cannot return slices from an if expression"),
Type::Reference => panic!("Cannot return references from an if expression"),
Type::Function => panic!("Cannot return functions from an if expression"),
}
Expand Down
9 changes: 4 additions & 5 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,15 @@ impl<'a> FunctionContext<'a> {
ast::Type::Unit => panic!("convert_non_tuple_type called on a unit type"),
ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"),
ast::Type::Function(_, _) => Type::Function,
ast::Type::Slice(element) => {
let element_types = Self::convert_type(element).flatten();
Type::Slice(Rc::new(element_types))
}
ast::Type::MutableReference(element) => {
// Recursive call to panic if element is a tuple
Self::convert_non_tuple_type(element);
Type::Reference
}

// How should we represent Vecs?
// Are they a struct of array + length + capacity?
// Or are they just references?
ast::Type::Vec(_) => Type::Reference,
}
}

Expand Down
Loading