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: revert merge changes, compile time down from 90 to 10 mins #10341

Merged
merged 5 commits into from
Dec 3, 2024
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
7 changes: 5 additions & 2 deletions noir-projects/noir-protocol-circuits/crates/blob/src/blob.nr
Original file line number Diff line number Diff line change
Expand Up @@ -482,11 +482,14 @@ mod tests {
assert_eq(lhs, rhs);
}

// TODO: After reverting some noir changes (see PR#10341) the below no longer works in unconstrained
// This happened previously in commit 893f1eca422d952d672eec75e3c9ff8f149a9ae8 but a sync fixed it.
// It works and passes in constrained, just takes a min longer.
#[test]
unconstrained fn test_base() {
fn test_base() {
let mut tx_data = FixtureBuilder::new();
// Add some random bits of state
tx_data.append_note_hashes_with_logs(50);
tx_data.append_note_hashes(50);
tx_data.set_first_nullifier();
tx_data.append_nullifiers(50);
tx_data.append_l2_to_l1_msgs(5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ mod tests {
array_concat([TX_FEE_PREFIX, 0], (0).to_be_bytes::<29>()),
true,
);
tx_effects[2] = field_from_bytes([NOTES_PREFIX, 0, 50 as u8], true);
tx_effects[2] = encode_blob_prefix(NOTES_PREFIX, 50);
for i in 0..50 {
tx_effects[i + 3] = builder.tube_data.note_hashes.storage()[i].value();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ mod tests {
array_concat([TX_FEE_PREFIX, 0], (0).to_be_bytes::<29>()),
true,
);
tx_effects[2] = field_from_bytes([NOTES_PREFIX, 0, 50 as u8], true);
tx_effects[2] = encode_blob_prefix(NOTES_PREFIX, 50);
for i in 0..50 {
tx_effects[i + 3] = builder.avm_data.note_hashes.storage()[i].value();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl BlockRootRollupInputs {
previous_block_hash: self.previous_block_hash,
end_block_hash: block_hash, // current newest block hash = this block hash
start_global_variables: left.constants.global_variables, // we have asserted that left.constants == right.constants => ...
end_global_variables: left.constants.global_variables, // with a current block range of 1, we only have 1 set of constants
end_global_variables: left.constants.global_variables, // ...with a current block range of 1, we only have 1 set of constants
out_hash: content_commitment.out_hash,
fees: fee_arr,
vk_tree_root: left.constants.vk_tree_root,
Expand Down
27 changes: 19 additions & 8 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,12 @@ pub(crate) enum Instruction {
/// else_value
/// }
/// ```
IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId },
IfElse {
then_condition: ValueId,
then_value: ValueId,
else_condition: ValueId,
else_value: ValueId,
},

/// Creates a new array or slice.
///
Expand Down Expand Up @@ -608,11 +613,14 @@ impl Instruction {
assert_message: assert_message.clone(),
}
}
Instruction::IfElse { then_condition, then_value, else_value } => Instruction::IfElse {
then_condition: f(*then_condition),
then_value: f(*then_value),
else_value: f(*else_value),
},
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
Instruction::IfElse {
then_condition: f(*then_condition),
then_value: f(*then_value),
else_condition: f(*else_condition),
else_value: f(*else_value),
}
}
Instruction::MakeArray { elements, typ } => Instruction::MakeArray {
elements: elements.iter().copied().map(f).collect(),
typ: typ.clone(),
Expand Down Expand Up @@ -671,9 +679,10 @@ impl Instruction {
| Instruction::RangeCheck { value, .. } => {
f(*value);
}
Instruction::IfElse { then_condition, then_value, else_value } => {
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
f(*then_condition);
f(*then_value);
f(*else_condition);
f(*else_value);
}
Instruction::MakeArray { elements, typ: _ } => {
Expand Down Expand Up @@ -836,7 +845,7 @@ impl Instruction {
None
}
}
Instruction::IfElse { then_condition, then_value, else_value } => {
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
let typ = dfg.type_of_value(*then_value);

if let Some(constant) = dfg.get_numeric_constant(*then_condition) {
Expand All @@ -855,11 +864,13 @@ impl Instruction {

if matches!(&typ, Type::Numeric(_)) {
let then_condition = *then_condition;
let else_condition = *else_condition;

let result = ValueMerger::merge_numeric_values(
dfg,
block,
then_condition,
else_condition,
then_value,
else_value,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,12 @@ fn simplify_slice_push_back(
let mut value_merger =
ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None, call_stack);

let new_slice =
value_merger.merge_values(len_not_equals_capacity, set_last_slice_value, new_slice);
let new_slice = value_merger.merge_values(
len_not_equals_capacity,
len_equals_capacity,
set_last_slice_value,
new_slice,
);

SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
}
Expand Down
8 changes: 6 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,15 @@ fn display_instruction_inner(
Instruction::RangeCheck { value, max_bit_size, .. } => {
writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
}
Instruction::IfElse { then_condition, then_value, else_value } => {
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
let then_condition = show(*then_condition);
let then_value = show(*then_value);
let else_condition = show(*else_condition);
let else_value = show(*else_value);
writeln!(f, "if {then_condition} then {then_value} else {else_value}")
writeln!(
f,
"if {then_condition} then {then_value} else if {else_condition} then {else_value}"
)
}
Instruction::MakeArray { elements, typ } => {
write!(f, "make_array [")?;
Expand Down
Loading
Loading