Skip to content

Commit bfaeff4

Browse files
georgwieseleonardoalt
authored andcommitted
Remove next references in lookups / permutations (#2141)
Currently, when we run the Goldilocks RISC-V machine with `--linker-mode bus` results in: `Double application of "'" on: main_binary::operation_id` because of #2140. This PR hot-fixes it, so we can run benchmarks using the bus. Only adds 3 witness columns (1 in binary, 2 in shift).
1 parent 6ea63a7 commit bfaeff4

File tree

4 files changed

+34
-4
lines changed

4 files changed

+34
-4
lines changed

riscv-executor/src/submachines.rs

+15
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,18 @@ impl SubmachineKind for BinaryMachine {
266266
trace.set_current_row("A_byte", (a1 as u32).into());
267267
trace.set_current_row("B_byte", (b1 as u32).into());
268268
trace.set_current_row("C_byte", (c1 as u32).into());
269+
trace.set_current_row("operation_id_next", op_id);
269270
} else {
270271
trace.set_final_row("A_byte", (a1 as u32).into());
271272
trace.set_final_row("B_byte", (b1 as u32).into());
272273
trace.set_final_row("C_byte", (c1 as u32).into());
274+
trace.set_final_row("operation_id_next", op_id);
273275
}
274276

275277
// 4 rows for each binary operation
276278
trace.push_row();
277279
trace.set_current_row("operation_id", op_id);
280+
trace.set_current_row("operation_id_next", op_id);
278281
trace.set_current_row("A_byte", (a2 as u32).into());
279282
trace.set_current_row("B_byte", (b2 as u32).into());
280283
trace.set_current_row("C_byte", (c2 as u32).into());
@@ -284,6 +287,7 @@ impl SubmachineKind for BinaryMachine {
284287

285288
trace.push_row();
286289
trace.set_current_row("operation_id", op_id);
290+
trace.set_current_row("operation_id_next", op_id);
287291
trace.set_current_row("A_byte", (a3 as u32).into());
288292
trace.set_current_row("B_byte", (b3 as u32).into());
289293
trace.set_current_row("C_byte", (c3 as u32).into());
@@ -293,6 +297,7 @@ impl SubmachineKind for BinaryMachine {
293297

294298
trace.push_row();
295299
trace.set_current_row("operation_id", op_id);
300+
trace.set_current_row("operation_id_next", op_id);
296301
trace.set_current_row("A_byte", (a4 as u32).into());
297302
trace.set_current_row("B_byte", (b4 as u32).into());
298303
trace.set_current_row("C_byte", (c4 as u32).into());
@@ -352,43 +357,53 @@ impl SubmachineKind for ShiftMachine {
352357
if trace.len() > 0 {
353358
trace.set_current_row("A_byte", (b1 as u32).into());
354359
trace.set_current_row("C_part", (((b1 as u32) << shl) >> shr).into());
360+
trace.set_current_row("operation_id_next", op_id);
361+
trace.set_current_row("B_next", b);
355362
} else {
356363
trace.set_final_row("A_byte", (b1 as u32).into());
357364
trace.set_final_row("C_part", (((b1 as u32) << shl) >> shr).into());
365+
trace.set_final_row("operation_id_next", op_id);
366+
trace.set_final_row("B_next", b);
358367
}
359368

360369
// 4 rows for each shift operation
361370
trace.push_row();
362371
trace.set_current_row("operation_id", op_id);
372+
trace.set_current_row("operation_id_next", op_id);
363373
trace.set_current_row("A_byte", (b2 as u32).into());
364374
let c_part_factor = (b2 as u32) << 8;
365375
let c_part = ((c_part_factor << shl) >> shr).into();
366376
trace.set_current_row("C_part", c_part);
367377
let a_row = a.u() & 0xff;
368378
trace.set_current_row("A", a_row.into());
369379
trace.set_current_row("B", b);
380+
trace.set_current_row("B_next", b);
370381
trace.set_current_row("C", ((a_row << shl) >> shr).into());
371382
//
372383
trace.push_row();
373384
trace.set_current_row("operation_id", op_id);
385+
trace.set_current_row("operation_id_next", op_id);
374386
trace.set_current_row("A_byte", (b3 as u32).into());
375387
let c_part_factor = (b3 as u32) << 16;
376388
let c_part = ((c_part_factor << shl) >> shr).into();
377389
trace.set_current_row("C_part", c_part);
378390
let a_row = a.u() & 0xffff;
379391
trace.set_current_row("A", a_row.into());
380392
trace.set_current_row("B", b);
393+
trace.set_current_row("B_next", b);
381394
trace.set_current_row("C", ((a_row << shl) >> shr).into());
382395
//
383396
trace.push_row();
384397
trace.set_current_row("operation_id", op_id);
398+
trace.set_current_row("operation_id_next", op_id);
385399
trace.set_current_row("A_byte", (b4 as u32).into());
386400
let c_part_factor = (b4 as u32) << 24;
387401
let c_part = ((c_part_factor << shl) >> shr).into();
388402
trace.set_current_row("C_part", c_part);
389403
let a_row = a.u() & 0xffffff;
390404
trace.set_current_row("A", a_row.into());
391405
trace.set_current_row("B", b);
406+
trace.set_current_row("B_next", b);
392407
trace.set_current_row("C", ((a_row << shl) >> shr).into());
393408
// latch row
394409
trace.push_row();

riscv/tests/common/mod.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use mktemp::Temp;
22
use powdr_number::{BabyBearField, FieldElement, GoldilocksField, KnownField, KoalaBearField};
33
use powdr_pipeline::{
4-
test_util::{run_pilcom_with_backend_variant, test_plonky3_pipeline, BackendVariant},
4+
test_util::{
5+
run_pilcom_with_backend_variant, test_mock_backend, test_plonky3_pipeline, BackendVariant,
6+
},
57
Pipeline,
68
};
79
use powdr_riscv::CompilerOptions;
@@ -32,6 +34,8 @@ pub fn verify_riscv_asm_string<T: FieldElement, S: serde::Serialize + Send + Syn
3234
// Compute the witness once for all tests that follow.
3335
pipeline.compute_witness().unwrap();
3436

37+
test_mock_backend(pipeline.clone());
38+
3539
// verify with PILCOM
3640
if T::known_field().unwrap() == KnownField::GoldilocksField {
3741
let pipeline_gl: Pipeline<GoldilocksField> =
@@ -72,7 +76,7 @@ pub fn verify_riscv_asm_string<T: FieldElement, S: serde::Serialize + Send + Syn
7276
pipeline.rollback_from_witness();
7377
let executor_trace: Vec<_> = execution.trace.into_iter().collect();
7478
let pipeline = pipeline.add_external_witness_values(executor_trace);
75-
test_plonky3_pipeline::<T>(pipeline);
79+
test_mock_backend(pipeline);
7680
}
7781
}
7882

std/machines/large_field/binary.asm

+6-1
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,10 @@ machine Binary(byte_binary: ByteBinary) with
3535
B' = B * (1 - latch) + B_byte * FACTOR;
3636
C' = C * (1 - latch) + C_byte * FACTOR;
3737

38-
link => C_byte = byte_binary.run(operation_id', A_byte, B_byte);
38+
// TODO: Currently, the bus linker does not support next references in operations and links.
39+
// We add an extra witness column to make the Goldilocks RISC-V machine work for now.
40+
// This will be fixed with #2140.
41+
col witness operation_id_next;
42+
operation_id' = operation_id_next;
43+
link => C_byte = byte_binary.run(operation_id_next, A_byte, B_byte);
3944
}

std/machines/large_field/shift.asm

+7-1
Original file line numberDiff line numberDiff line change
@@ -67,5 +67,11 @@ machine Shift(byte_shift: ByteShift) with
6767
unchanged_until(B, latch);
6868
C' = C * (1 - latch) + C_part;
6969

70-
link => C_part = byte_shift.run(operation_id', A_byte, B', FACTOR_ROW);
70+
// TODO: Currently, the bus linker does not support next references in operations and links.
71+
// We add an extra witness columns to make the Goldilocks RISC-V machine work for now.
72+
// This will be fixed with #2140.
73+
col witness operation_id_next, B_next;
74+
operation_id' = operation_id_next;
75+
B' = B_next;
76+
link => C_part = byte_shift.run(operation_id_next, A_byte, B_next, FACTOR_ROW);
7177
}

0 commit comments

Comments
 (0)