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

feat: Add support for bitshifts by distances known at runtime #2072

Merged
merged 12 commits into from
Aug 2, 2023
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[package]
name = "bit_shifts_runtime"
authors = [""]
compiler_version = "0.1"

[dependencies]
[dependencies]
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
x = 64
y = 1
y = 1
22 changes: 11 additions & 11 deletions crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ impl<'block> BrilligBlock<'block> {
dfg.instruction_results(instruction_id)[0],
dfg,
);

let heap_vec = self.brillig_context.extract_heap_vector(target_slice);
self.brillig_context.radix_instruction(
source,
self.function_context.extract_heap_vector(target_slice),
heap_vec,
radix,
limb_count,
matches!(endianness, Endian::Big),
Expand All @@ -355,10 +355,10 @@ impl<'block> BrilligBlock<'block> {
);

let radix = self.brillig_context.make_constant(2_usize.into());

let heap_vec = self.brillig_context.extract_heap_vector(target_slice);
self.brillig_context.radix_instruction(
source,
self.function_context.extract_heap_vector(target_slice),
heap_vec,
radix,
limb_count,
matches!(endianness, Endian::Big),
Expand Down Expand Up @@ -589,7 +589,7 @@ impl<'block> BrilligBlock<'block> {
dfg.instruction_results(instruction_id)[0],
dfg,
);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let target_vector = self.brillig_context.extract_heap_vector(target_variable);
let item_value = self.convert_ssa_register_value(arguments[1], dfg);
slice_push_back_operation(
self.brillig_context,
Expand All @@ -604,7 +604,7 @@ impl<'block> BrilligBlock<'block> {
dfg.instruction_results(instruction_id)[0],
dfg,
);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let target_vector = self.brillig_context.extract_heap_vector(target_variable);
let item_value = self.convert_ssa_register_value(arguments[1], dfg);
slice_push_front_operation(
self.brillig_context,
Expand All @@ -618,7 +618,7 @@ impl<'block> BrilligBlock<'block> {

let target_variable =
self.function_context.create_variable(self.brillig_context, results[0], dfg);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let target_vector = self.brillig_context.extract_heap_vector(target_variable);

let pop_item = self.function_context.create_register_variable(
self.brillig_context,
Expand All @@ -643,7 +643,7 @@ impl<'block> BrilligBlock<'block> {
);
let target_variable =
self.function_context.create_variable(self.brillig_context, results[1], dfg);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let target_vector = self.brillig_context.extract_heap_vector(target_variable);

slice_pop_front_operation(
self.brillig_context,
Expand All @@ -659,7 +659,7 @@ impl<'block> BrilligBlock<'block> {
let target_variable =
self.function_context.create_variable(self.brillig_context, results[0], dfg);

let target_vector = self.function_context.extract_heap_vector(target_variable);
let target_vector = self.brillig_context.extract_heap_vector(target_variable);
slice_insert_operation(
self.brillig_context,
target_vector,
Expand All @@ -674,7 +674,7 @@ impl<'block> BrilligBlock<'block> {

let target_variable =
self.function_context.create_variable(self.brillig_context, results[0], dfg);
let target_vector = self.function_context.extract_heap_vector(target_variable);
let target_vector = self.brillig_context.extract_heap_vector(target_variable);

let removed_item_register = self.function_context.create_register_variable(
self.brillig_context,
Expand Down Expand Up @@ -862,7 +862,7 @@ impl<'block> BrilligBlock<'block> {
Type::Slice(_) => {
let variable =
self.function_context.create_variable(self.brillig_context, result, dfg);
let vector = self.function_context.extract_heap_vector(variable);
let vector = self.brillig_context.extract_heap_vector(variable);

// Set the pointer to the current stack frame
// The stack pointer will then be update by the caller of this method
Expand Down
7 changes: 0 additions & 7 deletions crates/noirc_evaluator/src/brillig/brillig_gen/brillig_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,6 @@ impl FunctionContext {
}
}

pub(crate) fn extract_heap_vector(&self, variable: RegisterOrMemory) -> HeapVector {
match variable {
RegisterOrMemory::HeapVector(vector) => vector,
_ => unreachable!("ICE: Expected vector, got {variable:?}"),
}
}

/// Collects the registers that a given variable is stored in.
pub(crate) fn extract_registers(&self, variable: RegisterOrMemory) -> Vec<RegisterIndex> {
match variable {
Expand Down
12 changes: 12 additions & 0 deletions crates/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,18 @@ impl BrilligContext {
self.deallocate_register(end_value_register);
self.deallocate_register(index_at_end_of_array);
}

pub(crate) fn extract_heap_vector(&mut self, variable: RegisterOrMemory) -> HeapVector {
match variable {
RegisterOrMemory::HeapVector(vector) => vector,
RegisterOrMemory::HeapArray(array) => {
let size = self.allocate_register();
self.const_instruction(size, array.size.into());
HeapVector { pointer: array.pointer, size }
}
_ => unreachable!("ICE: Expected vector, got {variable:?}"),
}
}
}

/// Type to encapsulate the binary operation types in Brillig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl DebugToString for BinaryIntOp {
BinaryIntOp::Or => "||".into(),
BinaryIntOp::Xor => "^".into(),
BinaryIntOp::Shl | BinaryIntOp::Shr => {
unreachable!("bit shift should haver been replaced")
unreachable!("bit shift should have been replaced")
}
}
}
Expand Down
12 changes: 9 additions & 3 deletions crates/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
types::Type,
},
node_interner::{ExprId, FuncId},
CompTime, Shared, TypeBinding, TypeVariableKind, UnaryOp,
CompTime, Shared, Signedness, TypeBinding, TypeVariableKind, UnaryOp,
};

use super::{errors::TypeCheckError, TypeChecker};
Expand Down Expand Up @@ -916,8 +916,14 @@ impl<'interner> TypeChecker<'interner> {
span,
});
}
let comptime = comptime_x.and(comptime_y, op.location.span);
Ok(Integer(comptime, *sign_x, *bit_width_x))
if op.is_bit_shift()
&& (*sign_x != Signedness::Unsigned || *sign_y != Signedness::Unsigned)
jfecher marked this conversation as resolved.
Show resolved Hide resolved
guipublic marked this conversation as resolved.
Show resolved Hide resolved
{
Err(TypeCheckError::InvalidInfixOp { kind: "Signed integer", span })
} else {
let comptime = comptime_x.and(comptime_y, op.location.span);
Ok(Integer(comptime, *sign_x, *bit_width_x))
}
}
(Integer(..), FieldElement(..)) | (FieldElement(..), Integer(..)) => {
Err(TypeCheckError::IntegerAndFieldBinaryOperation { span })
Expand Down
5 changes: 5 additions & 0 deletions crates/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ impl HirBinaryOp {
use BinaryOpKind::*;
matches!(self.kind, And | Or | Xor | ShiftRight | ShiftLeft)
}

pub fn is_bit_shift(&self) -> bool {
use BinaryOpKind::*;
matches!(self.kind, ShiftRight | ShiftLeft)
}
}

#[derive(Debug, Clone)]
Expand Down