Skip to content

Commit

Permalink
Merge branch 'jf/comptime-globals' into jf/comptime-mut-globals
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher committed Apr 29, 2024
2 parents 1c5dced + cfc2bef commit e787045
Show file tree
Hide file tree
Showing 46 changed files with 724 additions and 179 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/publish-nargo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ permissions:

jobs:
build-apple-darwin:
runs-on: macos-latest
runs-on: macos-12
env:
CROSS_CONFIG: ${{ github.workspace }}/.github/Cross.toml
NIGHTLY_RELEASE: ${{ inputs.tag == '' }}
Expand Down
4 changes: 3 additions & 1 deletion acvm-repo/brillig_vm/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ pub(crate) fn evaluate_binary_int_op(
}
}
})?;
let rhs = rhs.expect_integer_with_bit_size(bit_size).map_err(|err| match err {
let rhs_bit_size =
if op == &BinaryIntOp::Shl || op == &BinaryIntOp::Shr { 8 } else { bit_size };
let rhs = rhs.expect_integer_with_bit_size(rhs_bit_size).map_err(|err| match err {
MemoryTypeError::MismatchedBitSize { value_bit_size, expected_bit_size } => {
BrilligArithmeticError::MismatchedRhsBitSize {
rhs_bit_size: value_bit_size,
Expand Down
14 changes: 10 additions & 4 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1279,8 +1279,11 @@ impl<'block> BrilligBlock<'block> {
dfg: &DataFlowGraph,
result_variable: SingleAddrVariable,
) {
let binary_type =
type_of_binary_operation(dfg[binary.lhs].get_type(), dfg[binary.rhs].get_type());
let binary_type = type_of_binary_operation(
dfg[binary.lhs].get_type(),
dfg[binary.rhs].get_type(),
binary.operator,
);

let left = self.convert_ssa_single_addr_value(binary.lhs, dfg);
let right = self.convert_ssa_single_addr_value(binary.rhs, dfg);
Expand Down Expand Up @@ -1766,7 +1769,7 @@ impl<'block> BrilligBlock<'block> {
}

/// Returns the type of the operation considering the types of the operands
pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type {
pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type, op: BinaryOp) -> Type {
match (lhs_type, rhs_type) {
(_, Type::Function) | (Type::Function, _) => {
unreachable!("Functions are invalid in binary operations")
Expand All @@ -1782,12 +1785,15 @@ pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type
}
// If both sides are numeric type, then we expect their types to be
// the same.
(Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => {
(Type::Numeric(lhs_type), Type::Numeric(rhs_type))
if op != BinaryOp::Shl && op != BinaryOp::Shr =>
{
assert_eq!(
lhs_type, rhs_type,
"lhs and rhs types in a binary operation are always the same but got {lhs_type} and {rhs_type}"
);
Type::Numeric(*lhs_type)
}
_ => lhs_type.clone(),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,6 @@ impl BrilligContext {
result: SingleAddrVariable,
operation: BrilligBinaryOp,
) {
assert!(
lhs.bit_size == rhs.bit_size,
"Not equal bit size for lhs and rhs: lhs {}, rhs {}",
lhs.bit_size,
rhs.bit_size
);
let is_field_op = lhs.bit_size == FieldElement::max_num_bits();
let expected_result_bit_size =
BrilligContext::binary_result_bit_size(operation, lhs.bit_size);
Expand Down
66 changes: 40 additions & 26 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use crate::brillig::brillig_ir::artifact::{BrilligParameter, GeneratedBrillig};
use crate::brillig::brillig_ir::BrilligContext;
use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
use crate::ssa::ir::function::InlineType;
pub(crate) use acir_ir::generated_acir::GeneratedAcir;
use noirc_frontend::monomorphization::ast::InlineType;

use acvm::acir::circuit::brillig::BrilligBytecode;
use acvm::acir::circuit::OpcodeLocation;
Expand Down Expand Up @@ -385,12 +385,12 @@ impl<'a> Context<'a> {
match function.runtime() {
RuntimeType::Acir(inline_type) => {
match inline_type {
InlineType::Fold => {}
InlineType::Inline => {
if function.id() != ssa.main_id {
panic!("ACIR function should have been inlined earlier if not marked otherwise");
}
}
InlineType::Fold | InlineType::Never => {}
}
// We only want to convert entry point functions. This being `main` and those marked with `InlineType::Fold`
Ok(Some(self.convert_acir_main(function, ssa, brillig)?))
Expand Down Expand Up @@ -2610,36 +2610,33 @@ mod test {
},
FieldElement,
};
use noirc_frontend::monomorphization::ast::InlineType;

use crate::{
brillig::Brillig,
ssa::{
acir_gen::acir_ir::generated_acir::BrilligStdlibFunc,
function_builder::FunctionBuilder,
ir::{
function::{FunctionId, InlineType},
instruction::BinaryOp,
map::Id,
types::Type,
},
ir::{function::FunctionId, instruction::BinaryOp, map::Id, types::Type},
},
};

fn build_basic_foo_with_return(
builder: &mut FunctionBuilder,
foo_id: FunctionId,
is_brillig_func: bool,
// `InlineType` can only exist on ACIR functions, so if the option is `None` we should generate a Brillig function
inline_type: Option<InlineType>,
) {
// fn foo f1 {
// b0(v0: Field, v1: Field):
// v2 = eq v0, v1
// constrain v2 == u1 0
// return v0
// }
if is_brillig_func {
builder.new_brillig_function("foo".into(), foo_id);
if let Some(inline_type) = inline_type {
builder.new_function("foo".into(), foo_id, inline_type);
} else {
builder.new_function("foo".into(), foo_id, InlineType::Fold);
builder.new_brillig_function("foo".into(), foo_id);
}
let foo_v0 = builder.add_parameter(Type::field());
let foo_v1 = builder.add_parameter(Type::field());
Expand All @@ -2650,8 +2647,25 @@ mod test {
builder.terminate_with_return(vec![foo_v0]);
}

/// Check that each `InlineType` which prevents inlining functions generates code in the same manner
#[test]
fn basic_call_with_outputs_assert() {
fn basic_calls_fold() {
basic_call_with_outputs_assert(InlineType::Fold);
call_output_as_next_call_input(InlineType::Fold);
basic_nested_call(InlineType::Fold);

call_output_as_next_call_input(InlineType::Never);
basic_nested_call(InlineType::Never);
basic_call_with_outputs_assert(InlineType::Never);
}

#[test]
#[should_panic]
fn call_without_inline_attribute() {
basic_call_with_outputs_assert(InlineType::Inline);
}

fn basic_call_with_outputs_assert(inline_type: InlineType) {
// acir(inline) fn main f0 {
// b0(v0: Field, v1: Field):
// v2 = call f1(v0, v1)
Expand Down Expand Up @@ -2679,7 +2693,7 @@ mod test {
builder.insert_constrain(main_call1_results[0], main_call2_results[0], None);
builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, false);
build_basic_foo_with_return(&mut builder, foo_id, Some(inline_type));

let ssa = builder.finish();

Expand Down Expand Up @@ -2745,8 +2759,7 @@ mod test {
}
}

#[test]
fn call_output_as_next_call_input() {
fn call_output_as_next_call_input(inline_type: InlineType) {
// acir(inline) fn main f0 {
// b0(v0: Field, v1: Field):
// v3 = call f1(v0, v1)
Expand Down Expand Up @@ -2775,7 +2788,7 @@ mod test {
builder.insert_constrain(main_call1_results[0], main_call2_results[0], None);
builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, false);
build_basic_foo_with_return(&mut builder, foo_id, Some(inline_type));

let ssa = builder.finish();

Expand All @@ -2794,8 +2807,7 @@ mod test {
check_call_opcode(&main_opcodes[1], 1, vec![Witness(2), Witness(1)], vec![Witness(3)]);
}

#[test]
fn basic_nested_call() {
fn basic_nested_call(inline_type: InlineType) {
// SSA for the following Noir program:
// fn main(x: Field, y: pub Field) {
// let z = func_with_nested_foo_call(x, y);
Expand Down Expand Up @@ -2851,7 +2863,7 @@ mod test {
builder.new_function(
"func_with_nested_foo_call".into(),
func_with_nested_foo_call_id,
InlineType::Fold,
inline_type,
);
let func_with_nested_call_v0 = builder.add_parameter(Type::field());
let func_with_nested_call_v1 = builder.add_parameter(Type::field());
Expand All @@ -2866,7 +2878,7 @@ mod test {
.to_vec();
builder.terminate_with_return(vec![foo_call[0]]);

build_basic_foo_with_return(&mut builder, foo_id, false);
build_basic_foo_with_return(&mut builder, foo_id, Some(inline_type));

let ssa = builder.finish();

Expand Down Expand Up @@ -2977,8 +2989,8 @@ mod test {
builder.insert_call(bar, vec![main_v0, main_v1], vec![Type::field()]).to_vec();
builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, true);
build_basic_foo_with_return(&mut builder, bar_id, true);
build_basic_foo_with_return(&mut builder, foo_id, None);
build_basic_foo_with_return(&mut builder, bar_id, None);

let ssa = builder.finish();
let brillig = ssa.to_brillig(false);
Expand Down Expand Up @@ -3106,7 +3118,7 @@ mod test {

builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, true);
build_basic_foo_with_return(&mut builder, foo_id, None);

let ssa = builder.finish();
// We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass.
Expand Down Expand Up @@ -3192,8 +3204,10 @@ mod test {

builder.terminate_with_return(vec![]);

build_basic_foo_with_return(&mut builder, foo_id, true);
build_basic_foo_with_return(&mut builder, bar_id, false);
// Build a Brillig function
build_basic_foo_with_return(&mut builder, foo_id, None);
// Build an ACIR function which has the same logic as the Brillig function above
build_basic_foo_with_return(&mut builder, bar_id, Some(InlineType::Fold));

let ssa = builder.finish();
// We need to generate Brillig artifacts for the regular Brillig function and pass them to the ACIR generation pass.
Expand Down
10 changes: 8 additions & 2 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::{borrow::Cow, rc::Rc};

use acvm::FieldElement;
use noirc_errors::Location;
use noirc_frontend::monomorphization::ast::InlineType;

use crate::ssa::ir::{
basic_block::BasicBlockId,
Expand All @@ -17,7 +18,7 @@ use super::{
ir::{
basic_block::BasicBlock,
dfg::{CallStack, InsertInstructionResult},
function::{InlineType, RuntimeType},
function::RuntimeType,
instruction::{ConstrainError, InstructionId, Intrinsic},
},
ssa_gen::Ssa,
Expand Down Expand Up @@ -229,7 +230,12 @@ impl FunctionBuilder {
) -> ValueId {
let lhs_type = self.type_of_value(lhs);
let rhs_type = self.type_of_value(rhs);
assert_eq!(lhs_type, rhs_type, "ICE - Binary instruction operands must have the same type");
if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
assert_eq!(
lhs_type, rhs_type,
"ICE - Binary instruction operands must have the same type"
);
}
let instruction = Instruction::Binary(Binary { lhs, rhs, operator });
self.insert_instruction(instruction, None).first()
}
Expand Down
27 changes: 2 additions & 25 deletions compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::BTreeSet;

use iter_extended::vecmap;
use noirc_frontend::monomorphization::ast::InlineType;

use super::basic_block::BasicBlockId;
use super::dfg::DataFlowGraph;
Expand All @@ -17,29 +18,14 @@ pub(crate) enum RuntimeType {
Brillig,
}

/// Represents how a RuntimeType::Acir function should be inlined.
/// This type is only relevant for ACIR functions as we do not inline any Brillig functions
#[derive(Default, Clone, Copy, PartialEq, Eq, Debug, Hash)]
pub(crate) enum InlineType {
/// The most basic entry point can expect all its functions to be inlined.
/// All function calls are expected to be inlined into a single ACIR.
#[default]
Inline,
/// Functions marked as foldable will not be inlined and compiled separately into ACIR
Fold,
}

impl RuntimeType {
/// Returns whether the runtime type represents an entry point.
/// We return `false` for InlineType::Inline on default, which is true
/// in all cases except for main. `main` should be supported with special
/// handling in any places where this function determines logic.
pub(crate) fn is_entry_point(&self) -> bool {
match self {
RuntimeType::Acir(inline_type) => match inline_type {
InlineType::Inline => false,
InlineType::Fold => true,
},
RuntimeType::Acir(inline_type) => inline_type.is_entry_point(),
RuntimeType::Brillig => true,
}
}
Expand Down Expand Up @@ -163,15 +149,6 @@ impl std::fmt::Display for RuntimeType {
}
}

impl std::fmt::Display for InlineType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
InlineType::Inline => write!(f, "inline"),
InlineType::Fold => write!(f, "fold"),
}
}
}

/// FunctionId is a reference for a function
///
/// This Id is how each function refers to other functions
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,12 +517,12 @@ impl<'function> PerFunctionContext<'function> {
#[cfg(test)]
mod test {
use acvm::FieldElement;
use noirc_frontend::monomorphization::ast::InlineType;

use crate::ssa::{
function_builder::FunctionBuilder,
ir::{
basic_block::BasicBlockId,
function::InlineType,
instruction::{BinaryOp, Intrinsic, TerminatorInstruction},
map::Id,
types::Type,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Context<'_> {
} 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());
self.numeric_constant(FieldElement::from(bit_size as u128), Type::unsigned(8));
let overflow = self.insert_binary(rhs, BinaryOp::Lt, bit_size_var);
let predicate = self.insert_cast(overflow, typ.clone());
// we can safely cast to unsigned because overflow_checks prevent bit-shift with a negative value
Expand Down
Loading

0 comments on commit e787045

Please sign in to comment.