Skip to content

Commit

Permalink
Merge branch 'master' into michaeljklein/proptest-arithmetic-generics
Browse files Browse the repository at this point in the history 
* master:
  feat!: Always Check Arithmetic Generics at Monomorphization (#6329)
  chore: split path and import lookups (#6430)
  fix(ssa): Resolve value IDs in terminator before comparing to array (#6448)
  fix: right shift is not a regular division (#6400)
  • Loading branch information
@TomAFrench
TomAFrench committed Nov 5, 2024
2 parents b0ab542 + 2972db2 commit 8d62cab
Show file tree
Hide file tree
Showing 47 changed files with 1,646 additions and 1,171 deletions.
20 changes: 17 additions & 3 deletions compiler/noirc_driver/src/abi_gen.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use iter_extended::vecmap;
use noirc_abi::{
Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign,
};
use noirc_errors::Span;
use noirc_frontend::ast::{Signedness, Visibility};
use noirc_frontend::TypeBinding;
use noirc_frontend::{
Expand Down Expand Up @@ -39,10 +40,20 @@ pub(super) fn gen_abi(
Abi { parameters, return_type, error_types }
}

// Get the Span of the root crate's main function, or else a dummy span if that fails
fn get_main_function_span(context: &Context) -> Span {
if let Some(func_id) = context.get_main_function(context.root_crate_id()) {
context.function_meta(&func_id).location.span
} else {
Span::default()
}
}

fn build_abi_error_type(context: &Context, typ: &Type) -> AbiErrorType {
match typ {
Type::FmtString(len, item_types) => {
let length = len.evaluate_to_u32().expect("Cannot evaluate fmt length");
let span = get_main_function_span(context);
let length = len.evaluate_to_u32(span).expect("Cannot evaluate fmt length");
let Type::Tuple(item_types) = item_types.as_ref() else {
unreachable!("FmtString items must be a tuple")
};
Expand All @@ -58,8 +69,9 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
match typ {
Type::FieldElement => AbiType::Field,
Type::Array(size, typ) => {
let span = get_main_function_span(context);
let length = size
.evaluate_to_u32()
.evaluate_to_u32(span)
.expect("Cannot have variable sized arrays as a parameter to main");
let typ = typ.as_ref();
AbiType::Array { length, typ: Box::new(abi_type_from_hir_type(context, typ)) }
Expand All @@ -86,8 +98,9 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
}
Type::Bool => AbiType::Boolean,
Type::String(size) => {
let span = get_main_function_span(context);
let size = size
.evaluate_to_u32()
.evaluate_to_u32(span)
.expect("Cannot have variable sized strings as a parameter to main");
AbiType::String { length: size }
}
Expand All @@ -102,6 +115,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
AbiType::Struct { fields, path }
}
Type::Alias(def, args) => abi_type_from_hir_type(context, &def.borrow().get_type(args)),
Type::CheckedCast { to, .. } => abi_type_from_hir_type(context, to),
Type::Tuple(fields) => {
let fields = vecmap(fields, |typ| abi_type_from_hir_type(context, typ));
AbiType::Tuple { fields }
Expand Down
156 changes: 61 additions & 95 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::brillig::brillig_ir::artifact::Label;
use crate::brillig::brillig_ir::brillig_variable::{
type_to_heap_value_type, BrilligArray, BrilligVariable, SingleAddrVariable,
};

use crate::brillig::brillig_ir::registers::Stack;
use crate::brillig::brillig_ir::{
BrilligBinaryOp, BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
Expand Down Expand Up @@ -1202,7 +1203,7 @@ impl<'block> BrilligBlock<'block> {
let brillig_binary_op = match binary.operator {
BinaryOp::Div => {
if is_signed {
self.convert_signed_division(left, right, result_variable);
self.brillig_context.convert_signed_division(left, right, result_variable);
return;
} else if is_field {
BrilligBinaryOp::FieldDiv
Expand Down Expand Up @@ -1234,7 +1235,14 @@ impl<'block> BrilligBlock<'block> {
BinaryOp::Or => BrilligBinaryOp::Or,
BinaryOp::Xor => BrilligBinaryOp::Xor,
BinaryOp::Shl => BrilligBinaryOp::Shl,
BinaryOp::Shr => BrilligBinaryOp::Shr,
BinaryOp::Shr => {
if is_signed {
self.convert_signed_shr(left, right, result_variable);
return;
} else {
BrilligBinaryOp::Shr
}
}
};

self.brillig_context.binary_instruction(left, right, result_variable, brillig_binary_op);
Expand All @@ -1250,98 +1258,6 @@ impl<'block> BrilligBlock<'block> {
);
}

/// Splits a two's complement signed integer in the sign bit and the absolute value.
/// For example, -6 i8 (11111010) is split to 00000110 (6, absolute value) and 1 (is_negative).
fn absolute_value(
&mut self,
num: SingleAddrVariable,
absolute_value: SingleAddrVariable,
result_is_negative: SingleAddrVariable,
) {
let max_positive = self
.brillig_context
.make_constant_instruction(((1_u128 << (num.bit_size - 1)) - 1).into(), num.bit_size);

// Compute if num is negative
self.brillig_context.binary_instruction(
max_positive,
num,
result_is_negative,
BrilligBinaryOp::LessThan,
);

// Two's complement of num
let zero = self.brillig_context.make_constant_instruction(0_usize.into(), num.bit_size);
let twos_complement =
SingleAddrVariable::new(self.brillig_context.allocate_register(), num.bit_size);
self.brillig_context.binary_instruction(zero, num, twos_complement, BrilligBinaryOp::Sub);

// absolute_value = result_is_negative ? twos_complement : num
self.brillig_context.codegen_branch(result_is_negative.address, |ctx, is_negative| {
if is_negative {
ctx.mov_instruction(absolute_value.address, twos_complement.address);
} else {
ctx.mov_instruction(absolute_value.address, num.address);
}
});

self.brillig_context.deallocate_single_addr(zero);
self.brillig_context.deallocate_single_addr(max_positive);
self.brillig_context.deallocate_single_addr(twos_complement);
}

fn convert_signed_division(
&mut self,
left: SingleAddrVariable,
right: SingleAddrVariable,
result: SingleAddrVariable,
) {
let left_is_negative = SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
let left_abs_value =
SingleAddrVariable::new(self.brillig_context.allocate_register(), left.bit_size);

let right_is_negative =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
let right_abs_value =
SingleAddrVariable::new(self.brillig_context.allocate_register(), right.bit_size);

let result_is_negative =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);

// Compute both absolute values
self.absolute_value(left, left_abs_value, left_is_negative);
self.absolute_value(right, right_abs_value, right_is_negative);

// Perform the division on the absolute values
self.brillig_context.binary_instruction(
left_abs_value,
right_abs_value,
result,
BrilligBinaryOp::UnsignedDiv,
);

// Compute result sign
self.brillig_context.binary_instruction(
left_is_negative,
right_is_negative,
result_is_negative,
BrilligBinaryOp::Xor,
);

// If result has to be negative, perform two's complement
self.brillig_context.codegen_if(result_is_negative.address, |ctx| {
let zero = ctx.make_constant_instruction(0_usize.into(), result.bit_size);
ctx.binary_instruction(zero, result, result, BrilligBinaryOp::Sub);
ctx.deallocate_single_addr(zero);
});

self.brillig_context.deallocate_single_addr(left_is_negative);
self.brillig_context.deallocate_single_addr(left_abs_value);
self.brillig_context.deallocate_single_addr(right_is_negative);
self.brillig_context.deallocate_single_addr(right_abs_value);
self.brillig_context.deallocate_single_addr(result_is_negative);
}

fn convert_signed_modulo(
&mut self,
left: SingleAddrVariable,
Expand All @@ -1354,7 +1270,7 @@ impl<'block> BrilligBlock<'block> {
SingleAddrVariable::new(self.brillig_context.allocate_register(), left.bit_size);

// i = left / right
self.convert_signed_division(left, right, scratch_var_i);
self.brillig_context.convert_signed_division(left, right, scratch_var_i);

// j = i * right
self.brillig_context.binary_instruction(
Expand Down Expand Up @@ -1401,6 +1317,56 @@ impl<'block> BrilligBlock<'block> {
self.brillig_context.deallocate_single_addr(bias);
}

fn convert_signed_shr(
&mut self,
left: SingleAddrVariable,
right: SingleAddrVariable,
result: SingleAddrVariable,
) {
// Check if left is negative
let left_is_negative = SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
let max_positive = self
.brillig_context
.make_constant_instruction(((1_u128 << (left.bit_size - 1)) - 1).into(), left.bit_size);
self.brillig_context.binary_instruction(
max_positive,
left,
left_is_negative,
BrilligBinaryOp::LessThan,
);

self.brillig_context.codegen_branch(left_is_negative.address, |ctx, is_negative| {
if is_negative {
let one = ctx.make_constant_instruction(1_u128.into(), left.bit_size);

// computes 2^right
let two = ctx.make_constant_instruction(2_u128.into(), left.bit_size);
let two_pow = ctx.make_constant_instruction(1_u128.into(), left.bit_size);
let right_u32 = SingleAddrVariable::new(ctx.allocate_register(), 32);
ctx.cast(right_u32, right);
let pow_body = |ctx: &mut BrilligContext<_, _>, _: SingleAddrVariable| {
ctx.binary_instruction(two_pow, two, two_pow, BrilligBinaryOp::Mul);
};
ctx.codegen_for_loop(None, right_u32.address, None, pow_body);

// Right shift using division on 1-complement
ctx.binary_instruction(left, one, result, BrilligBinaryOp::Add);
ctx.convert_signed_division(result, two_pow, result);
ctx.binary_instruction(result, one, result, BrilligBinaryOp::Sub);

// Clean-up
ctx.deallocate_single_addr(one);
ctx.deallocate_single_addr(two);
ctx.deallocate_single_addr(two_pow);
ctx.deallocate_single_addr(right_u32);
} else {
ctx.binary_instruction(left, right, result, BrilligBinaryOp::Shr);
}
});

self.brillig_context.deallocate_single_addr(left_is_negative);
}

#[allow(clippy::too_many_arguments)]
fn add_overflow_check(
&mut self,
Expand Down
82 changes: 82 additions & 0 deletions compiler/noirc_evaluator/src/brillig/brillig_ir.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod entry_point;
mod instructions;

use artifact::Label;
use brillig_variable::SingleAddrVariable;
pub(crate) use instructions::BrilligBinaryOp;
use registers::{RegisterAllocator, ScratchSpace};

Expand Down Expand Up @@ -109,6 +110,87 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
can_call_procedures: true,
}
}

/// Splits a two's complement signed integer in the sign bit and the absolute value.
/// For example, -6 i8 (11111010) is split to 00000110 (6, absolute value) and 1 (is_negative).
pub(crate) fn absolute_value(
&mut self,
num: SingleAddrVariable,
absolute_value: SingleAddrVariable,
result_is_negative: SingleAddrVariable,
) {
let max_positive = self
.make_constant_instruction(((1_u128 << (num.bit_size - 1)) - 1).into(), num.bit_size);

// Compute if num is negative
self.binary_instruction(max_positive, num, result_is_negative, BrilligBinaryOp::LessThan);

// Two's complement of num
let zero = self.make_constant_instruction(0_usize.into(), num.bit_size);
let twos_complement = SingleAddrVariable::new(self.allocate_register(), num.bit_size);
self.binary_instruction(zero, num, twos_complement, BrilligBinaryOp::Sub);

// absolute_value = result_is_negative ? twos_complement : num
self.codegen_branch(result_is_negative.address, |ctx, is_negative| {
if is_negative {
ctx.mov_instruction(absolute_value.address, twos_complement.address);
} else {
ctx.mov_instruction(absolute_value.address, num.address);
}
});

self.deallocate_single_addr(zero);
self.deallocate_single_addr(max_positive);
self.deallocate_single_addr(twos_complement);
}

pub(crate) fn convert_signed_division(
&mut self,
left: SingleAddrVariable,
right: SingleAddrVariable,
result: SingleAddrVariable,
) {
let left_is_negative = SingleAddrVariable::new(self.allocate_register(), 1);
let left_abs_value = SingleAddrVariable::new(self.allocate_register(), left.bit_size);

let right_is_negative = SingleAddrVariable::new(self.allocate_register(), 1);
let right_abs_value = SingleAddrVariable::new(self.allocate_register(), right.bit_size);

let result_is_negative = SingleAddrVariable::new(self.allocate_register(), 1);

// Compute both absolute values
self.absolute_value(left, left_abs_value, left_is_negative);
self.absolute_value(right, right_abs_value, right_is_negative);

// Perform the division on the absolute values
self.binary_instruction(
left_abs_value,
right_abs_value,
result,
BrilligBinaryOp::UnsignedDiv,
);

// Compute result sign
self.binary_instruction(
left_is_negative,
right_is_negative,
result_is_negative,
BrilligBinaryOp::Xor,
);

// If result has to be negative, perform two's complement
self.codegen_if(result_is_negative.address, |ctx| {
let zero = ctx.make_constant_instruction(0_usize.into(), result.bit_size);
ctx.binary_instruction(zero, result, result, BrilligBinaryOp::Sub);
ctx.deallocate_single_addr(zero);
});

self.deallocate_single_addr(left_is_negative);
self.deallocate_single_addr(left_abs_value);
self.deallocate_single_addr(right_is_negative);
self.deallocate_single_addr(right_abs_value);
self.deallocate_single_addr(result_is_negative);
}
}

/// Special brillig context to codegen compiler intrinsic shared procedures
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,8 @@ fn split_public_and_private_inputs(
func_sig
.0
.iter()
.map(|(_, typ, visibility)| {
let num_field_elements_needed = typ.field_count() as usize;
.map(|(pattern, typ, visibility)| {
let num_field_elements_needed = typ.field_count(&pattern.location()) as usize;
let witnesses = input_witnesses[idx..idx + num_field_elements_needed].to_vec();
idx += num_field_elements_needed;
(visibility, witnesses)
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl DataBusBuilder {
ast::Visibility::CallData(id) => DatabusVisibility::CallData(id),
ast::Visibility::ReturnData => DatabusVisibility::ReturnData,
};
let len = param.1.field_count() as usize;
let len = param.1.field_count(&param.0.location()) as usize;
params_is_databus.extend(vec![is_databus; len]);
}
params_is_databus
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ pub(crate) struct DataFlowGraph {
blocks: DenseMap<BasicBlock>,

/// Debugging information about which `ValueId`s have had their underlying `Value` substituted
/// for that of another. This information is purely used for printing the SSA, and has no
/// material effect on the SSA itself.
/// for that of another. In theory this information is purely used for printing the SSA,
/// and has no material effect on the SSA itself, however in practice the IDs can get out of
/// sync and may need this resolution before they can be compared.
#[serde(skip)]
replaced_value_ids: HashMap<ValueId, ValueId>,

Expand Down
Loading

0 comments on commit 8d62cab

Please sign in to comment.