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

chore: pull out sync changes #10274

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading