Skip to content

Commit

Permalink
Merge branch 'master' into tf/restructure-integration-tests
Browse files Browse the repository at this point in the history
* master:
  chore: clippy fix (#3561)
  chore: fix wrapping issue for constraints (#3590)
  feat: add support for tuple values in `noir_codegen` (#3592)
  chore: Remove concept of storage slot from the compiler (#3582)
  chore: Update ACIR artifacts (#3591)
  feat: export `CompiledCircuit` from codegened TS (#3589)
  fix: use 128 bits for constant bit shift (#3586)
  fix: Somewhat reduce mem2reg memory usage (#3572)
  fix: Remove quotes from println output (#3574)
  • Loading branch information
TomAFrench committed Nov 27, 2023
2 parents 0c75d16 + 70ee787 commit 377ffd3
Show file tree
Hide file tree
Showing 18 changed files with 100 additions and 97 deletions.
54 changes: 28 additions & 26 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,32 +282,34 @@ impl FunctionBuilder {
) -> ValueId {
let base = self.field_constant(FieldElement::from(2_u128));
let typ = self.current_function.dfg.type_of_value(lhs);
let (max_bit, pow) = if let Some(rhs_constant) =
self.current_function.dfg.get_numeric_constant(rhs)
{
// Happy case is that we know precisely by how many bits the the integer will
// increase: lhs_bit_size + rhs
let (rhs_bit_size_pow_2, overflows) =
2_u32.overflowing_pow(rhs_constant.to_u128() as u32);
if overflows {
let zero = self.numeric_constant(FieldElement::zero(), typ);
return InsertInstructionResult::SimplifiedTo(zero).first();
}
let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2 as u128), typ);
(bit_size + (rhs_constant.to_u128() as u32), pow)
} else {
// we use a predicate to nullify the result in case of overflow
let bit_size_var =
self.numeric_constant(FieldElement::from(bit_size as u128), typ.clone());
let overflow = self.insert_binary(rhs, BinaryOp::Lt, bit_size_var);
let one = self.numeric_constant(FieldElement::one(), Type::unsigned(1));
let predicate = self.insert_binary(overflow, BinaryOp::Eq, one);
let predicate = self.insert_cast(predicate, typ.clone());

let pow = self.pow(base, rhs);
let pow = self.insert_cast(pow, typ);
(FieldElement::max_num_bits(), self.insert_binary(predicate, BinaryOp::Mul, pow))
};
let (max_bit, pow) =
if let Some(rhs_constant) = self.current_function.dfg.get_numeric_constant(rhs) {
// Happy case is that we know precisely by how many bits the the integer will
// increase: lhs_bit_size + rhs
let (rhs_bit_size_pow_2, overflows) =
2_u128.overflowing_pow(rhs_constant.to_u128() as u32);
if overflows {
assert!(bit_size < 128, "ICE - shift left with big integers are not supported");
if bit_size < 128 {
let zero = self.numeric_constant(FieldElement::zero(), typ);
return InsertInstructionResult::SimplifiedTo(zero).first();
}
}
let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2), typ);
(bit_size + (rhs_constant.to_u128() as u32), pow)
} else {
// we use a predicate to nullify the result in case of overflow
let bit_size_var =
self.numeric_constant(FieldElement::from(bit_size as u128), typ.clone());
let overflow = self.insert_binary(rhs, BinaryOp::Lt, bit_size_var);
let one = self.numeric_constant(FieldElement::one(), Type::unsigned(1));
let predicate = self.insert_binary(overflow, BinaryOp::Eq, one);
let predicate = self.insert_cast(predicate, typ.clone());

let pow = self.pow(base, rhs);
let pow = self.insert_cast(pow, typ);
(FieldElement::max_num_bits(), self.insert_binary(predicate, BinaryOp::Mul, pow))
};

let instruction = Instruction::Binary(Binary { lhs, rhs: pow, operator: BinaryOp::Mul });
if max_bit <= bit_size {
Expand Down
19 changes: 8 additions & 11 deletions compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, collections::BTreeMap};
use std::borrow::Cow;

use crate::ssa::ir::{
function::Function,
Expand All @@ -19,27 +19,27 @@ pub(super) struct Block {
/// Maps a ValueId to the Expression it represents.
/// Multiple ValueIds can map to the same Expression, e.g.
/// dereferences to the same allocation.
pub(super) expressions: BTreeMap<ValueId, Expression>,
pub(super) expressions: im::OrdMap<ValueId, Expression>,

/// Each expression is tracked as to how many aliases it
/// may have. If there is only 1, we can attempt to optimize
/// out any known loads to that alias. Note that "alias" here
/// includes the original reference as well.
pub(super) aliases: BTreeMap<Expression, AliasSet>,
pub(super) aliases: im::OrdMap<Expression, AliasSet>,

/// Each allocate instruction result (and some reference block parameters)
/// will map to a Reference value which tracks whether the last value stored
/// to the reference is known.
pub(super) references: BTreeMap<ValueId, ReferenceValue>,
pub(super) references: im::OrdMap<ValueId, ReferenceValue>,

/// The last instance of a `Store` instruction to each address in this block
pub(super) last_stores: BTreeMap<ValueId, InstructionId>,
pub(super) last_stores: im::OrdMap<ValueId, InstructionId>,
}

/// An `Expression` here is used to represent a canonical key
/// into the aliases map since otherwise two dereferences of the
/// same address will be given different ValueIds.
#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)]
#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
pub(super) enum Expression {
Dereference(Box<Expression>),
ArrayElement(Box<Expression>),
Expand Down Expand Up @@ -111,10 +111,7 @@ impl Block {
}

fn invalidate_all_references(&mut self) {
for reference_value in self.references.values_mut() {
*reference_value = ReferenceValue::Unknown;
}

self.references.clear();
self.last_stores.clear();
}

Expand All @@ -137,7 +134,7 @@ impl Block {
}

// Keep only the references present in both maps.
let mut intersection = BTreeMap::new();
let mut intersection = im::OrdMap::new();
for (value_id, reference) in &other.references {
if let Some(existing) = self.references.get(value_id) {
intersection.insert(*value_id, existing.unify(*reference));
Expand Down
3 changes: 1 addition & 2 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,6 @@ fn resolve_globals(
let globals = vecmap(globals, |global| {
let module_id = ModuleId { local_id: global.module_id, krate: crate_id };
let path_resolver = StandardPathResolver::new(module_id);
let storage_slot = context.next_storage_slot(module_id);

let mut resolver = Resolver::new(
&mut context.def_interner,
Expand All @@ -662,7 +661,7 @@ fn resolve_globals(

context.def_interner.update_global(global.stmt_id, hir_stmt);

context.def_interner.push_global(global.stmt_id, name, global.module_id, storage_slot);
context.def_interner.push_global(global.stmt_id, name, global.module_id);

(global.file_id, global.stmt_id)
});
Expand Down
19 changes: 0 additions & 19 deletions compiler/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ pub struct Context {
/// A map of each file that already has been visited from a prior `mod foo;` declaration.
/// This is used to issue an error if a second `mod foo;` is declared to the same file.
pub visited_files: BTreeMap<fm::FileId, Location>,

/// Maps a given (contract) module id to the next available storage slot
/// for that contract.
pub storage_slots: BTreeMap<def_map::ModuleId, StorageSlot>,
}

#[derive(Debug, Copy, Clone)]
Expand All @@ -42,8 +38,6 @@ pub enum FunctionNameMatch<'a> {
Contains(&'a str),
}

pub type StorageSlot = u32;

impl Context {
pub fn new(file_manager: FileManager, crate_graph: CrateGraph) -> Context {
Context {
Expand All @@ -52,7 +46,6 @@ impl Context {
visited_files: BTreeMap::new(),
crate_graph,
file_manager,
storage_slots: BTreeMap::new(),
}
}

Expand Down Expand Up @@ -200,16 +193,4 @@ impl Context {
fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData {
module_id.module(&self.def_maps)
}

/// Returns the next available storage slot in the given module.
/// Returns None if the given module is not a contract module.
fn next_storage_slot(&mut self, module_id: def_map::ModuleId) -> Option<StorageSlot> {
let module = self.module(module_id);

module.is_contract.then(|| {
let next_slot = self.storage_slots.entry(module_id).or_insert(0);
*next_slot += 1;
*next_slot
})
}
}
15 changes: 2 additions & 13 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::ast::Ident;
use crate::graph::CrateId;
use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias};
use crate::hir::def_map::{LocalModuleId, ModuleId};
use crate::hir::StorageSlot;
use crate::hir_def::stmt::HirLetStatement;
use crate::hir_def::traits::TraitImpl;
use crate::hir_def::traits::{Trait, TraitConstraint};
Expand Down Expand Up @@ -399,10 +398,6 @@ impl DefinitionKind {
pub struct GlobalInfo {
pub ident: Ident,
pub local_id: LocalModuleId,

/// Global definitions have an associated storage slot if they are defined within
/// a contract. If they're defined elsewhere, this value is None.
pub storage_slot: Option<StorageSlot>,
}

impl Default for NodeInterner {
Expand Down Expand Up @@ -578,14 +573,8 @@ impl NodeInterner {
self.id_to_type.insert(definition_id.into(), typ);
}

pub fn push_global(
&mut self,
stmt_id: StmtId,
ident: Ident,
local_id: LocalModuleId,
storage_slot: Option<StorageSlot>,
) {
self.globals.insert(stmt_id, GlobalInfo { ident, local_id, storage_slot });
pub fn push_global(&mut self, stmt_id: StmtId, ident: Ident, local_id: LocalModuleId) {
self.globals.insert(stmt_id, GlobalInfo { ident, local_id });
}

/// Intern an empty global stmt. Used for collecting globals
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_printable_type/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option<String> {
}

(PrintableValue::String(s), PrintableType::String { .. }) => {
output.push_str(&format!(r#""{s}""#));
output.push_str(s);
}

(PrintableValue::Struct(map), PrintableType::Struct { name, fields, .. }) => {
Expand Down
Binary file modified test_programs/acir_artifacts/bit_shifts_comptime/target/acir.gz
Binary file not shown.
Binary file modified test_programs/acir_artifacts/bit_shifts_comptime/target/witness.gz
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ fn main(x: u64) {

//regression for 3481
assert(x << 63 == 0);

assert_eq((1 as u56) << (32 as u56), 0x0100000000);
}

fn regression_2250() {
Expand Down
2 changes: 1 addition & 1 deletion tooling/debugger/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ fn test_break_brillig_block_while_stepping_acir_opcodes() {

// set breakpoint
let breakpoint_location = OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 };
assert!(context.add_breakpoint(breakpoint_location.clone()));
assert!(context.add_breakpoint(breakpoint_location));

// execute the first ACIR opcode (Brillig block) -> should reach the breakpoint instead
let result = context.step_acir_opcode();
Expand Down
30 changes: 22 additions & 8 deletions tooling/nargo_fmt/src/visitor/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use noirc_frontend::{
ConstrainKind, ConstrainStatement, ExpressionKind, ForRange, Statement, StatementKind,
};

use crate::rewrite;
use crate::{rewrite, visitor::expr::wrap_exprs};

use super::ExpressionType;

Expand Down Expand Up @@ -33,30 +33,44 @@ impl super::FmtVisitor<'_> {
self.push_rewrite(format!("{let_str} {expr_str};"), span);
}
StatementKind::Constrain(ConstrainStatement(expr, message, kind)) => {
let mut nested_shape = self.shape();
let shape = nested_shape;

nested_shape.indent.block_indent(self.config);

let message =
message.map_or(String::new(), |message| format!(", \"{message}\""));
let constrain = match kind {

let (callee, args) = match kind {
ConstrainKind::Assert => {
let assertion = rewrite::sub_expr(self, self.shape(), expr);
let assertion = rewrite::sub_expr(self, nested_shape, expr);
let args = format!("{assertion}{message}");

format!("assert({assertion}{message});")
("assert", args)
}
ConstrainKind::AssertEq => {
if let ExpressionKind::Infix(infix) = expr.kind {
let lhs = rewrite::sub_expr(self, self.shape(), infix.lhs);
let rhs = rewrite::sub_expr(self, self.shape(), infix.rhs);
let lhs = rewrite::sub_expr(self, nested_shape, infix.lhs);
let rhs = rewrite::sub_expr(self, nested_shape, infix.rhs);

format!("assert_eq({lhs}, {rhs}{message});")
let args = format!("{lhs}, {rhs}{message}");

("assert_eq", args)
} else {
unreachable!()
}
}
ConstrainKind::Constrain => {
let expr = rewrite::sub_expr(self, self.shape(), expr);
format!("constrain {expr};")
let constrain = format!("constrain {expr};");
self.push_rewrite(constrain, span);
return;
}
};

let args = wrap_exprs("(", ")", args, nested_shape, shape, true);
let constrain = format!("{callee}{args};");

self.push_rewrite(constrain, span);
}
StatementKind::For(for_stmt) => {
Expand Down
12 changes: 7 additions & 5 deletions tooling/nargo_fmt/tests/expected/call.nr
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ fn foo() {

assert(x == y);

assert(p4_affine.eq(
Gaffine::new(
6890855772600357754907169075114257697580319025794532037257385534741338397365,
4338620300185947561074059802482547481416142213883829469920100239455078257889
assert(
p4_affine.eq(
Gaffine::new(
6890855772600357754907169075114257697580319025794532037257385534741338397365,
4338620300185947561074059802482547481416142213883829469920100239455078257889
)
)
));
);
}
6 changes: 4 additions & 2 deletions tooling/nargo_fmt/tests/expected/infix.nr
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ fn foo() {
}

fn big() {
assert(bjj_affine.contains(bjj_affine.gen)
assert(
bjj_affine.contains(bjj_affine.gen)
& bjj_affine.contains(p1_affine)
& bjj_affine.contains(p2_affine)
& bjj_affine.contains(p3_affine)
& bjj_affine.contains(p4_affine)
& bjj_affine.contains(p5_affine));
& bjj_affine.contains(p5_affine)
);
}
9 changes: 6 additions & 3 deletions tooling/noir_codegen/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const codegenPrelude = `/* Autogenerated file, do not edit! */
/* eslint-disable */
import { Noir, InputMap } from "@noir-lang/noir_js"
import { Noir, InputMap, CompiledCircuit } from "@noir-lang/noir_js"
`;

const codegenFunction = (
Expand All @@ -19,8 +19,11 @@ const codegenFunction = (
const args = function_signature.inputs.map(([name]) => `${name}`).join(', ');
const args_with_types = function_signature.inputs.map(([name, type]) => `${name}: ${type}`).join(', ');

return `export async function ${name}(${args_with_types}): Promise<${function_signature.returnValue}> {
const program = new Noir(${JSON.stringify(compiled_program)});
return `
export const ${name}_circuit: CompiledCircuit = ${JSON.stringify(compiled_program)};
export async function ${name}(${args_with_types}): Promise<${function_signature.returnValue}> {
const program = new Noir(${name}_circuit);
const args: InputMap = { ${args} };
const { returnValue } = await program.execute(args);
return returnValue as ${function_signature.returnValue};
Expand Down
4 changes: 4 additions & 0 deletions tooling/noir_codegen/src/noir_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ function abiTypeToTs(type: AbiType, primitiveTypeMap: Map<string, PrimitiveTypes
return `string`;
case 'struct':
return getLastComponentOfPath(type.path);
case 'tuple': {
const field_types = type.fields.map((field) => abiTypeToTs(field, primitiveTypeMap));
return `[${field_types.join(', ')}]`;
}
default:
throw new Error(`Unknown ABI type ${JSON.stringify(type)}`);
}
Expand Down
10 changes: 8 additions & 2 deletions tooling/noir_codegen/test/assert_lt/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@ struct MyStruct {
bar: [str<5>; 3],
}

fn main(x: u64, y: pub u64, array: [u8; 5], my_struct: MyStruct, string: str<5>) -> pub u64 {
fn main(
x: u64,
y: pub u64,
array: [u8; 5],
my_struct: MyStruct,
string: str<5>
) -> pub (u64, u64, MyStruct) {
assert(array.len() == 5);
assert(my_struct.foo);
assert(string == "12345");

assert(x < y);
x + y
(x + y, 3, my_struct)
}
2 changes: 1 addition & 1 deletion tooling/noir_codegen/test/assert_lt/target/assert_lt.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"noir_version":"0.19.2+87bb3f0d789765f2d65a1e7b7554742994da2680","hash":12941906747567599524,"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"private"},{"name":"y","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"public"},{"name":"array","type":{"kind":"array","length":5,"type":{"kind":"integer","sign":"unsigned","width":8}},"visibility":"private"},{"name":"my_struct","type":{"kind":"struct","path":"MyStruct","fields":[{"name":"foo","type":{"kind":"boolean"}},{"name":"bar","type":{"kind":"array","length":3,"type":{"kind":"string","length":5}}}]},"visibility":"private"},{"name":"string","type":{"kind":"string","length":5},"visibility":"private"}],"param_witnesses":{"array":[{"start":3,"end":8}],"my_struct":[{"start":8,"end":24}],"string":[{"start":24,"end":29}],"x":[{"start":1,"end":2}],"y":[{"start":2,"end":3}]},"return_type":{"kind":"integer","sign":"unsigned","width":64},"return_witnesses":[31]},"bytecode":"H4sIAAAAAAAA/82X206DQBCGF+qh9VDP2gO0eKlXuwVauGt8k7Ys0URTY4h9fTvprm4HJVFmEych8FE6/Ay7zP63jLF7tglnvblqPzXYRdxYb02DdxDvIt5DvK9Y35Op/BC8XoimcS8zb8jHUSQnIylCMeOjdJ7EPIrn40QkIk7ibJSEoUyiZJLO0wlPRRRKkcdpmKvETTqNXNehhepygPgQ8RHiY8RtxCeITxGfIT5HfIH4EvEV4mvEN4g7iLuIe4j7iD32NW502Bg/U6IxY1Nnh0CnzCEyqzq7ZDoXuU2dPTqd0qbOPp3OzKZOj07nAvqNy8rhEmt2GN3cd/+uS+AT3zw6WW6zrr7aD9imh+txoa+BPv/AymPGMY5ddY1bcY3zQ56WcU7/v238XvfhS8Uwb06V01eFpF6A+HQaPxcgAyOnjgZxPWxNqrq5AsJ6VtXvlzo50il8wmceEL7XGvWr/MD953lT9Z55vdiaJ7xeCMp5MmT03x2ds2+8c6gnNBhoPGAYtUmEpgDGCMwQGCAwPdAUwNyAoQETA8YFzAoYFDAlYETAfMAiGRagPXUvj203Kn08ZNtN5k7tPbWfFYV8eS2CYhnMsixYPRWPwfJdvuXPy9UHoDK8FUEPAAA="}
{"noir_version":"0.19.3+e9322d14070fa444d77ee5c43c905dd86a67c6e3","hash":9449934793688855780,"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"private"},{"name":"y","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"public"},{"name":"array","type":{"kind":"array","length":5,"type":{"kind":"integer","sign":"unsigned","width":8}},"visibility":"private"},{"name":"my_struct","type":{"kind":"struct","path":"MyStruct","fields":[{"name":"foo","type":{"kind":"boolean"}},{"name":"bar","type":{"kind":"array","length":3,"type":{"kind":"string","length":5}}}]},"visibility":"private"},{"name":"string","type":{"kind":"string","length":5},"visibility":"private"}],"param_witnesses":{"array":[{"start":3,"end":8}],"my_struct":[{"start":8,"end":24}],"string":[{"start":24,"end":29}],"x":[{"start":1,"end":2}],"y":[{"start":2,"end":3}]},"return_type":{"kind":"tuple","fields":[{"kind":"integer","sign":"unsigned","width":64},{"kind":"integer","sign":"unsigned","width":64},{"kind":"struct","path":"MyStruct","fields":[{"name":"foo","type":{"kind":"boolean"}},{"name":"bar","type":{"kind":"array","length":3,"type":{"kind":"string","length":5}}}]}]},"return_witnesses":[31,32,33,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23]},"bytecode":"H4sIAAAAAAAA/81XbU/CMBDu5hv4gopvvGw49JOJH1q2wfaN+E+AddFEgzGL/H250Go5dInumnhJ0z2jXJ9er7s+t4yxe7YyZ9lc1Y8N7CK8tWw1A28jvIPwLsJ7Cus5mfIPxquZqBlzmX5DPowiORpIEYoJH6TTJOZRPB0mIhFxEmeDJAxlEiWjdJqOeCqiUIo8TsNcOa7RceQ6DnUUl32EDxA+RPgI4QbCxwifIHyKcBPhM4TPEb5A+BLhK4RbCLcR7iDcRdhjX3mjzUb+jIlyxibPFgFPmYNlVnm2yXjOcps8O3Q8pU2eXTqemU2eHh3PGdQbl22aS8zZYXRn3/07L4FffLN0Mt9mXH3V99iqhuu80GOgzj+wzZxxjGdXjXFLxjg/+Kkb7/T/G8bvVRe/EQxzciqfvgok9QXEp+P4eQHpGT61bRHHw9ahqurrhjCeZfH7JU+OeAqfcM09wn2tEL/SD9x/Pjdl+8yr2do54dVMUJ6Ta0b/3TF92tr3gI53aJNnn3DfuwZHyE8o2FDIQYBr0Q1FFoQmiEsQlCAiociCWASBCKIQhCCIPxB8IPJA2IGYA9EBF3q4LMNcHlsv/E31XGUOyI1g2fpsvfDfqd5T/aQo5MtrERTzYJJlweKpeAzm7/Itf54vPgBYg2KL1RAAAA=="}
Loading

0 comments on commit 377ffd3

Please sign in to comment.