Skip to content

Commit 372ce58

Browse files
authored
Revert "Optimization: lazy conversion of circuit outputs to struct form (#1348)" (#1397)
This reverts commit c7a9a47.
1 parent 38106a1 commit 372ce58

File tree

2 files changed

+65
-69
lines changed

2 files changed

+65
-69
lines changed

src/libfuncs/circuit.rs

Lines changed: 58 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ fn build_eval<'ctx, 'this>(
313313
let circuit_data = entry.arg(3)?;
314314
let circuit_modulus = entry.arg(4)?;
315315

316-
// Arguments 5 and 6 are used to build the gate 0 (with constant value 1).
316+
// arguments 5 and 6 are used to build the gate 0 (with constant value 1)
317317
// let zero = entry.argument(5)?;
318318
// let one = entry.argument(6)?;
319319

@@ -361,11 +361,19 @@ fn build_eval<'ctx, 'this>(
361361
circuit_info.mul_offsets.len() * MUL_MOD_BUILTIN_SIZE,
362362
)?;
363363

364-
// Calculate capacity for array.
364+
// convert circuit output from integer representation to struct representation
365+
let gates = gates
366+
.into_iter()
367+
.map(|value| u384_integer_to_struct(context, ok_block, location, value))
368+
.collect::<Result<Vec<_>>>()?;
369+
370+
// Calculate full capacity for array.
365371
let outputs_capacity = circuit_info.values.len();
366-
let u384_integer_layout = get_integer_layout(384);
367-
let outputs_layout = layout_repeat(&u384_integer_layout, outputs_capacity)?.0;
368-
let outputs_capacity_bytes = outputs_layout.pad_to_align().size();
372+
let u384_struct_layout = layout_repeat(&get_integer_layout(96), 4)?.0;
373+
let outputs_capacity_bytes = layout_repeat(&u384_struct_layout, outputs_capacity)?
374+
.0
375+
.pad_to_align()
376+
.size();
369377
let outputs_capacity_bytes_value =
370378
ok_block.const_int(context, location, outputs_capacity_bytes, 64)?;
371379

@@ -379,14 +387,13 @@ fn build_eval<'ctx, 'this>(
379387
location,
380388
)?)?;
381389

382-
// Insert evaluated gates into the array.
383390
for (i, gate) in gates.into_iter().enumerate() {
384391
let value_ptr = ok_block.gep(
385392
context,
386393
location,
387394
outputs_ptr,
388395
&[GepIndex::Const(i as i32)],
389-
IntegerType::new(context, 384).into(),
396+
build_u384_struct_type(context),
390397
)?;
391398
ok_block.store(context, location, value_ptr, gate)?;
392399
}
@@ -457,24 +464,12 @@ fn build_eval<'ctx, 'this>(
457464
Ok(())
458465
}
459466

460-
/// Receives the circuit inputs, and builds the evaluation of the full circuit.
461-
///
462-
/// Returns two branches. The success block and the error block respectively.
463-
/// - The success block receives nothing.
464-
/// - The error block receives:
465-
/// - The index of the first gate that could not be computed.
466-
///
467-
/// The evaluated gates are returned separately, as a vector of `MLIR` values.
468-
/// Note that in the case of error, not all MLIR values are guaranteed to have been computed,
469-
/// and should not be used carelessly.
467+
/// Builds the evaluation of all circuit gates, returning:
468+
/// - An array of two branches, the success block and the error block respectively.
469+
/// - The error block contains the index of the first failure as argument.
470+
/// - A vector of the gate values. In case of failure, not all values are guaranteed to be computed.
470471
///
471-
/// TODO: Consider returning the evaluated gates through the block directly:
472-
/// - As a pointer to a heap allocated array of gates.
473-
/// - As a llvm struct/array of evaluted gates (its size could get really big).
474-
/// - As arguments to the block (one argument per block).
475-
///
476-
/// The original Cairo hint evaluates all gates, even in case of failure.
477-
/// This implementation exits on first error, as there is no need for the partial outputs yet.
472+
/// The original Cairo hint evaluates all gates, even in case of failure. This implementation exits on first error, as there is no need for the partial outputs yet.
478473
fn build_gate_evaluation<'ctx, 'this>(
479474
context: &'this Context,
480475
mut block: &'this Block<'ctx>,
@@ -484,44 +479,43 @@ fn build_gate_evaluation<'ctx, 'this>(
484479
circuit_data: Value<'ctx, 'ctx>,
485480
circuit_modulus: Value<'ctx, 'ctx>,
486481
) -> Result<([&'this Block<'ctx>; 2], Vec<Value<'ctx, 'ctx>>)> {
487-
// Each gate is represented as a MLIR value, and identified by an offset in the gate vector.
488-
// - `None` implies that the gate value *has not* been compiled yet.
489-
// - `Some` implies that the gate values *has* already been compiled, and therefore can be safely used.
490-
// Initially, some gate values are already known.
491-
let mut gates = vec![None; 1 + circuit_info.n_inputs + circuit_info.values.len()];
492-
493-
// The first gate always has a value of 1. It is implicity referred by some gate offsets.
494-
gates[0] = Some(block.const_int(context, location, 1, 384)?);
482+
// Throughout the evaluation of the circuit we maintain an array of known gate values
483+
// Initially, it only contains the inputs of the circuit.
484+
// Unknown values are represented as None
495485

496-
// The input gates are also known at the start. We take them from the `circuit_data` array.
497-
let u384_type = IntegerType::new(context, 384).into();
486+
let mut values = vec![None; 1 + circuit_info.n_inputs + circuit_info.values.len()];
487+
values[0] = Some(block.const_int(context, location, 1, 384)?);
498488
for i in 0..circuit_info.n_inputs {
499489
let value_ptr = block.gep(
500490
context,
501491
location,
502492
circuit_data,
503493
&[GepIndex::Const(i as i32)],
504-
u384_type,
494+
IntegerType::new(context, 384).into(),
505495
)?;
506-
gates[i + 1] = Some(block.load(context, location, value_ptr, u384_type)?);
496+
values[i + 1] = Some(block.load(
497+
context,
498+
location,
499+
value_ptr,
500+
IntegerType::new(context, 384).into(),
501+
)?);
507502
}
508503

509504
let err_block = helper.append_block(Block::new(&[(
510505
IntegerType::new(context, 64).into(),
511506
location,
512507
)]));
513-
let ok_block = helper.append_block(Block::new(&[]));
514508

515509
let mut add_offsets = circuit_info.add_offsets.iter().peekable();
516510
let mut mul_offsets = circuit_info.mul_offsets.iter().enumerate();
517511

518512
// We loop until all gates have been solved
519513
loop {
520514
// We iterate the add gate offsets as long as we can
521-
while let Some(&gate_offset) = add_offsets.peek() {
522-
let lhs_value = gates[gate_offset.lhs].to_owned();
523-
let rhs_value = gates[gate_offset.rhs].to_owned();
524-
let output_value = gates[gate_offset.output].to_owned();
515+
while let Some(&add_gate_offset) = add_offsets.peek() {
516+
let lhs_value = values[add_gate_offset.lhs].to_owned();
517+
let rhs_value = values[add_gate_offset.rhs].to_owned();
518+
let output_value = values[add_gate_offset.output].to_owned();
525519

526520
// Depending on the values known at the time, we can deduce if we are dealing with an ADD gate or a SUB gate.
527521
match (lhs_value, rhs_value, output_value) {
@@ -550,7 +544,7 @@ fn build_gate_evaluation<'ctx, 'this>(
550544
// Truncate back
551545
let value =
552546
block.trunci(value, IntegerType::new(context, 384).into(), location)?;
553-
gates[gate_offset.output] = Some(value);
547+
values[add_gate_offset.output] = Some(value);
554548
}
555549
// SUB: lhs = out - rhs
556550
(None, Some(rhs_value), Some(output_value)) => {
@@ -578,7 +572,7 @@ fn build_gate_evaluation<'ctx, 'this>(
578572
// Truncate back
579573
let value =
580574
block.trunci(value, IntegerType::new(context, 384).into(), location)?;
581-
gates[gate_offset.lhs] = Some(value);
575+
values[add_gate_offset.lhs] = Some(value);
582576
}
583577
// We can't solve this add gate yet, so we break from the loop
584578
_ => break,
@@ -588,10 +582,12 @@ fn build_gate_evaluation<'ctx, 'this>(
588582
}
589583

590584
// If we can't advance any more with add gate offsets, then we solve the next mul gate offset and go back to the start of the loop (solving add gate offsets).
591-
if let Some((gate_offset_idx, gate_offset)) = mul_offsets.next() {
592-
let lhs_value = gates[gate_offset.lhs].to_owned();
593-
let rhs_value = gates[gate_offset.rhs].to_owned();
594-
let output_value = gates[gate_offset.output].to_owned();
585+
if let Some((gate_offset_idx, &circuit::GateOffsets { lhs, rhs, output })) =
586+
mul_offsets.next()
587+
{
588+
let lhs_value = values[lhs].to_owned();
589+
let rhs_value = values[rhs].to_owned();
590+
let output_value = values[output].to_owned();
595591

596592
// Depending on the values known at the time, we can deduce if we are dealing with an MUL gate or a INV gate.
597593
match (lhs_value, rhs_value, output_value) {
@@ -620,7 +616,7 @@ fn build_gate_evaluation<'ctx, 'this>(
620616
// Truncate back
621617
let value =
622618
block.trunci(value, IntegerType::new(context, 384).into(), location)?;
623-
gates[gate_offset.output] = Some(value)
619+
values[output] = Some(value)
624620
}
625621
// INV: lhs = 1 / rhs
626622
(None, Some(rhs_value), Some(_)) => {
@@ -695,7 +691,7 @@ fn build_gate_evaluation<'ctx, 'this>(
695691
let inverse =
696692
block.trunci(inverse, IntegerType::new(context, 384).into(), location)?;
697693

698-
gates[gate_offset.lhs] = Some(inverse);
694+
values[lhs] = Some(inverse);
699695
}
700696
// The imposibility to solve this mul gate offset would render the circuit unsolvable
701697
_ => return Err(SierraAssertError::ImpossibleCircuit.into()),
@@ -706,17 +702,15 @@ fn build_gate_evaluation<'ctx, 'this>(
706702
}
707703
}
708704

709-
block.append_operation(cf::br(ok_block, &[], location));
710-
711705
// Validate all values have been calculated
712706
// Should only fail if the circuit is not solvable (bad form)
713-
let evaluated_gates = gates
707+
let values = values
714708
.into_iter()
715709
.skip(1 + circuit_info.n_inputs)
716710
.collect::<Option<Vec<Value>>>()
717711
.ok_or(SierraAssertError::ImpossibleCircuit)?;
718712

719-
Ok(([ok_block, err_block], evaluated_gates))
713+
Ok(([block, err_block], values))
720714
}
721715

722716
/// Generate MLIR operations for the `circuit_failure_guarantee_verify` libfunc.
@@ -882,8 +876,6 @@ fn build_get_output<'ctx, 'this>(
882876
};
883877
let output_type_id = &info.output_ty;
884878

885-
let u384_type = IntegerType::new(context, 384).into();
886-
887879
let output_offset_idx = *circuit_info
888880
.values
889881
.get(output_type_id)
@@ -893,7 +885,7 @@ fn build_get_output<'ctx, 'this>(
893885

894886
let outputs = entry.arg(0)?;
895887

896-
let circuit_ptr = entry.extract_value(
888+
let values_ptr = entry.extract_value(
897889
context,
898890
location,
899891
outputs,
@@ -908,15 +900,20 @@ fn build_get_output<'ctx, 'this>(
908900
1,
909901
)?;
910902

911-
let output_integer_ptr = entry.gep(
903+
let output_struct_ptr = entry.gep(
912904
context,
913905
location,
914-
circuit_ptr,
906+
values_ptr,
915907
&[GepIndex::Const(output_idx as i32)],
916-
u384_type,
908+
build_u384_struct_type(context),
909+
)?;
910+
911+
let output_struct = entry.load(
912+
context,
913+
location,
914+
output_struct_ptr,
915+
build_u384_struct_type(context),
917916
)?;
918-
let output_integer = entry.load(context, location, output_integer_ptr, u384_type)?;
919-
let output_struct = u384_integer_to_struct(context, entry, location, output_integer)?;
920917

921918
let guarantee_type_id = &info.branch_signatures()[0].vars[1].ty;
922919
let guarantee = build_struct_value(

src/types/circuit.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -278,17 +278,16 @@ pub fn build_circuit_data<'ctx>(
278278
///
279279
/// ## Layout:
280280
///
281-
/// Holds the evaluated circuit output gates and the circuit modulus.
282-
/// - The data is stored as a dynamic array of u384 integers.
283-
/// - The modulus is stored as a u384 in struct form (multi-limb).
281+
/// Holds N_VALUES elements, where each element is a u384 struct,
282+
/// A u384 struct contains 4 limbs, each a u96 integer.
284283
///
285284
/// ```txt
286285
/// type = struct {
287-
/// data: *u384,
288-
/// modulus: u384struct,
286+
/// data: *u384s,
287+
/// modulus: u384s,
289288
/// };
290289
///
291-
/// u384struct = struct {
290+
/// u384s = struct {
292291
/// limb1: u96,
293292
/// limb2: u96,
294293
/// limb3: u96,
@@ -332,15 +331,15 @@ pub fn build_circuit_outputs<'ctx>(
332331
0,
333332
)?;
334333

335-
let u384_integer_layout = get_integer_layout(384);
334+
let u384_struct_layout = layout_repeat(&get_integer_layout(96), 4)?.0;
336335

337336
let new_gates_ptr = build_array_dup(
338337
context,
339338
&entry,
340339
location,
341340
gates_ptr,
342341
circuit.circuit_info.values.len(),
343-
u384_integer_layout,
342+
u384_struct_layout,
344343
)?;
345344

346345
let new_outputs = entry.insert_value(context, location, outputs, new_gates_ptr, 0)?;

0 commit comments

Comments
 (0)