Skip to content

Commit

Permalink
chore: pull out sync changes (#10274)
Browse files Browse the repository at this point in the history
Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.
  • Loading branch information
TomAFrench authored Nov 28, 2024
1 parent 3d113b2 commit 391a6b7
Show file tree
Hide file tree
Showing 19 changed files with 356 additions and 73 deletions.
3 changes: 3 additions & 0 deletions noir/noir-repo/.github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,9 @@ jobs:

- name: Install Foundry
uses: foundry-rs/[email protected]
with:
version: nightly-8660e5b941fe7f4d67e246cfd3dafea330fb53b1


- name: Install `bb`
run: |
Expand Down
38 changes: 37 additions & 1 deletion noir/noir-repo/compiler/noirc_errors/src/position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ impl Span {
self.start() <= other.start() && self.end() >= other.end()
}

/// Returns `true` if any point of `self` intersects a point of `other`.
/// Adjacent spans are considered to intersect (so, for example, `0..1` intersects `1..3`).
pub fn intersects(&self, other: &Span) -> bool {
self.end() > other.start() && self.start() < other.end()
self.end() >= other.start() && self.start() <= other.end()
}

pub fn is_smaller(&self, other: &Span) -> bool {
Expand Down Expand Up @@ -140,3 +142,37 @@ impl Location {
self.file == other.file && self.span.contains(&other.span)
}
}

#[cfg(test)]
mod tests {
use crate::Span;

#[test]
fn test_intersects() {
assert!(Span::from(5..10).intersects(&Span::from(5..10)));

assert!(Span::from(5..10).intersects(&Span::from(5..5)));
assert!(Span::from(5..5).intersects(&Span::from(5..10)));

assert!(Span::from(10..10).intersects(&Span::from(5..10)));
assert!(Span::from(5..10).intersects(&Span::from(10..10)));

assert!(Span::from(5..10).intersects(&Span::from(6..9)));
assert!(Span::from(6..9).intersects(&Span::from(5..10)));

assert!(Span::from(5..10).intersects(&Span::from(4..11)));
assert!(Span::from(4..11).intersects(&Span::from(5..10)));

assert!(Span::from(5..10).intersects(&Span::from(4..6)));
assert!(Span::from(4..6).intersects(&Span::from(5..10)));

assert!(Span::from(5..10).intersects(&Span::from(9..11)));
assert!(Span::from(9..11).intersects(&Span::from(5..10)));

assert!(!Span::from(5..10).intersects(&Span::from(3..4)));
assert!(!Span::from(3..4).intersects(&Span::from(5..10)));

assert!(!Span::from(5..10).intersects(&Span::from(11..12)));
assert!(!Span::from(11..12).intersects(&Span::from(5..10)));
}
}
52 changes: 4 additions & 48 deletions noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,10 @@ mod big_int;
mod brillig_directive;
mod generated_acir;

use crate::brillig::brillig_gen::gen_brillig_for;
use crate::brillig::{
brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext,
brillig_ir::{
artifact::{BrilligParameter, GeneratedBrillig},
BrilligContext,
},
brillig_ir::artifact::{BrilligParameter, GeneratedBrillig},
Brillig,
};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
Expand Down Expand Up @@ -518,7 +516,7 @@ impl<'a> Context<'a> {
let outputs: Vec<AcirType> =
vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into());

let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?;
let code = gen_brillig_for(main_func, arguments.clone(), brillig)?;

// We specifically do not attempt execution of the brillig code being generated as this can result in it being
// replaced with constraints on witnesses to the program outputs.
Expand Down Expand Up @@ -878,8 +876,7 @@ impl<'a> Context<'a> {
None,
)?
} else {
let code =
self.gen_brillig_for(func, arguments.clone(), brillig)?;
let code = gen_brillig_for(func, arguments.clone(), brillig)?;
let generated_pointer =
self.shared_context.new_generated_pointer();
let output_values = self.acir_context.brillig_call(
Expand Down Expand Up @@ -999,47 +996,6 @@ impl<'a> Context<'a> {
.collect()
}

fn gen_brillig_for(
&self,
func: &Function,
arguments: Vec<BrilligParameter>,
brillig: &Brillig,
) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
// Create the entry point artifact
let mut entry_point = BrilligContext::new_entry_point_artifact(
arguments,
BrilligFunctionContext::return_values(func),
func.id(),
);
entry_point.name = func.name().to_string();

// Link the entry point with all dependencies
while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {
let artifact = &brillig.find_by_label(unresolved_fn_label.clone());
let artifact = match artifact {
Some(artifact) => artifact,
None => {
return Err(InternalError::General {
message: format!("Cannot find linked fn {unresolved_fn_label}"),
call_stack: CallStack::new(),
})
}
};
entry_point.link_with(artifact);
// Insert the range of opcode locations occupied by a procedure
if let Some(procedure_id) = &artifact.procedure {
let num_opcodes = entry_point.byte_code.len();
let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
// We subtract one as to keep the range inclusive on both ends
entry_point
.procedure_locations
.insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
}
}
// Generate the final bytecode
Ok(entry_point.finish())
}

/// Handles an ArrayGet or ArraySet instruction.
/// To set an index of the array (and create a new array in doing so), pass Some(value) for
/// store_value. To just retrieve an index of the array, pass None for store_value.
Expand Down
54 changes: 50 additions & 4 deletions noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ mod variable_liveness;
use acvm::FieldElement;

use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext};
use super::brillig_ir::{
artifact::{BrilligArtifact, Label},
BrilligContext,
use super::{
brillig_ir::{
artifact::{BrilligArtifact, BrilligParameter, GeneratedBrillig, Label},
BrilligContext,
},
Brillig,
};
use crate::{
errors::InternalError,
ssa::ir::{dfg::CallStack, function::Function},
};
use crate::ssa::ir::function::Function;

/// Converting an SSA function into Brillig bytecode.
pub(crate) fn convert_ssa_function(
Expand All @@ -36,3 +42,43 @@ pub(crate) fn convert_ssa_function(
artifact.name = func.name().to_string();
artifact
}

pub(crate) fn gen_brillig_for(
func: &Function,
arguments: Vec<BrilligParameter>,
brillig: &Brillig,
) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
// Create the entry point artifact
let mut entry_point = BrilligContext::new_entry_point_artifact(
arguments,
FunctionContext::return_values(func),
func.id(),
);
entry_point.name = func.name().to_string();

// Link the entry point with all dependencies
while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {
let artifact = &brillig.find_by_label(unresolved_fn_label.clone());
let artifact = match artifact {
Some(artifact) => artifact,
None => {
return Err(InternalError::General {
message: format!("Cannot find linked fn {unresolved_fn_label}"),
call_stack: CallStack::new(),
})
}
};
entry_point.link_with(artifact);
// Insert the range of opcode locations occupied by a procedure
if let Some(procedure_id) = &artifact.procedure {
let num_opcodes = entry_point.byte_code.len();
let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
// We subtract one as to keep the range inclusive on both ends
entry_point
.procedure_locations
.insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
}
}
// Generate the final bytecode
Ok(entry_point.finish())
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) struct FunctionInserter<'f> {
///
/// This is optional since caching arrays relies on the inserter inserting strictly
/// in control-flow order. Otherwise, if arrays later in the program are cached first,
/// they may be refered to by instructions earlier in the program.
/// they may be referred to by instructions earlier in the program.
array_cache: Option<ArrayCache>,

/// If this pass is loop unrolling, store the block before the loop to optionally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::ssa::opt::flatten_cfg::value_merger::ValueMerger;
use super::{
basic_block::BasicBlockId,
dfg::{CallStack, DataFlowGraph},
function::Function,
map::Id,
types::{NumericType, Type},
value::{Value, ValueId},
Expand Down Expand Up @@ -366,12 +367,12 @@ impl Instruction {
}
}

pub(crate) fn can_eliminate_if_unused(&self, dfg: &DataFlowGraph) -> bool {
pub(crate) fn can_eliminate_if_unused(&self, function: &Function) -> bool {
use Instruction::*;
match self {
Binary(binary) => {
if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) {
if let Some(rhs) = dfg.get_numeric_constant(binary.rhs) {
if let Some(rhs) = function.dfg.get_numeric_constant(binary.rhs) {
rhs != FieldElement::zero()
} else {
false
Expand All @@ -398,7 +399,7 @@ impl Instruction {
| RangeCheck { .. } => false,

// Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
Call { func, .. } => match dfg[*func] {
Call { func, .. } => match function.dfg[*func] {
// Explicitly allows removal of unused ec operations, even if they can fail
Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::MultiScalarMul))
| Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::EmbeddedCurveAdd)) => true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,8 @@ fn simplify_derive_generators(
results.push(is_infinite);
}
let len = results.len();
let typ = Type::Array(vec![Type::field()].into(), len);
let typ =
Type::Array(vec![Type::field(), Type::field(), Type::unsigned(1)].into(), len / 3);
let result = make_array(dfg, results.into(), typ, block, call_stack);
SimplifyResult::SimplifiedTo(result)
} else {
Expand All @@ -816,3 +817,34 @@ fn simplify_derive_generators(
unreachable!("Unexpected number of arguments to derive_generators");
}
}

#[cfg(test)]
mod tests {
use crate::ssa::{opt::assert_normalized_ssa_equals, Ssa};

#[test]
fn simplify_derive_generators_has_correct_type() {
let src = "
brillig(inline) fn main f0 {
b0():
v0 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
// This call was previously incorrectly simplified to something that returned `[Field; 3]`
v2 = call derive_pedersen_generators(v0, u32 0) -> [(Field, Field, u1); 1]
return v2
}
";
let ssa = Ssa::from_str(src).unwrap();

let expected = "
brillig(inline) fn main f0 {
b0():
v15 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
v19 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0] : [(Field, Field, u1); 1]
return v19
}
";
assert_normalized_ssa_equals(ssa, expected);
}
}
4 changes: 2 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,13 +272,13 @@ fn display_constrain_error(
) -> Result {
match error {
ConstrainError::StaticString(assert_message_string) => {
writeln!(f, " '{assert_message_string:?}'")
writeln!(f, ", {assert_message_string:?}")
}
ConstrainError::Dynamic(_, is_string, values) => {
if let Some(constant_string) =
try_to_extract_string_from_error_payload(*is_string, values, &function.dfg)
{
writeln!(f, " '{}'", constant_string)
writeln!(f, ", {constant_string:?}")
} else {
writeln!(f, ", data {}", value_list(function, values))
}
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl Context {
fn is_unused(&self, instruction_id: InstructionId, function: &Function) -> bool {
let instruction = &function.dfg[instruction_id];

if instruction.can_eliminate_if_unused(&function.dfg) {
if instruction.can_eliminate_if_unused(function) {
let results = function.dfg.instruction_results(instruction_id);
results.iter().all(|result| !self.used_values.contains(result))
} else if let Instruction::Call { func, arguments } = instruction {
Expand Down
7 changes: 7 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub(crate) enum ParsedInstruction {
Constrain {
lhs: ParsedValue,
rhs: ParsedValue,
assert_message: Option<AssertMessage>,
},
DecrementRc {
value: ParsedValue,
Expand Down Expand Up @@ -129,6 +130,12 @@ pub(crate) enum ParsedInstruction {
},
}

#[derive(Debug)]
pub(crate) enum AssertMessage {
Static(String),
Dynamic(Vec<ParsedValue>),
}

#[derive(Debug)]
pub(crate) enum ParsedTerminator {
Jmp { destination: Identifier, arguments: Vec<ParsedValue> },
Expand Down
Loading

0 comments on commit 391a6b7

Please sign in to comment.