Skip to content

Commit

Permalink
Merge branch 'master' into bump-backend
Browse files Browse the repository at this point in the history
* master:
  chore: address clippy warnings (#1239)
  chore(ssa refactor): Implement function calls (#1235)
  chore(ssa refactor): Implement mutable and immutable variables (#1234)
  chore(ssa refactor): Fix recursive printing of blocks (#1230)
  feat(noir): added assert keyword (#1227)
  chore(ssa refactor): Implement ssa-gen for indexing, cast, constrain, if, unary (#1225)
  feat(noir): added `distinct` keyword (#1219)
  • Loading branch information
TomAFrench committed Apr 27, 2023
2 parents be16e46 + 4e198c0 commit 57ae056
Show file tree
Hide file tree
Showing 29 changed files with 986 additions and 328 deletions.
5 changes: 5 additions & 0 deletions crates/nargo_cli/tests/test_data/assert/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
1 change: 1 addition & 0 deletions crates/nargo_cli/tests/test_data/assert/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = "1"
3 changes: 3 additions & 0 deletions crates/nargo_cli/tests/test_data/assert/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main(x: Field) {
assert(x == 1);
}
24 changes: 24 additions & 0 deletions crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,30 @@ impl std::fmt::Display for AbiVisibility {
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
/// Represents whether the return value should compromise of unique witness indices such that no
/// index occurs within the program's abi more than once.
///
/// This is useful for application stacks that require an uniform abi across across multiple
/// circuits. When index duplication is allowed, the compiler may identify that a public input
/// reaches the output unaltered and is thus referenced directly, causing the input and output
/// witness indices to overlap. Similarly, repetitions of copied values in the output may be
/// optimized away.
pub enum AbiDistinctness {
Distinct,
DuplicationAllowed,
}

impl std::fmt::Display for AbiDistinctness {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
AbiDistinctness::Distinct => write!(f, "distinct"),
AbiDistinctness::DuplicationAllowed => write!(f, "duplication-allowed"),
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Sign {
Expand Down
20 changes: 18 additions & 2 deletions crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ pub struct Evaluator {
// and increasing as for `public_parameters`. We then use a `Vec` rather
// than a `BTreeSet` to preserve this order for the ABI.
return_values: Vec<Witness>,
// If true, indicates that the resulting ACIR should enforce that all inputs and outputs are
// comprised of unique witness indices by having extra constraints if necessary.
return_is_distinct: bool,

opcodes: Vec<AcirOpcode>,
}
Expand Down Expand Up @@ -102,6 +105,11 @@ pub fn create_circuit(
}

impl Evaluator {
// Returns true if the `witness_index` appears in the program's input parameters.
fn is_abi_input(&self, witness_index: Witness) -> bool {
witness_index.as_usize() <= self.num_witnesses_abi_len
}

// Returns true if the `witness_index`
// was created in the ABI as a private input.
//
Expand All @@ -111,11 +119,17 @@ impl Evaluator {
// If the `witness_index` is more than the `num_witnesses_abi_len`
// then it was created after the ABI was processed and is therefore
// an intermediate variable.
let is_intermediate_variable = witness_index.as_usize() > self.num_witnesses_abi_len;

let is_public_input = self.public_parameters.contains(&witness_index);

!is_intermediate_variable && !is_public_input
self.is_abi_input(witness_index) && !is_public_input
}

// True if the main function return has the `distinct` keyword and this particular witness
// index has already occurred elsewhere in the abi's inputs and outputs.
fn should_proxy_witness_for_abi_output(&self, witness_index: Witness) -> bool {
self.return_is_distinct
&& (self.is_abi_input(witness_index) || self.return_values.contains(&witness_index))
}

// Creates a new Witness index
Expand All @@ -139,6 +153,8 @@ impl Evaluator {
enable_logging: bool,
show_output: bool,
) -> Result<(), RuntimeError> {
self.return_is_distinct =
program.return_distinctness == noirc_abi::AbiDistinctness::Distinct;
let mut ir_gen = IrGenerator::new(program);
self.parse_abi_alt(&mut ir_gen);

Expand Down
12 changes: 11 additions & 1 deletion crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use acvm::acir::native_types::Expression;

use crate::{
errors::RuntimeErrorKind,
ssa::{
Expand Down Expand Up @@ -46,7 +48,15 @@ pub(crate) fn evaluate(
"we do not allow private ABI inputs to be returned as public outputs",
)));
}
evaluator.return_values.push(witness);
// Check if the outputted witness needs separating from an existing occurrence in the
// abi. This behavior stems from usage of the `distinct` keyword.
let return_witness = if evaluator.should_proxy_witness_for_abi_output(witness) {
let proxy_constraint = Expression::from(witness);
evaluator.create_intermediate_variable(proxy_constraint)
} else {
witness
};
evaluator.return_values.push(return_witness);
}
}

Expand Down
97 changes: 49 additions & 48 deletions crates/noirc_evaluator/src/ssa_refactor/ir/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,14 @@ impl ControlFlowGraph {

#[cfg(test)]
mod tests {
use crate::ssa_refactor::ir::{instruction::TerminatorInstruction, types::Type};
use crate::ssa_refactor::ir::{instruction::TerminatorInstruction, map::Id, types::Type};

use super::{super::function::Function, ControlFlowGraph};

#[test]
fn empty() {
let mut func = Function::new("func".into());
let func_id = Id::test_new(0);
let mut func = Function::new("func".into(), func_id);
let block_id = func.entry_block();
func.dfg[block_id].set_terminator(TerminatorInstruction::Return { return_values: vec![] });

Expand All @@ -133,74 +134,74 @@ mod tests {
// Build function of form
// fn func {
// block0(cond: u1):
// jmpif cond(), then: block2, else: block1
// jmpif cond, then: block2, else: block1
// block1():
// jmpif cond(), then: block1, else: block2
// jmpif cond, then: block1, else: block2
// block2():
// return
// return ()
// }
let mut func = Function::new("func".into());
let func_id = Id::test_new(0);
let mut func = Function::new("func".into(), func_id);
let block0_id = func.entry_block();
let cond = func.dfg.add_block_parameter(block0_id, Type::unsigned(1));
let block1_id = func.dfg.new_block();
let block2_id = func.dfg.new_block();
let block1_id = func.dfg.make_block();
let block2_id = func.dfg.make_block();

func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
then_destination: block2_id,
else_destination: block1_id,
arguments: vec![],
});
func.dfg[block1_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
then_destination: block1_id,
else_destination: block2_id,
arguments: vec![],
});
func.dfg[block2_id].set_terminator(TerminatorInstruction::Return { return_values: vec![] });

let mut cfg = ControlFlowGraph::with_function(&func);

#[allow(clippy::needless_collect)]
{
let block0_predecessors = cfg.pred_iter(block0_id).collect::<Vec<_>>();
let block1_predecessors = cfg.pred_iter(block1_id).collect::<Vec<_>>();
let block2_predecessors = cfg.pred_iter(block2_id).collect::<Vec<_>>();
let block0_predecessors: Vec<_> = cfg.pred_iter(block0_id).collect();
let block1_predecessors: Vec<_> = cfg.pred_iter(block1_id).collect();
let block2_predecessors: Vec<_> = cfg.pred_iter(block2_id).collect();

let block0_successors = cfg.succ_iter(block0_id).collect::<Vec<_>>();
let block1_successors = cfg.succ_iter(block1_id).collect::<Vec<_>>();
let block2_successors = cfg.succ_iter(block2_id).collect::<Vec<_>>();
let block0_successors: Vec<_> = cfg.succ_iter(block0_id).collect();
let block1_successors: Vec<_> = cfg.succ_iter(block1_id).collect();
let block2_successors: Vec<_> = cfg.succ_iter(block2_id).collect();

assert_eq!(block0_predecessors.len(), 0);
assert_eq!(block1_predecessors.len(), 2);
assert_eq!(block2_predecessors.len(), 2);

assert_eq!(block1_predecessors.contains(&block0_id), true);
assert_eq!(block1_predecessors.contains(&block1_id), true);
assert_eq!(block2_predecessors.contains(&block0_id), true);
assert_eq!(block2_predecessors.contains(&block1_id), true);
assert!(block1_predecessors.contains(&block0_id));
assert!(block1_predecessors.contains(&block1_id));
assert!(block2_predecessors.contains(&block0_id));
assert!(block2_predecessors.contains(&block1_id));

assert_eq!(block0_successors.len(), 2);
assert_eq!(block1_successors.len(), 2);
assert_eq!(block2_successors.len(), 0);

assert_eq!(block0_successors.contains(&block1_id), true);
assert_eq!(block0_successors.contains(&block2_id), true);
assert_eq!(block1_successors.contains(&block1_id), true);
assert_eq!(block1_successors.contains(&block2_id), true);
assert!(block0_successors.contains(&block1_id));
assert!(block0_successors.contains(&block2_id));
assert!(block1_successors.contains(&block1_id));
assert!(block1_successors.contains(&block2_id));
}

// Modify function to form:
// fn func {
// block0(cond: u1):
// jmpif cond(), then: block1, else: ret_block
// jmpif cond, then: block1, else: ret_block
// block1():
// jmpif cond(), then: block1, else: block2
// jmpif cond, then: block1, else: block2
// block2():
// jmp ret_block
// jmp ret_block()
// ret_block():
// return
// return ()
// }
let ret_block_id = func.dfg.new_block();
let ret_block_id = func.dfg.make_block();
func.dfg[ret_block_id]
.set_terminator(TerminatorInstruction::Return { return_values: vec![] });
func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp {
Expand All @@ -211,41 +212,41 @@ mod tests {
condition: cond,
then_destination: block1_id,
else_destination: ret_block_id,
arguments: vec![],
});

// Recompute new and changed blocks
cfg.recompute_block(&mut func, block0_id);
cfg.recompute_block(&mut func, block2_id);
cfg.recompute_block(&mut func, ret_block_id);
cfg.recompute_block(&func, block0_id);
cfg.recompute_block(&func, block2_id);
cfg.recompute_block(&func, ret_block_id);

#[allow(clippy::needless_collect)]
{
let block0_predecessors = cfg.pred_iter(block0_id).collect::<Vec<_>>();
let block1_predecessors = cfg.pred_iter(block1_id).collect::<Vec<_>>();
let block2_predecessors = cfg.pred_iter(block2_id).collect::<Vec<_>>();
let block0_predecessors: Vec<_> = cfg.pred_iter(block0_id).collect();
let block1_predecessors: Vec<_> = cfg.pred_iter(block1_id).collect();
let block2_predecessors: Vec<_> = cfg.pred_iter(block2_id).collect();

let block0_successors = cfg.succ_iter(block0_id).collect::<Vec<_>>();
let block1_successors = cfg.succ_iter(block1_id).collect::<Vec<_>>();
let block2_successors = cfg.succ_iter(block2_id).collect::<Vec<_>>();
let block0_successors: Vec<_> = cfg.succ_iter(block0_id).collect();
let block1_successors: Vec<_> = cfg.succ_iter(block1_id).collect();
let block2_successors: Vec<_> = cfg.succ_iter(block2_id).collect();

assert_eq!(block0_predecessors.len(), 0);
assert_eq!(block1_predecessors.len(), 2);
assert_eq!(block2_predecessors.len(), 1);

assert_eq!(block1_predecessors.contains(&block0_id), true);
assert_eq!(block1_predecessors.contains(&block1_id), true);
assert_eq!(block2_predecessors.contains(&block0_id), false);
assert_eq!(block2_predecessors.contains(&block1_id), true);
assert!(block1_predecessors.contains(&block0_id));
assert!(block1_predecessors.contains(&block1_id));
assert!(!block2_predecessors.contains(&block0_id));
assert!(block2_predecessors.contains(&block1_id));

assert_eq!(block0_successors.len(), 2);
assert_eq!(block1_successors.len(), 2);
assert_eq!(block2_successors.len(), 1);

assert_eq!(block0_successors.contains(&block1_id), true);
assert_eq!(block0_successors.contains(&ret_block_id), true);
assert_eq!(block1_successors.contains(&block1_id), true);
assert_eq!(block1_successors.contains(&block2_id), true);
assert_eq!(block2_successors.contains(&ret_block_id), true);
assert!(block0_successors.contains(&block1_id));
assert!(block0_successors.contains(&ret_block_id));
assert!(block1_successors.contains(&block1_id));
assert!(block1_successors.contains(&block2_id));
assert!(block2_successors.contains(&ret_block_id));
}
}
}
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa_refactor/ir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ impl NumericConstant {
Self(value)
}

pub(crate) fn value(&self) -> &FieldElement {
&self.0
pub(crate) fn value(&self) -> FieldElement {
self.0
}
}

Expand Down
49 changes: 45 additions & 4 deletions crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::{
basic_block::{BasicBlock, BasicBlockId},
constant::{NumericConstant, NumericConstantId},
function::Signature,
instruction::{Instruction, InstructionId, InstructionResultType},
instruction::{Instruction, InstructionId, InstructionResultType, TerminatorInstruction},
map::{DenseMap, Id, SecondaryMap, TwoWayMap},
types::Type,
value::{Value, ValueId},
Expand Down Expand Up @@ -75,14 +75,14 @@ impl DataFlowGraph {
/// Creates a new basic block with no parameters.
/// After being created, the block is unreachable in the current function
/// until another block is made to jump to it.
pub(crate) fn new_block(&mut self) -> BasicBlockId {
pub(crate) fn make_block(&mut self) -> BasicBlockId {
self.blocks.insert(BasicBlock::new(Vec::new()))
}

/// Creates a new basic block with the given parameters.
/// After being created, the block is unreachable in the current function
/// until another block is made to jump to it.
pub(crate) fn new_block_with_parameters(
pub(crate) fn make_block_with_parameters(
&mut self,
parameter_types: impl Iterator<Item = Type>,
) -> BasicBlockId {
Expand Down Expand Up @@ -126,6 +126,17 @@ impl DataFlowGraph {
id
}

/// Replace an instruction id with another.
///
/// This function should generally be avoided if possible in favor of inserting new
/// instructions since it does not check whether the instruction results of the removed
/// instruction are still in use. Users of this function thus need to ensure the old
/// instruction's results are no longer in use or are otherwise compatible with the
/// new instruction's result count and types.
pub(crate) fn replace_instruction(&mut self, id: Id<Instruction>, instruction: Instruction) {
self.instructions[id] = instruction;
}

/// Insert a value into the dfg's storage and return an id to reference it.
/// Until the value is used in an instruction it is unreachable.
pub(crate) fn make_value(&mut self, value: Value) -> ValueId {
Expand All @@ -141,8 +152,11 @@ impl DataFlowGraph {

/// Attaches results to the instruction, clearing any previous results.
///
/// This does not normally need to be called manually as it is called within
/// make_instruction automatically.
///
/// Returns the results of the instruction
fn make_instruction_results(
pub(crate) fn make_instruction_results(
&mut self,
instruction_id: InstructionId,
ctrl_typevars: Option<Vec<Type>>,
Expand Down Expand Up @@ -230,6 +244,33 @@ impl DataFlowGraph {
) {
self.blocks[block].insert_instruction(instruction);
}

/// Returns the field element represented by this value if it is a numeric constant.
/// Returns None if the given value is not a numeric constant.
pub(crate) fn get_numeric_constant(&self, value: Id<Value>) -> Option<FieldElement> {
self.get_numeric_constant_with_type(value).map(|(value, _typ)| value)
}

/// Returns the field element and type represented by this value if it is a numeric constant.
/// Returns None if the given value is not a numeric constant.
pub(crate) fn get_numeric_constant_with_type(
&self,
value: Id<Value>,
) -> Option<(FieldElement, Type)> {
match self.values[value] {
Value::NumericConstant { constant, typ } => Some((self[constant].value(), typ)),
_ => None,
}
}

/// Sets the terminator instruction for the given basic block
pub(crate) fn set_block_terminator(
&mut self,
block: BasicBlockId,
terminator: TerminatorInstruction,
) {
self.blocks[block].set_terminator(terminator);
}
}

impl std::ops::Index<InstructionId> for DataFlowGraph {
Expand Down
Loading

0 comments on commit 57ae056

Please sign in to comment.