Skip to content

Commit

Permalink
hide-step: Simplify tests and keep details of StepRecord private (#226)
Browse files Browse the repository at this point in the history 
Co-authored-by: Aurélien Nicolas <[email protected]>
  • Loading branch information
@naure
naure and Aurélien Nicolas authored Sep 15, 2024
1 parent a8174ae commit b300e9b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 110 deletions.
1 change: 1 addition & 0 deletions ceno_emul/src/addr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use std::{fmt, ops};

pub const WORD_SIZE: usize = 4;
pub const PC_STEP_SIZE: usize = 4;

// Type aliases to clarify the code without wrapper types.
pub type Word = u32;
Expand Down
52 changes: 42 additions & 10 deletions ceno_emul/src/tracer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt, mem};
use crate::{
addr::{ByteAddr, Cycle, RegIdx, Word, WordAddr},
rv32im::DecodedInstruction,
CENO_PLATFORM,
CENO_PLATFORM, PC_STEP_SIZE,
};

/// An instruction and its context in an execution trace. That is concrete values of registers and memory.
Expand All @@ -17,18 +17,18 @@ use crate::{
/// - Any of `rs1 / rs2 / rd` **may be `x0`**. The trace handles this like any register, including the value that was _supposed_ to be stored. The circuits must handle this case: either **store `0` or skip `x0` operations**.
///
/// - Any pair of `rs1 / rs2 / rd` **may be the same**. Then, one op will point to the other op in the same instruction but a different subcycle. The circuits may follow the operations **without special handling** of repeated registers.
#[derive(Clone, Debug, Default)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct StepRecord {
pub cycle: Cycle,
pub pc: Change<ByteAddr>,
pub insn_code: Word,
cycle: Cycle,
pc: Change<ByteAddr>,
insn_code: Word,

pub rs1: Option<ReadOp>,
pub rs2: Option<ReadOp>,
rs1: Option<ReadOp>,
rs2: Option<ReadOp>,

pub rd: Option<WriteOp>,
rd: Option<WriteOp>,

pub memory_op: Option<WriteOp>,
memory_op: Option<WriteOp>,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand All @@ -53,6 +53,38 @@ pub type ReadOp = MemOp<Word>;
pub type WriteOp = MemOp<Change<Word>>;

impl StepRecord {
pub fn new_r_instruction(
cycle: Cycle,
pc: ByteAddr,
insn_code: Word,
rs1_read: Word,
rs2_read: Word,
rd: Change<Word>,
) -> StepRecord {
let insn = DecodedInstruction::new(insn_code);
StepRecord {
cycle,
pc: Change::new(pc, pc + PC_STEP_SIZE),
insn_code,
rs1: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(insn.rs1() as RegIdx).into(),
value: rs1_read,
previous_cycle: 0,
}),
rs2: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(insn.rs2() as RegIdx).into(),
value: rs2_read,
previous_cycle: 0,
}),
rd: Some(WriteOp {
addr: CENO_PLATFORM.register_vma(insn.rd() as RegIdx).into(),
value: rd,
previous_cycle: 0,
}),
memory_op: None,
}
}

pub fn cycle(&self) -> Cycle {
self.cycle
}
Expand Down Expand Up @@ -210,7 +242,7 @@ impl Tracer {
}
}

#[derive(Copy, Clone, Default)]
#[derive(Copy, Clone, Default, PartialEq, Eq)]
pub struct Change<T> {
pub before: T,
pub after: T,
Expand Down
132 changes: 34 additions & 98 deletions ceno_zkvm/src/instructions/riscv/addsub.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,14 @@ impl<E: ExtensionField> Instruction<E> for SubInstruction<E> {

#[cfg(test)]
mod test {
use ceno_emul::{Change, ReadOp, StepRecord, WriteOp, CENO_PLATFORM};
use ceno_emul::{Change, StepRecord};
use goldilocks::GoldilocksExt2;
use itertools::Itertools;
use multilinear_extensions::mle::IntoMLEs;

use crate::{
circuit_builder::{CircuitBuilder, ConstraintSystem},
instructions::{riscv::constants::PC_STEP_SIZE, Instruction},
instructions::Instruction,
scheme::mock_prover::{MockProver, MOCK_PC_ADD, MOCK_PC_SUB, MOCK_PROGRAM},
};

Expand All @@ -316,30 +316,14 @@ mod test {
let (raw_witin, _) = AddInstruction::assign_instances(
&config,
cb.cs.num_witin as usize,
vec![StepRecord {
cycle: 3,
pc: Change::new(MOCK_PC_ADD, MOCK_PC_ADD + PC_STEP_SIZE),
insn_code: MOCK_PROGRAM[0],
rs1: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(2).into(),
value: 11u32,
previous_cycle: 0,
}),
rs2: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(3).into(),
value: 0xfffffffeu32,
previous_cycle: 0,
}),
rd: Some(WriteOp {
addr: CENO_PLATFORM.register_vma(4).into(),
value: Change {
before: 0u32,
after: 11u32.wrapping_add(0xfffffffeu32),
},
previous_cycle: 0,
}),
..Default::default()
}],
vec![StepRecord::new_r_instruction(
3,
MOCK_PC_ADD,
MOCK_PROGRAM[0],
11,
0xfffffffe,
Change::new(0, 11_u32.wrapping_add(0xfffffffe)),
)],
)
.unwrap();

Expand Down Expand Up @@ -374,30 +358,14 @@ mod test {
let (raw_witin, _) = AddInstruction::assign_instances(
&config,
cb.cs.num_witin as usize,
vec![StepRecord {
cycle: 3,
pc: Change::new(MOCK_PC_ADD, MOCK_PC_ADD + PC_STEP_SIZE),
insn_code: MOCK_PROGRAM[0],
rs1: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(2).into(),
value: u32::MAX - 1,
previous_cycle: 0,
}),
rs2: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(3).into(),
value: u32::MAX - 1,
previous_cycle: 0,
}),
rd: Some(WriteOp {
addr: CENO_PLATFORM.register_vma(4).into(),
value: Change {
before: 0u32,
after: (u32::MAX - 1).wrapping_add(u32::MAX - 1),
},
previous_cycle: 0,
}),
..Default::default()
}],
vec![StepRecord::new_r_instruction(
3,
MOCK_PC_ADD,
MOCK_PROGRAM[0],
u32::MAX - 1,
u32::MAX - 1,
Change::new(0, (u32::MAX - 1).wrapping_add(u32::MAX - 1)),
)],
)
.unwrap();

Expand Down Expand Up @@ -432,30 +400,14 @@ mod test {
let (raw_witin, _) = SubInstruction::assign_instances(
&config,
cb.cs.num_witin as usize,
vec![StepRecord {
cycle: 3,
pc: Change::new(MOCK_PC_SUB, MOCK_PC_SUB + PC_STEP_SIZE),
insn_code: MOCK_PROGRAM[1],
rs1: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(2).into(),
value: 11u32,
previous_cycle: 0,
}),
rs2: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(3).into(),
value: 2u32,
previous_cycle: 0,
}),
rd: Some(WriteOp {
addr: CENO_PLATFORM.register_vma(4).into(),
value: Change {
before: 0u32,
after: 11u32.wrapping_sub(2u32),
},
previous_cycle: 0,
}),
..Default::default()
}],
vec![StepRecord::new_r_instruction(
3,
MOCK_PC_SUB,
MOCK_PROGRAM[1],
11,
2,
Change::new(0, 11_u32.wrapping_sub(2)),
)],
)
.unwrap();

Expand Down Expand Up @@ -490,30 +442,14 @@ mod test {
let (raw_witin, _) = SubInstruction::assign_instances(
&config,
cb.cs.num_witin as usize,
vec![StepRecord {
cycle: 3,
pc: Change::new(MOCK_PC_SUB, MOCK_PC_SUB + PC_STEP_SIZE),
insn_code: MOCK_PROGRAM[1],
rs1: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(2).into(),
value: 3u32,
previous_cycle: 0,
}),
rs2: Some(ReadOp {
addr: CENO_PLATFORM.register_vma(3).into(),
value: 11u32,
previous_cycle: 0,
}),
rd: Some(WriteOp {
addr: CENO_PLATFORM.register_vma(4).into(),
value: Change {
before: 0u32,
after: 3u32.wrapping_sub(11u32),
},
previous_cycle: 0,
}),
..Default::default()
}],
vec![StepRecord::new_r_instruction(
3,
MOCK_PC_SUB,
MOCK_PROGRAM[1],
3,
11,
Change::new(0, 3_u32.wrapping_sub(11)),
)],
)
.unwrap();

Expand Down
3 changes: 1 addition & 2 deletions ceno_zkvm/src/instructions/riscv/constants.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::fmt;

use crate::uint::UInt;

pub(crate) const PC_STEP_SIZE: usize = 4;
pub use ceno_emul::PC_STEP_SIZE;

pub const OPCODE_OP: usize = 0x33;
pub const FUNCT3_ADD_SUB: usize = 0;
Expand Down

0 comments on commit b300e9b

Please sign in to comment.