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

fix: Add checks for nop #1160

Merged
merged 2 commits into from
Apr 17, 2023
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
5 changes: 5 additions & 0 deletions crates/nargo_cli/tests/test_data/regression/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
2 changes: 2 additions & 0 deletions crates/nargo_cli/tests/test_data/regression/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = [0x3f, 0x1c, 0xb8, 0x99, 0xab]
z = 3
56 changes: 56 additions & 0 deletions crates/nargo_cli/tests/test_data/regression/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
global NIBBLE_LENGTH: comptime Field = 16;

fn compact_decode<N>(input: [u8; N], length: Field) -> ([u4; NIBBLE_LENGTH], Field)
{
constrain 2*input.len() as u64 <= NIBBLE_LENGTH as u64;
constrain length as u64 <= input.len() as u64;

let mut nibble = [0 as u4; NIBBLE_LENGTH];

let first_nibble = (input[0] >> 4) as u4;
let parity = first_nibble as u1;

if parity == 1
{
nibble[0] = (input[0] & 0x0f) as u4;
for i in 1..input.len()
{
if i as u64 < length as u64
{
let x = input[i];
nibble[2*i - 1] = (x >> 4) as u4;
nibble[2*i] = (x & 0x0f) as u4;
}
}
}
else
{
for i in 0..2
{
if (i as u64) < length as u64 - 1
{
let x = input[i + 1];
nibble[2*i] = (x >> 4) as u4;
nibble[2*i + 1] = (x & 0x0f) as u4;
}
}
}

let out = (nibble, 2*length + (parity as Field) - 2);

out
}

fn main(x: [u8; 5], z: Field)
{
//Issue 1144
let (nib, len) = compact_decode(x,z);
constrain len == 5;
constrain [nib[0], nib[1], nib[2], nib[3], nib[4]] == [15, 1, 12, 11, 8];
}

#[test]
fn test_1144()
{
main([0x3f, 0x1c, 0xb8, 0x99, 0xab], 3);
}
7 changes: 5 additions & 2 deletions crates/noirc_evaluator/src/ssa/acir_gen/internal_var_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,11 @@ impl InternalVarCache {
let w = evaluator.create_intermediate_variable(Expression::from(value));
for &id in ids {
let mut cached_var = self.get_or_compute_internal_var_unwrap(id, evaluator, ctx);
assert!(cached_var.cached_witness().is_none());
cached_var.set_witness(w);
if let Some(cached_witness) = cached_var.cached_witness() {
assert_eq!(*cached_witness, w);
} else {
cached_var.set_witness(w);
}
self.update(cached_var);
}
w
Expand Down
13 changes: 12 additions & 1 deletion crates/noirc_evaluator/src/ssa/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,18 @@ pub(super) fn merge_path(
removed_blocks.push_back(next);

if short_circuit.is_dummy() {
instructions.extend(&block.instructions);
if instructions.is_empty() {
instructions.extend(&block.instructions);
} else {
let nonop = block.instructions.iter().filter(|&i| {
if let Some(ins) = ctx.try_get_instruction(*i) {
ins.operation.opcode() != Opcode::Nop
} else {
true
}
});
instructions.extend(nonop);
}
}

if short_circuit.is_dummy() && block.is_short_circuit(ctx, assumption) {
Expand Down
10 changes: 8 additions & 2 deletions crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ use noirc_frontend::monomorphization::ast::{Definition, Expression, FuncId, Lite
use num_bigint::BigUint;
use std::collections::{HashMap, HashSet};

use super::node::Opcode;

// This is a 'master' class for generating the SSA IR from the AST
// It contains all the data; the node objects representing the source code in the nodes arena
// and The CFG in the blocks arena
Expand Down Expand Up @@ -726,7 +728,6 @@ impl SsaContext {
inline::inline_tree(self, self.first_block, &decision)?;

block::merge_path(self, self.first_block, BlockId::dummy(), None)?;

//The CFG is now fully flattened, so we keep only the first block.
let mut to_remove = Vec::new();
for b in &self.blocks {
Expand All @@ -741,7 +742,6 @@ impl SsaContext {
self[first_block].dominated.clear();

optimizations::cse(self, first_block, true)?;

//Truncation
integer::overflow_strategy(self)?;
self.log(enable_logging, "\noverflow:", "");
Expand Down Expand Up @@ -1072,6 +1072,12 @@ impl SsaContext {
if a == NodeId::dummy() || b == NodeId::dummy() {
return NodeId::dummy();
}
if let Some(ins) = self.try_get_instruction(a) {
if ins.operation.opcode() == Opcode::Nop {
assert_eq!(self.try_get_instruction(b).unwrap().operation.opcode(), Opcode::Nop);
return NodeId::dummy();
}
}

let exit_block = self.current_block;
let block1 = self[exit_block].predecessor[0];
Expand Down
5 changes: 5 additions & 0 deletions crates/noirc_evaluator/src/ssa/optimizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,11 @@ fn cse_block_with_anchor(
new_list.push(*ins_id);
}
}
Operation::Nop => {
if new_list.is_empty() {
new_list.push(*ins_id);
}
}
_ => {
//TODO: checks we do not need to propagate res arguments
new_list.push(*ins_id);
Expand Down