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: Remove comptime and warn upon usage #2178

Merged
merged 15 commits into from
Aug 8, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct MyStruct<S> {
}

impl<S> MyStruct<S> {
fn insert(mut self: Self, index: comptime Field, elem: Field) -> Self {
fn insert(mut self: Self, index: Field, elem: Field) -> Self {
// Regression test for numeric generics on impls
assert(index as u64 < S as u64);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main(x: Field, len3: [u8; 3], len4: [Field; 4]) {
assert(add_lens(len3, len4) == 7);
assert(nested_call(len4) == 5);

// std::array::len returns a comptime value
// std::array::len returns a compile-time known value
assert(len4[len3.len()] == 4);

// Regression for #1023, ensure .len still works after calling to_le_bytes on a witness.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ fn main(x: u64) {
let two: u64 = 2;
let three: u64 = 3;

// comptime shifts on comptime values
// shifts on constant values
assert(two << 2 == 8);
assert((two << 3) / 8 == two);
assert((three >> 1) == 1);

// comptime shifts on runtime values
// shifts on runtime values
assert(x << 1 == 128);
assert(x >> 2 == 16);
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
fn main(x: u64, y: u64) {
// runtime shifts on comptime values
// runtime shifts on compile-time known values
assert(64 << y == 128);
assert(64 >> y == 32);

// runtime shifts on runtime values
assert(x << y == 128);
assert(x >> y == 32);
}
}

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn main(a: [Field; M + N - N], b: [Field; 30 + N / 2], c : pub [Field; foo::MAGI
let mut y = 5;
let mut x = M;
for i in 0..N*N {
let M: comptime Field = 10;
let M: Field = 10;
x = M;

y = i;
Expand Down Expand Up @@ -87,8 +87,8 @@ mod mysubmodule {
assert(x | y == 1);
}

fn my_helper() -> comptime Field {
let N: comptime Field = 15; // Like in Rust, local variables override globals
fn my_helper() -> Field {
let N: Field = 15; // Like in Rust, local variables override globals
let x = N;
x
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
global NIBBLE_LENGTH: comptime Field = 16;
global NIBBLE_LENGTH: Field = 16;

fn compact_decode<N>(input: [u8; N], length: Field) -> ([u4; NIBBLE_LENGTH], Field)
{
Expand Down Expand Up @@ -87,4 +87,4 @@ fn main(x: [u8; 5], z: Field)
assert(enc_val1.0 == [0x94,0xb8,0x8f,0x61,0xe6,0xfb,0xda,0x83,0xfb,0xff,0xfa,0xbe,0x36,0x41,0x12,0x13,0x74,0x80,0x39,0x80,0x18,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00]);
assert(enc_val1.1 == 21);

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl<'block> BrilligBlock<'block> {
self.create_block_label_for_current_function(*else_destination),
);
}
TerminatorInstruction::Jmp { destination, arguments } => {
TerminatorInstruction::Jmp { destination, arguments, location: _ } => {
let target = &dfg[*destination];
for (src, dest) in arguments.iter().zip(target.parameters()) {
// Destination variable might have already been created by another block that jumps to this target
Expand Down
41 changes: 20 additions & 21 deletions crates/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ pub enum RuntimeError {
UnInitialized { name: String, location: Option<Location> },
#[error("Integer sized {num_bits:?} is over the max supported size of {max_num_bits:?}")]
UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32, location: Option<Location> },
#[error("Could not determine loop bound at compile-time")]
UnknownLoopBound { location: Option<Location> },
}

#[derive(Debug, PartialEq, Eq, Clone, Error)]
Expand All @@ -50,34 +52,36 @@ pub enum InternalError {
UnExpected { expected: String, found: String, location: Option<Location> },
}

impl From<RuntimeError> for FileDiagnostic {
fn from(error: RuntimeError) -> Self {
match error {
RuntimeError::InternalError(ref ice_error) => match ice_error {
impl RuntimeError {
fn location(&self) -> Option<Location> {
match self {
RuntimeError::InternalError(
InternalError::DegreeNotReduced { location }
| InternalError::EmptyArray { location }
| InternalError::General { location, .. }
| InternalError::MissingArg { location, .. }
| InternalError::NotAConstant { location, .. }
| InternalError::UndeclaredAcirVar { location }
| InternalError::UnExpected { location, .. } => {
let file_id = location.map(|loc| loc.file).unwrap();
FileDiagnostic { file_id, diagnostic: error.into() }
}
},
RuntimeError::FailedConstraint { location, .. }
| InternalError::UnExpected { location, .. },
)
| RuntimeError::FailedConstraint { location, .. }
| RuntimeError::IndexOutOfBounds { location, .. }
| RuntimeError::InvalidRangeConstraint { location, .. }
| RuntimeError::TypeConversion { location, .. }
| RuntimeError::UnInitialized { location, .. }
| RuntimeError::UnsupportedIntegerSize { location, .. } => {
let file_id = location.map(|loc| loc.file).unwrap();
FileDiagnostic { file_id, diagnostic: error.into() }
}
| RuntimeError::UnknownLoopBound { location, .. }
| RuntimeError::UnsupportedIntegerSize { location, .. } => *location,
}
}
}

impl From<RuntimeError> for FileDiagnostic {
fn from(error: RuntimeError) -> Self {
let file_id = error.location().map(|loc| loc.file).unwrap();
FileDiagnostic { file_id, diagnostic: error.into() }
}
}

impl From<RuntimeError> for Diagnostic {
fn from(error: RuntimeError) -> Diagnostic {
match error {
Expand All @@ -87,13 +91,8 @@ impl From<RuntimeError> for Diagnostic {
"".to_string(),
noirc_errors::Span::new(0..0)
),
RuntimeError::FailedConstraint { location, .. }
| RuntimeError::IndexOutOfBounds { location, .. }
| RuntimeError::InvalidRangeConstraint { location, .. }
| RuntimeError::TypeConversion { location, .. }
| RuntimeError::UnInitialized { location, .. }
| RuntimeError::UnsupportedIntegerSize { location, .. } => {
let span = if let Some(loc) = location { loc.span } else { noirc_errors::Span::new(0..0) };
_ => {
let span = if let Some(loc) = error.location() { loc.span } else { noirc_errors::Span::new(0..0) };
Diagnostic::simple_error("".to_owned(), error.to_string(), span)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub(crate) fn optimize_into_acir(
ssa = ssa
.inline_functions()
.print(print_ssa_passes, "After Inlining:")
.unroll_loops()
.unroll_loops()?
.print(print_ssa_passes, "After Unrolling:")
.simplify_cfg()
.print(print_ssa_passes, "After Simplifying:")
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_evaluator/src/ssa/ir/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ mod tests {
func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp {
destination: ret_block_id,
arguments: vec![],
location: None,
});
func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,11 +408,11 @@ impl DataFlowGraph {
}

pub(crate) fn get_location(&self, id: &InstructionId) -> Option<Location> {
self.locations.get(id).cloned()
self.locations.get(id).copied()
}

pub(crate) fn get_value_location(&self, id: &ValueId) -> Option<Location> {
match &self.values[*id] {
match &self.values[self.resolve(*id)] {
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
Value::Instruction { instruction, .. } => self.get_location(instruction),
_ => None,
}
Expand Down
15 changes: 10 additions & 5 deletions crates/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use acvm::{acir::BlackBoxFunc, FieldElement};
use iter_extended::vecmap;
use noirc_errors::Location;
use num_bigint::BigUint;

use super::{
Expand Down Expand Up @@ -419,8 +420,10 @@ pub(crate) enum TerminatorInstruction {

/// Unconditional Jump
///
/// Jumps to specified `destination` with `arguments`
Jmp { destination: BasicBlockId, arguments: Vec<ValueId> },
/// Jumps to specified `destination` with `arguments`.
/// The optional Location here is expected to be used to issue an error when the start range of
/// a for loop cannot be deduced at compile-time.
Jmp { destination: BasicBlockId, arguments: Vec<ValueId>, location: Option<Location> },

/// Return from the current function with the given return values.
///
Expand All @@ -445,9 +448,11 @@ impl TerminatorInstruction {
then_destination: *then_destination,
else_destination: *else_destination,
},
Jmp { destination, arguments } => {
Jmp { destination: *destination, arguments: vecmap(arguments, |value| f(*value)) }
}
Jmp { destination, arguments, location } => Jmp {
destination: *destination,
arguments: vecmap(arguments, |value| f(*value)),
location: *location,
},
Return { return_values } => {
Return { return_values: vecmap(return_values, |value| f(*value)) }
}
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub(crate) fn display_terminator(
f: &mut Formatter,
) -> Result {
match terminator {
Some(TerminatorInstruction::Jmp { destination, arguments }) => {
Some(TerminatorInstruction::Jmp { destination, arguments, location: _ }) => {
writeln!(f, " jmp {}({})", destination, value_list(function, arguments))
}
Some(TerminatorInstruction::JmpIf { condition, then_destination, else_destination }) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'f> Context<'f> {
let end = self.branch_ends[&block];
self.inline_branch_end(end, then_branch, else_branch)
}
TerminatorInstruction::Jmp { destination, arguments } => {
TerminatorInstruction::Jmp { destination, arguments, location: _ } => {
if let Some((end_block, _)) = self.conditions.last() {
if destination == end_block {
return block;
Expand Down
8 changes: 6 additions & 2 deletions crates/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,14 @@ impl<'function> PerFunctionContext<'function> {
block_queue: &mut Vec<BasicBlockId>,
) -> Option<(BasicBlockId, Vec<ValueId>)> {
match self.source_function.dfg[block_id].unwrap_terminator() {
TerminatorInstruction::Jmp { destination, arguments } => {
TerminatorInstruction::Jmp { destination, arguments, location } => {
let destination = self.translate_block(*destination, block_queue);
let arguments = vecmap(arguments, |arg| self.translate_value(*arg));
self.context.builder.terminate_with_jmp(destination, arguments);
self.context.builder.terminate_with_jmp_with_location(
destination,
arguments,
*location,
);
None
}
TerminatorInstruction::JmpIf { condition, then_destination, else_destination } => {
Expand Down
3 changes: 2 additions & 1 deletion crates/noirc_evaluator/src/ssa/opt/simplify_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ fn check_for_constant_jmpif(
let destination =
if constant.is_zero() { *else_destination } else { *then_destination };

let jmp = TerminatorInstruction::Jmp { destination, arguments: Vec::new() };
let arguments = Vec::new();
let jmp = TerminatorInstruction::Jmp { destination, arguments, location: None };
function.dfg[block].set_terminator(jmp);
cfg.recompute_block(function, block);
}
Expand Down
Loading